Skip to content

[perf] Enforce single-pass rule application in tests, make rules idempotent - #8719

Merged
TomasVotruba merged 2 commits into
mainfrom
worktree-enforce-single-pass-rules
Oct 19, 2026
Merged

TomasVotruba merged 2 commits into
mainfrom
worktree-enforce-single-pass-rules

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Why

FileProcessor::processFile() runs rules in a do { ... } 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 FileProcessor that 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.

  • Gated by the RECTOR_ENFORCE_SINGLE_PASS env (set in phpunit.xml), so sibling packages pulling rector-src as dev-main are not affected by it.
  • Post rectors are excluded, as import handling legitimately converges over several passes.

Make the rules that depended on the re-run resolve their nested or chained case inside refactor():

  • ReplaceConstantBooleanNotRector - fold the whole !! negation chain by parity
  • RemoveDeadZeroAndOneOperationRector - reduce the returned operand chain
  • PowToExpRector - convert nested pow() calls
  • TernaryToNullCoalescingRector - convert a nested coalescable else
  • ReplaceBlockToItsStmtsRector - flatten nested blocks
  • RemoveAlwaysElseRector - split nested always-else in the returned stmts

ClosureToArrowFunctionRector was 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.

TomasVotruba and others added 2 commits October 19, 2026 19:20
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
@TomasVotruba

Copy link
Copy Markdown
Member Author

Heads up: the single red check, packages_tests (rector-downgrade-php, Unit Tests) - DowngradeNullJsonTest, already fails on main (e.g. commit fcdacd4) and is unrelated to this PR. The guard here is gated behind the RECTOR_ENFORCE_SINGLE_PASS env set only in this repo's phpunit.xml, so sibling packages are not affected by it - all other sibling jobs are green.

@TomasVotruba TomasVotruba changed the title Enforce single-pass rule application in tests, make rules idempotent [perf] Enforce single-pass rule application in tests, make rules idempotent Oct 19, 2026
@TomasVotruba
TomasVotruba merged commit 1133b77 into main Oct 19, 2026
48 of 49 checks passed
@TomasVotruba
TomasVotruba deleted the worktree-enforce-single-pass-rules branch October 19, 2026 17:27
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.

2 participants