Skip to content

Single-pass expression analysis groundwork - answer type questions from ExpressionResults - #5857

Open
ondrejmirtes wants to merge 394 commits into
2.2.xfrom
resolve-type-rewrite-2
Open

Single-pass expression analysis groundwork - answer type questions from ExpressionResults#5857
ondrejmirtes wants to merge 394 commits into
2.2.xfrom
resolve-type-rewrite-2

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Groundwork for the "new world" where an expression is traversed once: after processExpr, its ExpressionResult knows the before/after scopes, the type (typeCallback) and the narrowing (specifyTypesCallback), composed from child results instead of re-walking subtrees. Handlers then stop implementing TypeResolvingExprHandler; the old entry points (MutatingScope::resolveType, the TypeSpecifier dispatcher) are guarded behind NewWorld::disableOldWorld() and get mass-deleted in PHPStan 3.0.

What's on the branch, bottom up:

  • Guards + ExpressionResultFactory: old-world type resolution entry points throw when NewWorld::disableOldWorld() is flipped (the migration meter); all ExpressionResult construction goes through a generated factory.
  • ExpressionResult carries beforeScope, expr, typeCallback, specifyTypesCallback and is stored per node in ExpressionResultStorage (layered O(1) duplicate()), replacing the stored before-Scope.
  • ExprHandler / TypeResolvingExprHandler split: resolveType/specifyTypes move to the sub-interface so handlers can shed them one by one.
  • ExpressionResultStorageStack: old-world consumers (TypeSpecifier dispatcher, extensions, rules below PHP 8.1, unconverted handlers' resolveType) keep working for converted handlers' nodes. Every scope shares the stack created by its internal scope factory; NodeScopeResolver pushes the storage of the analysis in progress through MutatingScope::pushExpressionResultStorage() (always popped in finally, throwing on imbalance), and MutatingScope answers from the stored result - or processes a synthetic node on demand. Scopes never reference a storage directly, so nothing pins the result graph with the cycle collector disabled in bin/phpstan. Also adds MutatingScope::applySpecifiedTypes - filterBySpecifiedTypes without Scope::getType().
  • First two migrations: ScalarHandler and ArrayHandler no longer implement TypeResolvingExprHandler. The array migration is a precision win the old world cannot reach: each item type is captured at its own evaluation point, so [$b = 1, $b + 1, $c = $b, $c + 2, $c++, $c] infers array{1, 2, 1, 3, 1, 2}.

Verified: full test suite green, make phpstan clean, and analysis memory back at baseline (no leak from the result graph despite gc_disable()).

Closes phpstan/phpstan#13944
Closes phpstan/phpstan#12207
Closes phpstan/phpstan#7155

Closes phpstan/phpstan#2032

Closes phpstan/phpstan#10786

Closes phpstan/phpstan#13253
Closes phpstan/phpstan#14396
Closes phpstan/phpstan#11953
Closes phpstan/phpstan#13802
Closes phpstan/phpstan#13789
Closes phpstan/phpstan#12780
Closes phpstan/phpstan#14914
Closes phpstan/phpstan#14908

🤖 Generated with Claude Code

@ondrejmirtes ondrejmirtes mentioned this pull request Jun 12, 2026
Closed
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 3 times, most recently from ebb19a0 to 457689b Compare June 12, 2026 17:15
staabm
staabm reviewed Jun 12, 2026
View reviewed changes
Comment thread src/Analyser/DirectInternalScopeFactory.php
return $this->withFlavor(false);
}

private function withFlavor(bool $fiber): self

@staabm staabm Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this read withFiber?

@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 2 times, most recently from eb31077 to 59cbf22 Compare June 19, 2026 11:44
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from 59cbf22 to 125cf22 Compare June 20, 2026 11:56
@ondrejmirtes ondrejmirtes mentioned this pull request Jun 25, 2026
Closed
@SanderMuller SanderMuller mentioned this pull request Jun 25, 2026
Merged
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 4 times, most recently from f98892f to 4455baa Compare July 6, 2026 22:20
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from 61fe06e to e38aadd Compare July 16, 2026 14:56
@marcella-galambos-framna marcella-galambos-framna mentioned this pull request Jul 16, 2026
Open
ondrejmirtes referenced this pull request Jul 23, 2026
@ondrejmirtes @claude
Add bench for the expression chain walk speedup from #6084
cdc6ca2 
Every property fetch / method call resolves its type by walking down to
the chain root to detect a nullsafe operator (NullsafeShortCircuitingHelper),
costing O(N²) walk steps per chain of depth N — with or without an actual
nullsafe operator in the chain. Deep loop-wrapped plain chains make that
walk dominate: 3.71s -> 3.14s wall (-15%), -18% user CPU from the
recursion-to-loop rewrite. The real-world counterpart is Symfony
TreeBuilder fluent chains (300+ calls in one statement) in Sylius bundle
Configuration classes, which dropped up to 23% per file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016szvNF5RXhACdfMQNc6DVL
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 12 times, most recently from fb22d34 to 84b1614 Compare July 28, 2026 17:31
ondrejmirtes and others added 29 commits July 28, 2026 20:26
@ondrejmirtes
Run engine-feeding node-callback gatherers synchronously at the emiss…
96f4624 
…ion position

Body walks pair a gathering closure (impure points, invalidations,
execution ends, return statements) with the rule-facing node callback and
read the gathered arrays as soon as the walk returns. With the whole
composite running inside a fiber, a rule parking on an unsettled
expression deferred the gathering past that read - a closure assigning
static::$prop lost its propertyAssign impure point whenever a rule
suspended on the PropertyAssignNode, so the invocation was reported as
having no side effects.

GatheringNodeCallback makes the pairing explicit: FiberNodeScopeResolver
unwraps it and runs the gatherer at the emission position, deferring only
the rule-facing remainder to a fiber.
@ondrejmirtes
Align the isset verdict gate with 2.2.x and pin null in falsey isset …
28d8f86 
…of native-typed properties

Three connected changes to the isset/empty/?? verdict machinery:

The maybe-uninitialized gate in IssetabilityResolution::isSet() drops the
nativeHasDefaultValue conjunct - the old-world MutatingScope::issetCheck()
never had it, only the rule side does (the two deliberately diverge
upstream). In its place the gate learns to read conditional-expression
entries about the fetch as initialization evidence: such entries exist
only when the fetch was narrowed in an evaluated condition, and evaluating
a condition reads the fetch, which would have thrown on an uninitialized
typed property. A typed-property read witnesses initialization.

IssetHandler evaluates its verdict and narrowing callbacks on the
post-revert scope instead of the scope before the subjects were processed:
revertNonNullability() leaves an originally-untracked nullable chain link
tracked at its original type, and that state is what the old world's
narrowing evaluation saw.

The falsey narrowing of a single isset() subject pins a native-typed
property to null even when the verdict is maybe, as long as the inner
chain is fully set: the maybe can only mean a nullable value or a possibly
uninitialized property, and reading an uninitialized typed property throws
instead of yielding a value - so in the !isset() branch any read that
completes yields null.
@ondrejmirtes
Store the assign-target property fetch's pre-assign result for parked…
0cf8570 
… askers

The raw target fetch of a property assignment is emitted to node callbacks
at the top of processAssignVar() (rules and DependencyResolver listen on
it and ask its type), but the assign flow never processed it as a read -
under the fiber engine those askers parked forever and were flushed
through the on-demand path, tripping the PHPSTAN_GUARD_NW diagnostic on
~320 suite tests. The property branches now price the target once at the
pre-assign position, consuming the receiver's and name's stored results;
storing it resumes the parked fibers with the pre-assign type.

The pending-fiber guard also learns the same processed-node check the
other three guards already have: a node whose per-body storage was
released since it was stored (class-level rules asking about gathered
method-body expressions) legitimately re-prices through the on-demand
rule bridge and must not be flagged.
@ondrejmirtes
Price closure parameter defaults without a scope walk
9d299b4 
A parameter default is a constant expression - resolve it through
InitializerExprTypeResolver like parameter defaults elsewhere, instead of
Scope::getType() on a node the single pass has not walked.
@ondrejmirtes
Create the function call's impure point after its arguments are proce…
dd94fe7 
…ssed

