rector: RemoveUnusedVariableAssignRector (no automatic changes)#5415
rector: RemoveUnusedVariableAssignRector (no automatic changes)#5415sreichel wants to merge 74 commits intoOpenMage:mainfrom
RemoveUnusedVariableAssignRector (no automatic changes)#5415Conversation
- see https://getrector.com/rule-detail/remove-unused-variable-assign-rector Rector only removes method var, no not method call ... (thanks)
RemoveUnusedVariableAssignRector (no automatic changes)
…signRector' into rector/dc/RemoveUnusedVariableAssignRector
|
There was a problem hiding this comment.
Pull request overview
This PR applies dead-code cleanup (primarily removal of unused local variable assignments) across core/test code, and updates local tooling configuration (Composer scripts, PHPMD rules/baselines) to better enforce unused-code detection.
Changes:
- Remove unused local variable assignments and other no-op locals across many core classes/controllers and unit tests.
- Update Composer scripts and Makefile targets to use
phpmd:test/phpstan:testnaming, and add baseline-generation scripts. - Enable/adjust PHPMD rules (notably
UnusedLocalVariable) and update PHPMD/PHPStan baselines accordingly.
Reviewed changes
Copilot reviewed 145 out of 145 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/Traits/DataProvider/Mage/Core/Helper/DataTrait.php | Remove unused date formatting local. |
| tests/unit/Mage/Core/Helper/EnvironmentConfigLoaderTest.php | Remove unused env-var path local. |
| Makefile | Update phpmd helper message to new Composer script name. |
| lib/Varien/Simplexml/Config.php | Remove unused assignment; keep fluent return. |
| lib/Varien/Filter/Template/Tokenizer/Variable.php | Remove unused local initialization. |
| lib/Varien/Filter/Email.php | Simplify method body (but now empty). |
| lib/Varien/Db/Tree.php | Remove unused local. |
| lib/Varien/Data/Form/Element/Gallery.php | Remove unused local. |
| lib/Varien/Data/Collection.php | Tighten docblock generics. |
| lib/Varien/Convert/Parser/Csv.php | Remove unused locals/blank lines. |
| lib/Varien/Convert/Container/Abstract.php | Iterate array keys only (avoid unused values). |
| lib/Mage/HTTP/Client/Socket.php | Remove unused local; minor call ordering cleanup. |
| lib/Mage/Archive/Tar.php | Remove unused locals/assignments in pack/header parse. |
| composer.json | Rename scripts to phpmd:test / phpstan:test, add baseline scripts, update test script. |
| app/code/core/Mage/Widget/Model/Widget/Instance.php | Iterate product type keys only. |
| app/code/core/Mage/Widget/Block/Adminhtml/Widget/Instance/Edit/Tab/Settings.php | Remove unused local. |
| app/code/core/Mage/Widget/Block/Adminhtml/Widget/Instance/Edit/Tab/Main/Layout.php | Iterate product type keys only. |
| app/code/core/Mage/Widget/Block/Adminhtml/Widget/Form.php | Remove unused local assignment. |
| app/code/core/Mage/Weee/Model/Total/Quote/Weee.php | Remove unused totals locals and unused discount locals. |
| app/code/core/Mage/Usa/Model/Shipping/Carrier/Ups/Source/Unitofmeasure.php | Iterate keys only (unused values removed). |
| app/code/core/Mage/Usa/Model/Shipping/Carrier/Ups/Source/OriginShipment.php | Iterate keys only (unused values removed). |
| app/code/core/Mage/Usa/Model/Shipping/Carrier/Ups.php | Remove unused defaults local. |
| app/code/core/Mage/Usa/Model/Shipping/Carrier/Dhl.php | Remove unused locals. |
| app/code/core/Mage/Tax/Model/Sales/Total/Quote/Tax.php | Remove unused locals; whitespace adjustments. |
| app/code/core/Mage/Tax/Model/Sales/Pdf/Tax.php | Remove unused local. |
| app/code/core/Mage/Tax/Model/Observer.php | Remove unused locals; keep legacy commented join section. |
| app/code/core/Mage/Tax/Helper/Data.php | Minor whitespace cleanup. |
| app/code/core/Mage/Tag/controllers/CustomerController.php | Remove unused assignment to loaded/aggregated tag model. |
| app/code/core/Mage/Shipping/Model/Carrier/Tablerate.php | Iterate code keys only; whitespace cleanup. |
| app/code/core/Mage/Shipping/Model/Carrier/Pickup.php | Remove previous rate/method append logic (now returns empty result). |
| app/code/core/Mage/Sales/Model/Resource/Order/Status/Collection.php | Tighten docblock return type. |
| app/code/core/Mage/Sales/Model/Quote/Address/Total/Msrp.php | Remove unused quote/store locals. |
| app/code/core/Mage/Sales/Model/Quote/Address/Total/Discount.php | Remove unused $hasDiscount tracking. |
| app/code/core/Mage/Sales/Model/Order/Status.php | Remove unused local. |
| app/code/core/Mage/Sales/Model/Order/Pdf/Abstract.php | Remove unused locals; rename unused loop variable. |
| app/code/core/Mage/Sales/Model/Order/Creditmemo/Total/Tax.php | Remove unused locals. |
| app/code/core/Mage/Sales/Model/Order/Config.php | Tighten docblock types. |
| app/code/core/Mage/Sales/Model/Order.php | Remove unused assignment; keep fluent call chain. |
| app/code/core/Mage/Sales/Model/Entity/Sale/Collection.php | Remove unused locals; replace empty() with strict array check; iterate keys only. |
| app/code/core/Mage/Sales/Model/Config.php | Rename unused foreach value to $ignored. |
| app/code/core/Mage/Sales/Controller/Abstract.php | Use in_array(..., true) without unused $strict var. |
| app/code/core/Mage/Sales/Block/Recurring/Profiles.php | Remove unused local. |
| app/code/core/Mage/Sales/Block/Adminhtml/Report/Filter/Form/Order.php | Remove unused locals. |
| app/code/core/Mage/Rule/Model/Condition/Combine.php | Iterate option keys only. |
| app/code/core/Mage/Reports/Model/Resource/Review/Product/Collection.php | Remove unused helper local. |
| app/code/core/Mage/Paypal/Model/Pro.php | Fix validation to check schedule description length. |
| app/code/core/Mage/Paypal/Model/Express.php | Remove unused transaction locals. |
| app/code/core/Mage/Paypal/Model/Api/Nvp.php | Remove unused call result local. |
| app/code/core/Mage/Payment/Model/Config.php | Tighten docblock generics. |
| app/code/core/Mage/Log/Model/Visitor.php | Remove assignment-in-condition of $customer. |
| app/code/core/Mage/Install/Model/Wizard.php | Iterate step keys only. |
| app/code/core/Mage/Install/Model/Installer.php | Add finish() docblock; iterate cache type keys only. |
| app/code/core/Mage/Install/Model/Config.php | Tighten docblocks; rename unused foreach value to $ignored. |
| app/code/core/Mage/Index/Model/Resource/Setup.php | Rename unused foreach value to $ignored. |
| app/code/core/Mage/ImportExport/Model/Export/Entity/Product.php | Remove unused locals; simplify row collection flow. |
| app/code/core/Mage/ImportExport/Block/Adminhtml/Export/Filter.php | Split assignment from condition; minor simplifications. |
| app/code/core/Mage/Eav/Model/Entity/Setup.php | Remove unused $defaultGroup local. |
| app/code/core/Mage/Eav/Model/Entity/Collection/Abstract.php | Remove unused helper local; simplify linked-entity selection. |
| app/code/core/Mage/Eav/Model/Entity/Attribute.php | Remove unused local. |
| app/code/core/Mage/Downloadable/Helper/File.php | Remove unused locals; keep side-effect call. |
| app/code/core/Mage/Dataflow/Model/Session/Parser/Csv.php | Remove unused locals. |
| app/code/core/Mage/Dataflow/Model/Profile.php | Refactor file array access; avoid assignment-in-condition. |
| app/code/core/Mage/Dataflow/Model/Convert/Parser/Xml/Excel.php | Remove unused locals; remove unused assignment before fluent chain. |
| app/code/core/Mage/Dataflow/Model/Convert/Parser/Csv.php | Remove unused assignment before fluent chain; remove unused locals. |
| app/code/core/Mage/Dataflow/Model/Convert/Iterator/Http.php | Remove unused $row variable from CSV loop. |
| app/code/core/Mage/Dataflow/Model/Convert/Container/Abstract.php | Iterate row keys only. |
| app/code/core/Mage/Customer/Model/Convert/Parser/Customer.php | Remove unused locals; remove unused assignment before fluent chain. |
| app/code/core/Mage/Core/Model/Url.php | Remove unused $routePath building locals in setRoutePath(). |
| app/code/core/Mage/Core/Model/Resource/Translate/String.php | Rename unused explode part to $ignored; add (typo’d) comment. |
| app/code/core/Mage/Core/Model/Resource/Setup.php | Replace empty($sql) with strict truthy check; remove unused local. |
| app/code/core/Mage/Core/Model/Resource/Helper/Mysql4.php | Remove unused $matches variable from preg_match(). |
| app/code/core/Mage/Core/Model/Resource/Db/Abstract.php | Rename unused tuple elements to $ignored in destructuring. |
| app/code/core/Mage/Core/Model/Layout/Update.php | Remove unused assignment; remove unused local. |
| app/code/core/Mage/Core/Model/Layout/Element.php | Remove unused locals. |
| app/code/core/Mage/Core/Model/Design/Source/Design.php | Remove unused local. |
| app/code/core/Mage/Core/Model/Config.php | Rename unused foreach value to $ignored. |
| app/code/core/Mage/Core/Model/Cache.php | Iterate keys only. |
| app/code/core/Mage/Core/Helper/Data.php | Tighten docblock generics. |
| app/code/core/Mage/Core/Block/Profiler.php | Iterate timer keys only. |
| app/code/core/Mage/Core/Block/Html/Link.php | Remove unused locals. |
| app/code/core/Mage/Core/Block/Html/Calendar.php | Remove unused days list; comment updated. |
| app/code/core/Mage/Checkout/Model/Type/Abstract.php | Remove unused $mailer local (keep fluent call). |
| app/code/core/Mage/Checkout/Model/Cart/Payment/Api.php | Iterate cc type keys only; strict in_array; replace empty() with === []. |
| app/code/core/Mage/Checkout/Model/Cart.php | Remove unused locals / keep side-effect call. |
| app/code/core/Mage/Checkout/controllers/MultishippingController.php | Replace assignment-in-condition with explicit diff check. |
| app/code/core/Mage/CatalogInventory/Model/Resource/Stock/Status.php | Remove unused local. |
| app/code/core/Mage/Catalog/Model/Resource/Category/Flat.php | Remove unused locals. |
| app/code/core/Mage/Catalog/Model/Resource/Category/Collection.php | Remove assignment-in-condition; keep truthy check. |
| app/code/core/Mage/Catalog/Model/Resource/Category/Attribute/Source/Layout.php | Remove unused local. |
| app/code/core/Mage/Catalog/Model/Product/Type.php | Tighten docblock return shape for getTypes(). |
| app/code/core/Mage/Catalog/Model/Product/Option/Type/Date.php | Remove unused $option locals. |
| app/code/core/Mage/Catalog/Model/Product/Option/Observer.php | Remove unused assignment; keep fluent chain. |
| app/code/core/Mage/Catalog/Model/Product/Attribute/Media/Api.php | Remove unused assignment; keep side-effect call. |
| app/code/core/Mage/Catalog/Model/Convert/Adapter/Product.php | Iterate option keys only. |
| app/code/core/Mage/Api2/Model/Config.php | Rename unused foreach value to $ignored; tighten docblock. |
| app/code/core/Mage/Api/Model/Wsdl/Config/Element.php | Rename unused foreach variable to $ignored. |
| app/code/core/Mage/Api/Model/Wsdl/Config.php | Remove unused local. |
| app/code/core/Mage/Api/Model/Server/Adapter/Soap.php | Remove unused local; keep side-effect setter call. |
| app/code/core/Mage/Api/Model/Config.php | Rename unused foreach value to $ignored. |
| app/code/core/Mage/Adminhtml/Model/System/Config/Source/Design/Package.php | Replace incomplete method with empty body. |
| app/code/core/Mage/Adminhtml/Model/System/Config/Source/Category.php | Remove unused locals. |
| app/code/core/Mage/Adminhtml/Model/Sales/Order/Create.php | Iterate keys only; refactor removeItem branches; add TODO comments. |
| app/code/core/Mage/Adminhtml/controllers/Tax/RateController.php | Remove unused helper local. |
| app/code/core/Mage/Adminhtml/controllers/Tax/ClassController.php | Remove unused locals. |
| app/code/core/Mage/Adminhtml/controllers/TagController.php | Remove unused storeId local. |
| app/code/core/Mage/Adminhtml/controllers/System/Convert/GuiController.php | Remove unused locals, leaving no-op registry reads. |
| app/code/core/Mage/Adminhtml/controllers/Sales/OrderController.php | Replace unused-body action with empty method. |
| app/code/core/Mage/Adminhtml/controllers/Sales/Order/ShipmentController.php | Remove unused request param locals. |
| app/code/core/Mage/Adminhtml/controllers/Sales/Order/InvoiceController.php | Remove unused local. |
| app/code/core/Mage/Adminhtml/controllers/Sales/Order/CreditmemoController.php | Remove unused assignment; keep init call. |
| app/code/core/Mage/Adminhtml/controllers/Sales/Order/CreateController.php | Remove assignment-in-condition; keep truthy check. |
| app/code/core/Mage/Adminhtml/controllers/Report/ShopcartController.php | Remove unused local but leaves a no-op call. |
| app/code/core/Mage/Adminhtml/controllers/Newsletter/QueueController.php | Remove queue/template load logic from edit action. |
| app/code/core/Mage/Adminhtml/controllers/IndexController.php | Remove unused locals in login/global search handling. |
| app/code/core/Mage/Adminhtml/controllers/Cms/BlockController.php | Remove unused title local. |
| app/code/core/Mage/Adminhtml/controllers/Catalog/ProductController.php | Remove unused assignment; keep init call. |
| app/code/core/Mage/Adminhtml/controllers/Catalog/CategoryController.php | Remove assignment-in-condition; keep init call. |
| app/code/core/Mage/Adminhtml/controllers/CacheController.php | Remove unused return values; replace empty() checks with strict array checks. |
| app/code/core/Mage/Adminhtml/controllers/Api/RoleController.php | Remove unused resources local. |
| app/code/core/Mage/Adminhtml/Controller/Sales/Shipment.php | Replace assignment-in-condition; adjust array checks; print-action refactor. |
| app/code/core/Mage/Adminhtml/Controller/Sales/Invoice.php | Replace assignment-in-condition; adjust array checks; print-action refactor. |
| app/code/core/Mage/Adminhtml/Controller/Sales/Creditmemo.php | Replace assignment-in-condition; adjust array checks; print-action refactor. |
| app/code/core/Mage/Adminhtml/Block/Widget/Container.php | Tighten docblock generics; iterate keys only. |
| app/code/core/Mage/Adminhtml/Block/System/Config/Dwstree.php | Rename unused foreach value to $ignored. |
| app/code/core/Mage/Adminhtml/Block/System/Cache/Form.php | Remove commented-out unused options line. |
| app/code/core/Mage/Adminhtml/Block/Sales/Order/View/Messages.php | Remove unused local assignment; add note comment. |
| app/code/core/Mage/Adminhtml/Block/Sales/Order/Shipment/View/Form.php | Remove unused locals. |
| app/code/core/Mage/Adminhtml/Block/Sales/Order/Create/Items/Grid.php | Remove unused locals; reorder string concatenation. |
| app/code/core/Mage/Adminhtml/Block/Report/Sales/Tax/Grid.php | Iterate status keys only. |
| app/code/core/Mage/Adminhtml/Block/Permissions/Tab/Rolesedit.php | Remove unused locals. |
| app/code/core/Mage/Adminhtml/Block/Permissions/Tab/Roleinfo.php | Remove unused local. |
| app/code/core/Mage/Adminhtml/Block/Newsletter/Template/Preview/Form.php | Iterate data keys only. |
| app/code/core/Mage/Adminhtml/Block/Newsletter/Queue/Preview/Form.php | Iterate data keys only. |
| app/code/core/Mage/Adminhtml/Block/Cms/Wysiwyg/Images/Tree.php | Remove unused local; alignment cleanup. |
| app/code/core/Mage/Adminhtml/Block/Catalog/Product/Helper/Form/Price.php | Remove unused local. |
| app/code/core/Mage/Adminhtml/Block/Catalog/Product/Frontend/Product/Watermark.php | Remove unused local. |
| app/code/core/Mage/Adminhtml/Block/Catalog/Product/Edit/Tabs.php | Remove unused storeId derivation locals. |
| app/code/core/Mage/Adminhtml/Block/Catalog/Product/Attribute/Edit/Tabs.php | Reformat commented code to avoid unused locals. |
| app/code/core/Mage/Adminhtml/Block/Catalog/Product/Attribute/Edit/Tab/Main.php | Remove unused local assignment. |
| app/code/core/Mage/Adminhtml/Block/Api/Tab/Rolesedit.php | Remove unused locals. |
| app/code/core/Mage/Adminhtml/Block/Api/Tab/Roleinfo.php | Remove unused local. |
| .rector.php | Remove RemoveUnusedVariableAssignRector rule from configured set. |
| .phpstan.dist.baseline.php | Update ignore list/counts consistent with removed empty() and other changes. |
| .phpmd.dist.xml | Enable UnusedLocalVariable rule (with exception) and enable CountInLoopExpression in design ruleset. |
| .phpmd.dist.baseline.xml | Update baseline entries for newly enabled rules. |
Comments suppressed due to low confidence (4)
app/code/core/Mage/Adminhtml/controllers/Newsletter/QueueController.php:182
editAction()registerscurrent_queuebut no longer loads the queue byid(or initializes fromtemplate_id). This likely breaks editing existing queues because blocks read fromMage::registry('current_queue')and will get an empty model. Consider restoring the load/initialization logic (similar tosaveAction()) before rendering the layout.
app/code/core/Mage/Adminhtml/Controller/Sales/Invoice.php:112$invoiceis always truthy afterMage::getModel(...)->load($invoiceId), even when the entity doesn't exist. This condition won't prevent generating a PDF for a non-existent invoice. Check$invoice->getId()(and handle the not-found case) instead ofif ($invoice).
$invoiceId = $this->getRequest()->getParam('invoice_id');
if ($invoiceId) {
$invoice = Mage::getModel('sales/order_invoice')->load($invoiceId);
if ($invoice) {
$pdf = Mage::getModel('sales/order_pdf_invoice')->getPdf([$invoice]);
$this->_prepareDownloadResponse('invoice' . Mage::getSingleton('core/date')->date('Y-m-d_H-i-s')
. '.pdf', $pdf->render(), 'application/pdf');
}
app/code/core/Mage/Adminhtml/Controller/Sales/Creditmemo.php:115
$creditmemois always truthy afterMage::getModel(...)->load($creditmemoId), even when the entity doesn't exist. This condition won't prevent generating a PDF for a non-existent credit memo. Check$creditmemo->getId()(and handle the not-found case) instead ofif ($creditmemo).
$creditmemoId = $this->getRequest()->getParam('creditmemo_id');
if ($creditmemoId) {
$creditmemo = Mage::getModel('sales/order_creditmemo')->load($creditmemoId);
if ($creditmemo) {
$pdf = Mage::getModel('sales/order_pdf_creditmemo')->getPdf([$creditmemo]);
$this->_prepareDownloadResponse('creditmemo' . Mage::getSingleton('core/date')->date('Y-m-d_H-i-s')
. '.pdf', $pdf->render(), 'application/pdf');
}
app/code/core/Mage/Adminhtml/controllers/Report/ShopcartController.php:23
$this->getRequest()->getActionName();is now a no-op statement (the return value is unused and it has no side effects). It should be removed to avoid leaving dead code behind.
| public function collectRates(Mage_Shipping_Model_Rate_Request $request) | ||
| { | ||
| if (!$this->getConfigFlag('active')) { | ||
| return false; | ||
| } | ||
|
|
||
| $result = Mage::getModel('shipping/rate_result'); | ||
|
|
||
| if (!empty($rate)) { | ||
| $method = Mage::getModel('shipping/rate_result_method'); | ||
|
|
||
| $method->setCarrier('pickup'); | ||
| $method->setCarrierTitle($this->getConfigData('title')); | ||
|
|
||
| $method->setMethod('store'); | ||
| $method->setMethodTitle(Mage::helper('shipping')->__('Store Pickup')); | ||
|
|
||
| $method->setPrice(0); | ||
| $method->setCost(0); | ||
|
|
||
| $result->append($method); | ||
| } | ||
|
|
||
| return $result; | ||
| return Mage::getModel('shipping/rate_result'); | ||
| } |
There was a problem hiding this comment.
collectRates() now returns an empty shipping/rate_result without appending a method, so the pickup carrier will never produce a selectable shipping rate. It should create a shipping/rate_result_method, set carrier/method titles, set price/cost (typically 0), append it to the result, and then return the result.
| if (str_contains($original, '::')) { | ||
| [$scope, $original] = explode('::', $original); | ||
| // ignorerd scope | ||
| [$ignored, $original] = explode('::', $original); | ||
| } |
There was a problem hiding this comment.
Typo in comment: "ignorerd" should be "ignored".


Rector only removes method var, not the method call ... (thanks)
Related Pull Requests
UnusedLocalVariable#5392