Skip to content

[DeadCode] Skip RemoveDoubleAssignRector when assign target contains a side-effecting call#8110

Merged
TomasVotruba merged 1 commit into
mainfrom
fix-double-assign-call-in-target
Jun 29, 2026
Merged

[DeadCode] Skip RemoveDoubleAssignRector when assign target contains a side-effecting call#8110
TomasVotruba merged 1 commit into
mainfrom
fix-double-assign-call-in-target

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Bug

RemoveDoubleAssignRector removes repeated assignments when the left-hand target of the assignment contains a call expression with side effects.

Found while reviewing the skip list of spiral/framework's rector.php, which had to disable this rule on FinalizeAttributeTest:

$root->runScoped(static function (Container $c1): void {
    $c1->get(AttrScopeFooFinalize::class)->throwException = true;
    $c1->get(AttrScopeFooFinalize::class)->throwException = true; // removed by Rector
    $c1->get(AttrScopeFooFinalize::class)->throwException = true; // removed by Rector
}, name: 'foo');

Each ->get(...) resolves a fresh object from the container — a side-effecting call. Collapsing the duplicates drops those calls and changes behavior.

Cause

The rule already guards against calls on the right-hand side ($stmt->expr->expr) and inside try/catch, but never inspects the assignment target ($stmt->expr->var). A property-fetch chain such as $obj->getService()->prop hides a MethodCall in the target, so the duplicate was removed.

detectCallExpr() only inspects the top node, and the call here is nested under a PropertyFetch, so the check is done via BetterNodeFinder::findFirst() over the whole target subtree.

Fix

Skip the statement when the assignment target contains any side-effecting call expression. Added fixture skip_call_in_assign_target.php.inc (no-change), which is mangled without the guard.

@TomasVotruba

Copy link
Copy Markdown
Member Author

Looks good 👍

@TomasVotruba TomasVotruba merged commit ab3d1a8 into main Jun 29, 2026
65 checks passed
@TomasVotruba TomasVotruba deleted the fix-double-assign-call-in-target branch June 29, 2026 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant