Repository navigation
Reuse the errors of rules on CollectedDataNode from the result cache - #6840
SanderMuller wants to merge 3 commits into
Conversation
b36b4f2 to
b6b5518
Compare
|
This is nice but I worry it's also a BC break. Users before expect that CollectedDataNode rules always run. A few ideas:
|
ResultCacheManager::process() now returns the save as a callback, and both callers call it after AnalyserResultFinalizer::finalize(). Nothing else changes: the same data is saved. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CollectedDataNode records which collectors a rule reads and whether it reads isOnlyFilesAnalysis(). The result cache stores the errors of each rule with a hash of that input. On the next run, a rule whose input is the same returns its stored errors and does not run again. A rule can declare what else it reads with the DependencyTracker methods, for example trackFileDependency() or trackDirectoryDependency(). These values are stored with its errors, and the rule runs again when one of them changes. The ignore extensions and the local ignores still run on every error, also on reused errors. This is behind the reuseCollectedDataRuleErrors feature toggle, which is on in bleeding edge only. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
b6b5518 to
c96f229
Compare
|
Thanks, I did 1 and 2:
One gap is left: values that a class reflection extension tracks with For 3, I will open issues on larastan, dead-code-detector and the other extensions with collectors after 2.3.3 is out. |
Yeah, these should all work |
A class reflection extension records with DeclarationDependencyTracker what a class declares depends on. A rule on CollectedDataNode can read such a class, but it is not known which classes it read. An extension also describes a class only once per process, so a second rule that reads the class records nothing. So a rule now gets the values of every class seen in the process. A changed value runs the rule again. A value of a class that the rule did not read causes an extra run only. Every rule now has the same values, so the finalizer asks for each current value once per run, not once per rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Done in c7b1469. A rule does not say which classes it read. So each rule on The tracking E2E test now has a class reflection extension that describes a class from a file that is not analysed. When the test empties that file, the rule runs again and reports the view. Without this commit, the rule is reused and the error is missing. |
This follows the design from phpstan/phpstan#15447 (comment).
The rules on
CollectedDataNodenow run again only when their input changed.CollectedDataNoderecords which collectors a rule reads withget(), and whether it callsisOnlyFilesAnalysis(). The result cache stores the errors of each rule with a hash of that data. On the next run, a rule with the same input gets its stored errors back.The ignore extensions and the local ignores still run on every error, also on reused ones. A rule that throws is not stored, so it runs every time. Each rule gets its own
CollectedDataNode, so a service that memoizes per node cannot hide a read from the next rule.To store the rule results, the result cache is now saved after the finalizer ran. That is the first commit, and it is not behind the toggle.
PHPStan self-analysis with dead-code-detector (
build/phpstan.neon, 13 errors in both arms):Warm rows are the median of 5 rounds, cold of 9 rounds in both orders. The result cache grows from 39.4 MB to 39.7 MB.
I measured the table on an earlier version of this PR and did not measure it again.
build/phpstan.neonincludes bleeding edge, and no extension in it usesDeclarationDependencyTracker.A rule can also depend on data the node does not see. larastan's
UnusedViewsRulereads the Blade view files from disk. Such a rule declares that data with theDependencyTrackermethods, for exampletrackDirectoryDependency(). The values are stored with its errors, and the rule runs again when one of them changes. Before this PR, these methods did nothing in a rule onCollectedDataNode.Values that a class reflection extension tracks with
DeclarationDependencyTrackerreach these rules too. A rule does not say which classes it read, so each rule depends on the values of every class that the process saw. When one of these values changes, a rule that did not read that class can also run again.The reuse is behind the
reuseCollectedDataRuleErrorsfeature toggle, which is on in bleeding edge only. Without it, every rule runs on every run, as before.This overlaps with #6373, which loads the cached collected data only when the rules run. This PR hashes the collected data on every warm run, so it reads it every time.
Refs phpstan/phpstan#15447
🤖 Generated with Claude Code