Repository navigation
[perf] Enforce single-pass rule application in tests, make rules idempotent - #8719
Merged
Merged
Conversation
Add a test-only guard in FileProcessor that fails a rule which changes a file again after it already changed it in a previous pass. Such a rule is not finished in a single pass and relies on the do-while loop re-running it, which masks incomplete rules. The guard is gated by the RECTOR_ENFORCE_SINGLE_PASS env (set in phpunit.xml), so sibling packages pulling rector-src as dev-main are not affected. Post rectors are excluded, as import handling legitimately converges over passes. Make the 7 rules that depended on the re-run resolve their nested or chained case inside refactor(): - ReplaceConstantBooleanNotRector - fold the whole !! negation chain - RemoveDeadZeroAndOneOperationRector - reduce the returned operand chain - PowToExpRector - convert nested pow() calls - TernaryToNullCoalescingRector - convert a nested coalescable else - ClosureToArrowFunctionRector - convert a nested closure body - ReplaceBlockToItsStmtsRector - flatten nested blocks - RemoveAlwaysElseRector - split nested always-else in the returned stmts
Member
Author
|
Heads up: the single red check, |
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.
Why
FileProcessor::processFile()runs rules in ado { ... } while (true)loop that re-runs the whole file until nothing changes. The loop is needed for legitimate cross-rule composition, but it also hides rules that are not finished in a single pass: a rule that re-applies to its own previous-pass output silently relies on the loop re-running it, instead of resolving the nested or chained case itself.What
Add a test-only guard in
FileProcessorthat fails a rule which changes a file again after it already changed it in the previous pass. The loop stays as the runtime safety net; the guard only turns the hidden dependency into a test failure.RECTOR_ENFORCE_SINGLE_PASSenv (set inphpunit.xml), so sibling packages pullingrector-srcasdev-mainare not affected by it.Make the rules that depended on the re-run resolve their nested or chained case inside
refactor():ReplaceConstantBooleanNotRector- fold the whole!!negation chain by parityRemoveDeadZeroAndOneOperationRector- reduce the returned operand chainPowToExpRector- convert nestedpow()callsTernaryToNullCoalescingRector- convert a nested coalescableelseReplaceBlockToItsStmtsRector- flatten nested blocksRemoveAlwaysElseRector- split nested always-else in the returned stmtsClosureToArrowFunctionRectorwas already made idempotent on main.All fixtures keep their current expected output - they now reach it in a single pass. Full suite, ECS and PHPStan are green.