rector: RemoveUnusedVariableAssignRector

[sreichel]

• see https://getrector.com/rule-detail/change-if-else-value-assign-to-early-return-rector

[Copilot]

Pull request overview

This pull request applies Rector's RemoveUnusedVariableAssignRector rule to remove unused variable assignments throughout the codebase. The changes are primarily automated refactoring that:

• Removes variables that are assigned but never used
• Renames single-letter or unclear variables to more descriptive names (e.g., $i → $index, $e → $exception)
• Simplifies method bodies by removing unnecessary assignments
• Adds missing @throws PHPDoc annotations

Reviewed changes

Copilot reviewed 99 out of 99 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
.rector.php	Removes RemoveUnusedVariableAssignRector from skip list to enable the rule
tests/unit/Traits/DataProvider/Mage/Core/Helper/DataTrait.php	Removes unused $dateShortTime variable
tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php	Removes unused $websiteWithUnderscorePath variable
lib/Varien/Simplexml/Config.php	Removes unused $xml return value capture
lib/Varien/Filter/Template/Tokenizer/Variable.php	Removes unused $numberStr variable
lib/Varien/Filter/Email.php	Simplifies empty method body
lib/Varien/Db/Tree.php	Renames variables and removes unused assignments; adds missing PHPDoc
lib/Varien/Data/Form/Element/Gallery.php	Renames $i to $index and removes commented code
lib/Varien/Convert/Parser/Csv.php	Renames variables and removes unused variable; adds PHPDoc
lib/Mage/HTTP/Client/Socket.php	Renames variables and removes unused return captures; adds PHPDoc
lib/Mage/Archive/Tar.php	Removes unused variables
app/code/core/Mage/Widget/*	Removes unused variables and improves code clarity
app/code/core/Mage/Usa/*	Removes unused variables from shipping carriers
app/code/core/Mage/Tax/*	Extensive unused variable removal and PHPDoc additions
app/code/core/Mage/Sales/*	Major refactoring with variable renaming and unused variable removal
app/code/core/Mage/Payment/*	Variable renaming and PHPDoc improvements
app/code/core/Mage/Index/*	Exception variable renaming and unused variable removal
app/code/core/Mage/Eav/*	Variable renaming and PHPDoc improvements
app/code/core/Mage/Catalog/*	Removes unused variables from catalog models
app/code/core/Mage/Api/*	Removes unused methods and variables
app/code/core/Mage/Adminhtml/controllers/*	Exception renaming and unused variable removal
app/code/core/Mage/Adminhtml/Block/*	Removes unused variables from blocks

Comments suppressed due to low confidence (1)

lib/Varien/Db/Tree.php:478

• In __moveNode() the catch block prints the exception message, the SQL statement ($sql), and the database adapter’s errorInfo() directly to the HTTP response, then terminates with exit(). If an attacker can cause this query to fail, they will see detailed SQL text and low-level driver error information, which can expose table/column names and other internal details useful for further attacks.

These debug outputs should be removed from the public response and replaced with server-side logging only, returning a generic error to the caller instead of echoing $sql and errorInfo().

[Copilot]

These catch blocks in appendChild() output raw exception messages, the SQL query string ($sql), and var_dump($data) directly to the HTTP response, then call exit(). An attacker who can trigger a database error here can gain detailed insight into your schema and application data (including potentially sensitive values in $data), leading to information disclosure and easier exploitation of other bugs.

You should remove the echo/var_dump calls from this error path and instead log the details server-side (e.g. via a logger) while returning a generic error message to the client.

[sonarqubecloud]

Please retry analysis of this Pull-Request directly on SonarQube Cloud

[Hanmac]

Probably should fix this? otherwise, it could crash?