The side-effect flip parameters (print_r's $return, var_export's, ...)
read an argument's type to decide whether the call has side effects. At
the pre-args position that argument is not walked yet; after processArgs()
its result is stored and the read is answered from it.
@ondrejmirtes @claude
Restore the Array_ import dropped when replaying onto the ScopeOps ex…
4f28cc3 
…traction

Upstream removed the then-unused import from MutatingScope; the branch's
applySpecifiedTypes still guards array literals with instanceof Array_,
which silently matched the nonexistent PHPStan\Analyser\Array_ and let
array literals into the scope's tracked expressions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Update test expectations after the rebase onto 2.2.x
886f53c 
array-merge2: restore the key order upstream moved when getSortedTypes()
stopped sorting in place. preg_replace_callback: the flags-refined closure
parameter makes the arrow body's return type precise, so returning $m[0]
under PREG_OFFSET_CAPTURE is correctly reported as returning an array where
a string is required.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Remove the superseded ScopeOps invalidation helpers after the rebase
8603959 
The branch answers invalidation checks from the per-holder index of
contained node keys in MutatingScope, so the extracted
invalidateExpressionEntries/shouldInvalidateExpression/
containsExpressionToInvalidate/buildTypeSpecifications helpers have no
callers left. Also drop the baseline entries for the unused-use closure
errors that upstream's include/require handling (#6056) resolved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Answer invalidation checks from ScopeOps again
1622bd9 
The per-holder contained-node-keys invalidation lived as private
MutatingScope methods, out of reach of the turbo twin — with the extension
active, every one of the ~2.3M shouldInvalidateExpression() calls of a
self-analysis run paid a userland frame that 2.2.x answers natively.
Extracted back into ScopeOps statics (the shape the pre-index code had),
called unconditionally; '$this' invalidations keep the finder on the
asking scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Sync the turbo native twins with the single-pass engine's shadowed cl…
e07981a 
…asses

ExpressionResultStorage stores ExpressionResults in id-keyed tables with an
O(1) fallback-chain duplicate() and gains mergeResults(), mirroring the
twin's SplObjectStorage model. ExpressionTypeHolder gains the lazy
containedNodeKeys subtree index. ScopeOps: mergeVariableHolders reports
differing keys through the new by-ref parameter,
createConditionalExpressions iterates only those keys, nodeKey drops the
retired keep-void suffix, buildTypeSpecifications goes away with its PHP
twin, and shouldInvalidateExpression/invalidateExpressionEntries answer
from the holder's contained-node-keys index (built natively on first use
and cached in the holder's slot) instead of the subtree scan - measured
about -1.5% make phpstan user CPU against the 2.2.x-with-turbo control.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes
Pin turbo extension version to cac4327
5cc1a86
@ondrejmirtes @claude
Resolve the call return type once per call
49c7e55 
The call handlers' throw-point leg resolved the return type eagerly and
directly, outside the result's per-flavour memo - the first later type
read ran resolveReturnType() again (2.78 resolutions per method call on
the self-analysis corpus; dynamic return type extensions such as sprintf's
ran twice per call). The eager leg now reads through the stored
preliminary result, and the memoized value seeds the final result's
raw own-type slot via the new ExpressionResult $resolvedType parameter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Complete the stored preliminary call result in place instead of overw…
abbbe23 
…riting it

