Repository navigation
Re-analyse files with errors of rules on collected data when a symbol appears - #6896
Merged
Merged
Conversation
… appears Rules on CollectedDataNode run in AnalyserResultFinalizer, and their errors are not in the cached error map: they re-run on the merged collected data on every run, so a cached copy would be reported twice. That map is what restore() sweeps when a new symbol appears - the recovery for an error that could record no dependency because the symbol it is about did not exist yet. A file whose only error came from collected data was never swept, so it kept the error after the symbol arrived, while a run with a cleared cache reported nothing. The rule results stored for reuseCollectedDataRuleErrors do not cover this: they are only stored with that toggle on. The cache now keeps the files with errors of these rules, taken from the finalizer result on every save regardless of the toggle, and restore() treats them like the files in the error map. Amp-Thread-ID: https://ampcode.com/threads/T-01a19695-f1c1-75fa-ac1f-125f79f272c7
ondrejmirtes
force-pushed
the
fix-collector-sweep
branch
from
November 1, 2026 10:29
591cdb4 to
b93eb8a
Compare
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.
Summary
When a new symbol appears,
ResultCacheManager::restore()re-analyses the files in the cached error map. That's how the cache recovers an error that could record no dependency, because the symbol it is about did not exist yet. Errors of rules onCollectedDataNodeare never in that map: those rules re-run on the merged collected data every time, so a cached copy would be reported twice. A file whose only error came from such a rule was never re-analysed. It kept the error after the symbol arrived, while a run with a cleared cache reported nothing.The
collectedDataRuleResultsalready saved after the finalizer don't cover this, because they're only stored with thereuseCollectedDataRuleErrorstoggle (bleeding edge). Without the toggle, the finalizer deliberately behaves as before.The fix
filesWithCollectedDataRuleErrorslist. It's built fromFinalizerResult::getCollectorErrors()on every save, whatever the toggle.restore()treats those files like files in the error map. They're re-analysed when a symbol appears, and they count towards the "every file with errors changed anyway" shortcut.ResultCacheProcessResult::save()takes theFinalizerResultinstead of only the rule results.CACHE_VERSIONis bumped.Test
This includes the E2E fixture from #6358 (
e2e/result-cache-collector-sweep). Before this change the second run reportedCaller.php:7 function.resultUnusedand exited 1. Now it re-analysesCaller.phpand exits 0. Deleting the new file brings both errors back. I also checked the same scenario with bleeding edge: the affected rule re-runs and the others are reused. Warm no-op runs still skip rewriting the cache.Closes phpstan/phpstan#15162
Supersedes #6358