[Arguments] Fix ReplaceArgumentDefaultValueRector re-applying already-migrated imported class constant - #8475
Merged
TomasVotruba merged 1 commit intoSep 8, 2026
Conversation
…-migrated imported class constant and report change in applied rules
TomasVotruba
deleted the
worktree-fix-9898-replace-arg-default-idempotency
branch
September 8, 2026 20:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9898
Problem
ReplaceArgumentDefaultValueRectorhad two related defects when a scalarvalueBeforeis migrated to a class constantvalueAfter(e.g.'lax'->Cookie::SAMESITE_LAX):Re-applies on already-migrated code. After the first run the argument holds
Cookie::SAMESITE_LAX, which resolves back to'lax', so the rule matched it again and rewrote the constant to a fully qualified re-fetch on every run. The existing idempotency guard only coveredself::/static::/parent::constants (isSpecialClassName()), not imported/FQ class constants.Change not reported in applied rules.
processReplaces()mutates the node in place and returns the same instance, but the caller checked$replacedNode !== $currentNode- always false - so valid migrations were applied to the AST without being attributed to the rule (diff shown, empty "Applied rules"). The siblingFunctionArgumentDefaultValueReplacerRectoralready uses the correctinstanceof Nodecheck.Fix
isSpecialClassName()requirement from the already-at-target guard so imported class constants are skipped too.processReplaces()returns a non-null node, matching the sibling rule.Test
Added
skip_imported_class_const_already_replacedfixture asserting an argument already holding the target imported constant is left untouched.