The four call handlers (function, method, static, new) stored a preliminary
result before the throw-point leg and then built a second ExpressionResult
with the resolved scope and effects, losing every memo the preliminary had
accumulated - including the eagerly resolved return type. finalize() turns
the stored preliminary into the final result in place, carrying the
own-type and narrowing memos and dropping only the truthy/falsey scopes
derived from the preliminary scope. The $resolvedType factory seed from
the previous commit is superseded and removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Walk closure by-ref convergence passes in deep statement context
7072851 
The closure by-ref fixpoint walked its intermediate passes with a top-level
statement context, so every inner loop re-converged from scratch on every
pass - convergence within convergence, which the loop handlers themselves
avoid by walking their passes with enterDeep(). A recursive by-ref closure
with nested loops re-created the innermost expressions' results up to 78
times; deep-context passes bring it in line with the loop handlers'
discipline, and the final top-level walk keeps full precision. Inherited
from mainline (2.2.x has the same top-level pass).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Drop unnecessary ?? null on always-set parameter types
d3c1799 
Rebase adaptation: the mainline restructuring of processArgs assigns
$parameterType on every path before the closure/arrow-function branches,
which the new nullCoalesce.unnecessary check now proves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5cXTK6VFMziQHez43qhea
@ondrejmirtes @claude
Drop the buildTypeSpecifications smoke block for the removed helper
eef3194 
Rebase adaptation: ScopeOps::buildTypeSpecifications() no longer exists on
this branch (superseded by the per-holder index), so the differential smoke
block resurrected from the mainline side tested a method with no PHP twin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5cXTK6VFMziQHez43qhea
@ondrejmirtes @claude
Fix use-statement order left by the rebase auto-merge
11a6f0e 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5cXTK6VFMziQHez43qhea
@ondrejmirtes @claude
Price an offset read on never as ErrorType in the narrowing-base reader
91ed434 
resolveScopeStateType() derives the base a narrowing is intersected into
from tracked state instead of re-walking. Its ArrayDimFetch case called
NeverType::getOffsetValueType() directly, yielding never - but genuine
pricing of an offset read on never yields ErrorType (a benevolent mixed).
A narrowing applied in a dead branch (is_object($x[0]) with $x already
never) intersected object against never and lost the narrowed type,
silencing every rule that reads it - e.g. the non-ignorable 'Accessing
::class constant on an expression is supported only on PHP 8.0 and later'
disappeared at phpVersion < 8.0 because ClassConstantRule bailed on the
ErrorType it derived from the never.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Avoid the nullsafe operator - simple-downgrader passes it through
a1e3289 
The downgrade step silently leaves ?-> untouched, so the transformed
sources kept PHP 8.0 syntax and would fail to parse on PHP 7.4. Rewritten
as explicit null checks on a hoisted receiver; the first-class-callable
case was already gone (the extraction of shouldInvalidateExpression into
ScopeOps replaced $this->getNodeKey(...) with an arrow function). Verified
by running simple-downgrade 7.4 on the tree: 17 nullsafe remnants in the
output before, zero after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes
Use ExtensionsCollection in ExpressionResult and tag PerFileAnalysisR…
698e5e1 
…esettable via attribute
@ondrejmirtes
Construct name checks from extensions collections in ClassConstantPhp…
6777b6f 
…74RuleTest
@ondrejmirtes
Bump expected turbo version
78d3480
@ondrejmirtes @claude
Restore the default-value gate in the isset verdict
0c28cc4 
2.2.x now consults hasDefaultValue() in MutatingScope::issetCheck, so
the gate-parity alignment that removed nativeHasDefaultValue() from the
isset verdict points the other way again: a property with a default
value is always set, so isset() on it is true. Also absorb the upstream
isset-property-default-value fixture: the no-default coalesce narrows
to int here because the === null reads in the guard prove both
properties are initialized.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes @claude
Complete the callable-union parameter merge in createCallableParameters
1a1dcfd 
The rebase kept the stale pre-c2c1497254 shape of the acceptor merge
loop: a longer acceptor's raw parameters (possibly ExtendedDummyParameter)
were stored and the next acceptor called union() on them. Port the
upstream form - wrap every acceptor's parameters in
NativeParameterReflection up front and mark length-mismatched positions
optional.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes
Bump expected turbo version
ea67c11
@ondrejmirtes @claude
Absorb the upstream TypeSpecifierTest additions after the rebase
a061278 
Drop the duplicated alternative-entry harness loop (both worlds added
one) and update the new rows to this branch's richer results: the
composed nullsafe narrowing also constrains the call expression itself,
and the disjunction falsey merge keeps its sure-first description
order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes @claude
Drop the upstream toSureTypes copy that flowed into the holder helper
274f320 
This branch's eager fold lives on DefaultNarrowingHelper; the copy the
rebase carried into ConditionalExpressionHolderHelper was dead and
referenced imports this file does not have.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes
Bump expected turbo version
8495201
@ondrejmirtes @claude
Drop the upstream duplicate of the differingKeys smoke block
a834c78 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from dc0e107 to a834c78 Compare July 28, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment