Fix --only runs caching files as unchanged, hiding pending changes from full runs#8029
Merged
TomasVotruba merged 3 commits intoJun 19, 2026
Merged
Conversation
A file clean under one rule is not necessarily clean under all rules. An --only run cached such files as unchanged, so the next full run silently skipped their pending changes until --clear-cache. Covered by a new e2e scenario that fails on main: an --only run followed by a full run must still report the change pending under the other rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SanderMuller
added a commit
to SanderMuller/rector-src
that referenced
this pull request
Jun 10, 2026
The cache only checked each file's own content, so a clean file stayed skipped on warm runs even when one of its dependencies changed, e.g. a parent class method gaining a return type that lets a child file infer its own. A fresh run reports the new change, a warm run misses it. PHPStanNodeScopeResolver now records each file's dependencies during scope resolution using PHPStan's own DependencyResolver. Cache entries store the file's own hash plus one hash per dependency, all re-validated on load; legacy string entries self-upgrade on the next write. A failed capture skips caching entirely rather than caching a partial set. To keep capture cheap, only nodes whose resolver branch can produce dependencies are resolved, and native function calls are skipped via NativeFunctionCallAnalyzer, avoiding PHPStan's function signature map load. Two tripwire tests pin the external assumptions: one parses the bundled DependencyResolver and fails if its branch set changes on a PHPStan bump, the other asserts native function reflections stay fileless. Selective runs (--only, --only-suffix) bypass the cache write, same guard as rectorphp#8029. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
|
Thanks for the fix 👍 e2e seems heavy for such a verfication. |
ApplicationFileProcessor::processFiles() with no rules registered reaches the cache-write branch directly, so the guard is testable without an e2e project: an --only / --only-suffix run must leave the file uncached, a plain dry-run caches it. Fails on main, passes with the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Good call — replaced the e2e with a unit test, PR is now 81 lines total.
|
samsonasik
reviewed
Jun 10, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SanderMuller
added a commit
to SanderMuller/rector-src
that referenced
this pull request
Jun 11, 2026
The cache only checked each file's own content, so a clean file stayed skipped on warm runs even when one of its dependencies changed, e.g. a parent class method gaining a return type that lets a child file infer its own. A fresh run reports the new change, a warm run misses it. PHPStanNodeScopeResolver now records each file's dependencies during scope resolution using PHPStan's own DependencyResolver, the same engine behind PHPStan's result cache. Cache entries store the file's own hash plus one hash per dependency, all re-validated on load; legacy string entries self-upgrade on the next write. A failed capture skips caching entirely rather than caching a partial set. Function calls memoize their dependency files per resolved name, as signature dependencies are identical at every call site. Selective runs (--only, --only-suffix) bypass the cache write, same guard as rectorphp#8029. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SanderMuller
added a commit
to SanderMuller/rector-src
that referenced
this pull request
Jun 11, 2026
The cache only checked each file's own content, so a clean file stayed skipped on warm runs even when one of its dependencies changed, e.g. a parent class method gaining a return type that lets a child file infer its own. A fresh run reports the new change, a warm run misses it. PHPStanNodeScopeResolver now records each file's dependencies during scope resolution using PHPStan's own DependencyResolver, the same engine behind PHPStan's result cache. Cache entries store the file's own hash plus one hash per dependency, all re-validated on load; legacy string entries self-upgrade on the next write. A failed capture skips caching entirely rather than caching a partial set. Function calls memoize their dependency files per resolved name, as signature dependencies are identical at every call site. Selective runs (--only, --only-suffix) bypass the cache write, same guard as rectorphp#8029. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
|
I'd love to merge this one. @samsonasik Can you check if its ready? |
Member
|
@TomasVotruba seems ok, the drawback is with this change, the |
Member
|
@samsonasik Hm, that would be a bit counter productive. But cost of having full cache cleared on single |
Member
|
Yeah, I tried to handle this in the past, but it not works well as expected, as the cache hit will back and forth, see old PR: |
Member
|
In that case, I'll go with this approach, seems more intuitive. |
Member
|
Thank you @SanderMuller 👍 |
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
3 participants
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.
Problem
A file that is clean under one rule is not necessarily clean under all rules. A run with
--only SomeRule(or--only-suffix) caches such files as unchanged, so the next full run silently skips their pending changes until--clear-cache.Repro on
main:rector process --dry-run --clear-cache --only "Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector"on a file with an always-true if → clean under that rule, file gets cachedrector process --dry-run→ reports[OK] Rector is done!instead of the pendingRemoveAlwaysTrueIfConditionRectorchangeFix
Skip the unchanged-files cache write on selective runs. One guard in
ApplicationFileProcessor.Test
Unit test on
ApplicationFileProcessor::processFiles(): an--only/--only-suffixrun must leave the file uncached, a plain dry-run caches it (control test). The two guard tests fail onmain, pass with the fix.Split out of #8028 as a standalone pre-existing bugfix, as discussed with @TomasVotruba.
🤖 Generated with Claude Code