Return dependencies in the results of the handlers - #6662
Merged
Merged
Conversation
ondrejmirtes
force-pushed
the
dependency-tracker-scenarios
branch
from
October 2, 2026 15:29
6f58af6 to
2c7c9cf
Compare
ExprPrinter::printExpr() remembers the printed form of every expression on its node, a multi-line one too, and Printer::p() reused it when the same node was printed nested in another expression, where it needs the indentation of that level. The printed key then depended on which expression was printed first, which differs with the turbo extension loaded: a remembered possibly impure call with a closure in a closure in its arguments was a different expression in the two. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ
ondrejmirtes
force-pushed
the
dependency-recording-in-handlers
branch
from
October 2, 2026 15:40
11f05d9 to
6eab5a4
Compare
DependencyResolver looked at every node the rules saw and resolved again what the walk had just resolved: the method a call calls, the type of the expression, the class a declaration extends. Now each handler puts what it depends on into the ExpressionResult or InternalStatementResult it returns, together with what the results of its parts depend on, like impure points and variable flow. NodeScopeResolver::processFileNodes() returns what the whole file depends on and DependencyResolver turns that into file and package dependencies once per file. Exported nodes are resolved per node as before. Dependencies is an immutable value. Merging only points to the merged values, the tree is walked once, at the end of the file. Loop handlers keep only the dependencies of their final pass. Nodes that a rule passes to the node callback through NodeCallbackInvoker are not walked by the handlers, so their dependencies are no longer collected. Dependencies is shadowed natively and the native twins return the dependencies the same way. Compared with DependencyResolver on phpstan-src and WordPress, the dependencies are the same except for stdClass, the declaring class of a dummy method on mixed, which shows up or goes away in a few files. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ
ondrejmirtes
force-pushed
the
dependency-recording-in-handlers
branch
from
October 2, 2026 20:47
6eab5a4 to
4cf9d34
Compare
ondrejmirtes
changed the title
… that can have one FileAnalyserCallback called resolveExportedNode() for every node the walk reported. Now it asks once per node class whether a node of the class can be exported or change the PHPDoc name scope, and skips the call for the rest. resolveFileDependencies() now expands each class found in a file to its ancestors once, and reads the referenced classes of each type object once. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ
DirectoryResultCacheValueExtensionTest built the key from __DIR__ . '/data', which a round trip through the result cache turns into backslashes on Windows. ValueDependencyCollector normalizes the directory before it creates the key, so the test now does that too. ValueDependencyCollectorTest counted all calls of the extension, which the container shares with the other tests in the class. It now counts the calls the test itself makes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ
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
1 participant
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.
Follows #6659, which is merged now.
DependencyResolver looked at every node the rules saw and resolved again what the walk had already resolved: the method a call calls, the type of the expression, the class a declaration extends. This PR moves that into the handlers. Each handler puts what it depends on into the
ExpressionResultorInternalStatementResultit returns, together with what the results of its parts depend on, the same way impure points and variable flow are carried.NodeScopeResolver::processFileNodes()returns what the whole file depends on, andDependencyResolver::resolveFileDependencies()turns that into file and package dependencies once per file. Exported nodes are resolved per node as before.Dependenciesis an immutable value. Merging only points to the merged values, and the tree is walked once at the end of the file.NodeCallbackInvokerare not walked by the handlers, so their dependencies are no longer collected.Dependenciesis shadowed natively and the native twins return the dependencies the same way.The first commit fixes a printer bug that this change exposed in the walk-trace check.
Printer::p()reused a multi-line form thatExprPrinter::printExpr()had cached at the top level, so an expression key depended on which expression was printed first. That order differs with turbo loaded. It shows up with a remembered possibly impure call that has a closure in a closure in its arguments.The last commit makes FileAnalyserCallback ask DependencyResolver about exported nodes only for the node classes that can have one, instead of calling it for every node. It also makes
resolveFileDependencies()expand each class to its ancestors once per file.Same dependencies as before
The per-file results (files, used-trait files, packages and exported nodes) were compared with 2.3.x (ed978d4):
jetbrains/phpstorm-stubsinExportedInterfaceNode.php. It came fromstdClass, the declaring class of a dummy method on mixed.Reported errors are the same as on 2.3.x in all three projects.
Benchmark
phpstan-src
src, turbo on, result cache cleared, one process (--debug), instructions retired (stable to about 0.05G between runs):User CPU with parallel workers is too noisy on this machine to show the difference reliably. Without turbo the change is within noise.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