Track file dependencies for global constant fetches#6023
Open
SanderMuller wants to merge 3 commits into
Open
Conversation
DependencyResolver recorded dependencies for class constants, functions, methods and properties, but not for global/namespaced constant fetches (Node\Expr\ConstFetch). A file that used only a global constant declared elsewhere therefore recorded no dependency on it, so changing the constant did not re-analyse the consumer from the result cache, leaving a stale result (also surfaced by package-granular cache invalidation when the declaring package is bumped). Resolve the constant and record its declaring file, guarding the ubiquitous true/false/null literals so the common path stays cheap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
staabm
reviewed
Jul 8, 2026
The dependency edge added in the previous commit records that a file reads a global constant, but the result cache only re-analyses a changed file's dependents when that file's exported nodes change, and global constant declarations were not exported nodes. So editing a global constant still left the reading files with stale cached results. Export global constant declarations (name + value) like class constants already are, so renaming or changing a constant re-analyses the files that use it. Covered by a result-cache e2e test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
|
This pull request has been marked as ready for review. |
staabm
reviewed
Jul 9, 2026
The e2e test in the previous commit exercises the full result-cache flow, so the AnalyserTest case that asserted the raw dependency edge is redundant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
DependencyResolverrecorded file dependencies for class constants, functions, methods and properties, but not for global/namespaced constant fetches (Node\Expr\ConstFetch). On top of that, global constant declarations were not exported nodes. The result cache re-analyses a changed file's dependents only when the changed file's exported nodes change, so a file that reads a global constant was never re-analysed when that constant changed, leaving a stale result.This fixes both halves:
ConstFetchinDependencyResolver), guarding the ubiquitoustrue/false/nullliterals so the common path stays a couple of string comparisons.RootExportedNodes, like class constants already are, so renaming or changing a constant re-analyses the files that use it.Two ways to hit the stale result, both fixed and covered by the
result-cache-constantse2e test:Performance
The
ConstFetchbranch runs for every constant fetch, so it is guarded. Measured over a cold, single-process analysis:ConstFetchnodestrue/false/null)getConstant()callsAdded time is under 0.02% of analysis CPU, and that figure is an upper bound (it includes the measurement probes). The exported-node part runs only for global
constdeclarations, which are rare.Reproducing the staleness
Before this change the second run keeps
b.phpcached and reports nothing; a cold run reports thatf()returnsstring. With the change the second run re-analysesb.phpand matches the cold run.