Skip to content

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

Merged
ondrejmirtes merged 90 commits into
2.3.xfrom
resolve-type-rewrite-2
Sep 2, 2026
Merged

Single-pass expression analysis groundwork - answer type questions from ExpressionResults#5857
ondrejmirtes merged 90 commits into
2.3.xfrom
resolve-type-rewrite-2

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Jun 12, 2026

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#14396
Closes phpstan/phpstan#11953
Closes phpstan/phpstan#12780

🤖 Generated with Claude Code

Closes phpstan/phpstan#14999
Closes phpstan/phpstan#13334

Closes phpstan/phpstan#15004

return $this->withFlavor(false);
}

private function withFlavor(bool $fiber): self

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 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
ondrejmirtes referenced this pull request Jul 23, 2026
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 27 commits September 2, 2026 15:14
processArgs() captures an ExpressionResult for every argument it walks, so
the readers of ArgsResult no longer decide per call site whether one might
be missing:

* $argResults is a required constructor parameter.
* The nullable accessor is renamed to findArgResult() and documented as a
  membership query - "is this expression one of the call's arguments?" - for
  the two call sites that ask about arbitrary expressions (the callee name
  and synthetic nodes in FuncCallHandler's getType bridge, the narrowed
  subexpressions of a SpecifiedTypes entry in ImpossibleCheckTypeHelper).
  Everything holding an actual argument uses requireArgResult().
* DynamicReturnTypeStoragePrimer no longer takes an argument list and looks
  each argument up: it primes the captured results themselves, keyed by the
  very expression each was processed for. The list it used to receive was
  the normalized one, which contains arguments that were never processed -
  the default-value arguments ArgumentsNormalizer synthesizes for omitted
  optional parameters, and arguments an invalid call's normalization drops
  (a duplicate named argument overwriting a positional one).
* The array_key_first()/array_key_last()/array_find_key() narrowing reads
  the normalized argument, which is the one processArgs() walked.
* processArgs()'s own side-effects branch reads the captured result through
  a require, instead of falling back to a storage lookup that never fired
  (0 misses in 4790 lookups analysing src/, 837 analysing vendor/).
CalledMethodProcessor decided whether an execution end evaluates to an
explicit never by reading the statement's expression through the
may-or-may-not-be-stored lookup. The node now carries the very
ExpressionResult the expression was processed into, so the read is the
result's own type on the end scope.

The result is null exactly when there is no processed expression behind the
end: the statement is not an expression statement, or it is the synthetic
statement wrapping a closure with an empty body. For every real expression
statement the result is there - 9050 ends across the test suite and src/,
none missing.
The either-branch union recovery and the disjunction-holder projection pin
their target as tracked (hasExpressionType() yes) before reading its type,
so readScopeStateOrSyntheticType()'s on-demand walk was unreachable there -
the call sites read as undecided about whether the expression was analysed
while in fact they had just decided it. They now read through
requireScopeStateType(), which answers from the scope state and throws
instead of silently walking.

The targets themselves have no ExpressionResult to read: the projection
discovers them from the conditional holders registered on the applying
scope, not from a walk of its own.
The boolean-decomposition recipe read each condition subject's current type
by asking the applying scope for its state. It now reads it through the
result of the operand walk that produced the narrowing, captured with the
rest of the entry: the result answers from the applying scope's state where
that scope owns the expression, and consults the expression-type-resolver
extensions the plain state read skipped.

A subject no walk produced keeps the state read - a narrowing extension is
free to specify a type for an expression the source never evaluated on its
own, which is 114 of 30148 condition entries analysing src/.

Error sets over src/ and tests/PHPStan/Analyser are identical before and
after, and the nsrt type assertions are unchanged.
The single-pass rewrite moved the void->null projection to the value-read
boundary of every ExpressionResult, which widened it: a `void`-typed
parameter, a call through a callable value (`$f()`), and the native-type
flavour of every call all started evaluating to `null`. That silently
changed 26 test expectations - `assertType('void', ...)`, "expects int,
void given", "not subtype of native type void" - all of which are restored
here.

The projection is back to where it was before the rewrite: the phpdoc-type
flavour of a call to a resolved function or method. Both call handlers
short-circuited the native flavour before reaching the transformer, and
only the named-function and method paths ever reached it, so a dynamic
callee and every non-call expression kept void. Rules that flag a void
value being used read that type, so widening the projection had made them
silent.

The raw type an ExpressionResult carries still keeps void, which is what
getKeepVoidType() answers from.
MutatingScope::getType() resolves the late-resolvable types of whatever it
returns. The three reads that go to the tracked expression holder directly,
to avoid re-entering that method, skipped the resolution - so an expression
whose holder still carries a conditional type was answered with the raw
conditional.

A narrowed one made it visible: intersecting the asserted type into
`($success is true ? T : ErrorPayload)` gave
`($success is true ? MessagePayload : ErrorPayload)&MessagePayload` where
the conditional resolves to `MessagePayload|ErrorPayload` and the whole
intersection to `MessagePayload`.
simple-downgrade cannot rewrite a named argument whose receiver it does not
resolve - a private method on $this, and the two container-chain calls - so
these three reached the 7.4 lint job as named arguments and failed to parse.
The parameter before $storage is a bool defaulting to false in all three
signatures; spelling it out keeps the call positional.
findScopeStateType() read a variable by name, and otherwise read the tracked
expression - but it excluded every Variable from that second branch, so a
variable whose name is an expression ($$name) matched neither and the method
answered null. Its callers had already pinned the expression as tracked, so
requireScopeStateType() turned that into a ShouldNotHappenException:

  Internal error: PhpParser\Node\Expr\Variable on line 426 is not tracked on
  the scope it was pinned as tracked on.

which crashed the analysis of briannesbitt/Carbon. Only the by-name read has
to skip such a variable; the scope tracks it like any other expression.
getKeepVoidType() answered from the result's own raw type, which skips the
holder tracked for the expression - so a match arm body narrowed by that
arm's own condition was read at its declared type. A property fetch lost
the narrowing (a local variable did not, its own type reads the scope):

    match (true) {
        $this->shipment instanceof Shipment => $this->shipment,
        $this->shipment instanceof ReturnShipment => throw ...,
    }

gave BaseShipment instead of Shipment, so the match's return type no longer
satisfied the declared Shipment. The raw type is still the answer whenever
it carries void - that is the whole point of this read - but with no void to
keep it is an ordinary value read and goes through getType().
A multi-condition arm subtracts its conditions from the subject through a
synthetic in_array() whose haystack was built from the arm's own condition
nodes. The falsey narrowing of that call is evaluated on the scope carrying
each condition's own falsey narrowing, and `$subject === $cond` specifies
both sides - so on that scope a condition node is itself narrowed to never.
Re-pricing the haystack there collapsed every condition the subject had been
narrowed down to, and the arm stopped subtracting them:

    match ($this->get()) {          // ?E
        E::A, E::B => true,
        null, E::C, E::D => false,  // haystack: array{null, E::C, *NEVER*}
    };

reported E::D as unhandled. The haystack now carries the conditions' walked
types, which are what it always meant, and cannot be re-narrowed by the
scope it is read on. Pricing it on the arm's entry scope instead was the
other candidate and is wrong - it over-subtracts (bug-10128).
processArgs() in consume mode reuses a closure argument's stored result
instead of re-walking its body, so an on-demand re-walk of the enclosing
call does not re-run the closure's by-ref convergence; on a store miss it
priced the closure through getClosureType() with no body walk at all.

The nullsafe call's plain-twin walk runs in that mode too, and it is the
FIRST and only walk of the call's arguments - nothing has stored them, so
the miss branch always fired and a closure passed to `$x?->foo(fn...)`
never had its body walked: no Closure node callback, no InClosureNode,
no rule saw it. shipmonk/phpstan-rules' ForbidCheckedExceptionInCallableRule
stopped reporting for exactly such calls.

A miss in consume mode now means "walk it"; only the on-demand re-walk mode
keeps pricing.
A truthy `$a?->b()?->c()` did not short-circuit, so the source reads it as
the plain `$a->b()->c()` once every link is known non-null. The nullsafe
handlers keyed the one-level twin (`$a?->b()->c()`) and the chain's own
key, and for a two-link chain the one-level twin IS the fully plain chain -
so only chains of three or more links lost the narrowing:

    if (!$refund->getTransactionCapture()?->getTransaction()?->getOrder()) {
        return;
    }
    $refund->getTransactionCapture()->getTransaction()->getOrder(); // Order|null

reported by shopware/shopware. The twin is keyed through the create path so
the impure gate applies to it exactly as to the chain (an impure call's
value is never remembered, plain or nullsafe - bug-4757), and only in the
truthy direction: a falsey chain may have short-circuited.
A function carrying @phpstan-assert narrowed its arguments AND keyed its
own truthiness unconditionally; only the assert-less default narrowing
went through the purity gate. So a second evaluation of an impure
function read the first one's remembered truthiness:

    if (realpath($p) ?: null) {
        realpath($p) ?: ''; // "Ternary operator condition is always true"
    }

reported by efabrica-team/phpstan-latte. realpath() is impure and carries
`@phpstan-assert-if-true =non-empty-string $path`. The asserts still narrow
the arguments; the call's own key gets the same purity gate as the default
narrowing - what the old specifyTypesInCondition() reached through create().
A for loop narrows its post-loop scope to the condition's falsey branch.
The branch read that narrowing off the condition's stored result, which
was walked BEFORE generalizeWith() widened the counter - so for a nested
loop whose counter is seeded from the enclosing counter the verdict was
stale: `$k <= $d` with the literal `$k = 0` and `$d = 0` reads as
always-true, its falsey branch is unreachable, and every operand was
narrowed to never. That never killed the enclosing loop's `$d++`, the
enclosing counter never widened, and the inner counter stayed literal on
every pass:

    for ($d = 0; $d <= $max; $d++) {
        for ($k = -$d; $k <= $d; $k += 2) { ... }   // $k: 0, should be int
    }

reported by nikic/PHP-Parser (Differ::calculateTrace(): "comparison always
true", "strict comparison between *NEVER* and 0", "unreachable statement").

The condition is now re-priced on the generalized exit scope, exactly as
WhileHandler already does. The myers-diff-loop-widening fixture had been
rewritten by the branch to expect int<0, max> for values that 2.2.x (and
the fixture's own docblock) give as (float|int) - it pinned this bug and is
restored to 2.2.x's expectations.
The scope-read fallback was documented as serving rule-facing bridge asks
whose scope carries no storage. A census over the full suite and a
self-analysis found every ask answered from the stored result (139 hits,
0 fallbacks) - the mode argument is always processed with the call. The
fallback is dead and a miss is now an invariant violation.

Of the three remaining Scope::getType() reads in the handler helpers, this
was the only one whose fallback never fires. ClosureTypeResolver's
readExprType() falls back only from MutatingScope::getType() (the
rule-facing bridge, 390 asks in the suite, none from a walk), and its
immediately-invoked-closure argument read is an ordering seam: those
arguments are walked after the closure they are passed to.
isset()/empty()/?? ensure every link of their operand non-null ahead of the
operand's walk, reading each link's current type from the scope's state.
resolveScopeStateType() had state arms for a property fetch and an
argument-less method call but none for their nullsafe forms, so a ?-> link
fell through to a getType() walk of a node that was not processed yet.

The nullsafe arms answer like the plain ones - reflection on the receiver's
state - with the short-circuit null added. Found by PHPSTAN_GUARD_NW=1
(4 of its 64 remaining violations).
PHPSTAN_GUARD_NW=1 flags every getType() on a node the walk has not stored
yet. This round closes the rule-side sites it found (64 -> 38 violations):

* Impossible-check rules read an argument through the call's ArgsResult,
  carried on the call's ExpressionResult and on the FunctionCall/MethodCall/
  StaticMethodCall expression nodes. A narrowed variable an argument ASSIGNS
  (`is_string($data = json_encode($data))`, chained too) is answered from
  the assignment result's own type - a re-pricing of the variable on the
  callback scope is exactly the read the guard forbids. The
  doNotTreatPhpDocTypesAsCertain() re-check passes the same ArgsResult.
* The @var-changed-type node is emitted AFTER the statement handler walked
  the expression (return / throw), on the scope from before the tag re-typed
  it - the rule compares the tag against the walked type. The type gate that
  used to read the expression ahead of its walk is gone: the rule decides.
* A static variable's constant default and compact()'s literal names are
  position-independent and priced through the initializer resolver;
  a variable-variable's name reads scope state by name.
* declare() values are processed before their callback fires.

The count() mode read and the closure-argument walk of a nullsafe call's
twin landed separately (4bf2a51, 9921952, 49d7f80).
A string-named variable read answers from scope state and a literal is a
constant - neither needs the node walked, so a rule asking about them
ahead of the walk (an assign-op target, a $$name name, a list key) is not
the on-demand pricing the guard exists to catch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
An immediately invoked closure's or a pipe's literal operand is walked
only after the callee whose parameters it types. A literal is
position-independent, so the scope prices it without a walk instead of
processing the node ahead of its turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
InvalidKeyInArrayItemRule reads the key's type from the item node; firing
the callback after the key walk lets it consume the stored key result
instead of pricing the node ahead of its walk - the same order the
literal-array handler uses.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
PHP hands out the spl_object_id() of a freed object again. The file's own
AST nodes live for the whole analysis, but a synthetic node built and
dropped mid-file frees its id, and the next node allocated may get it:
ClosureTypeResolver then answered an arrow function's cached generator
return type for an unrelated closure sharing the id
(ExpressionResultTest #23 under the paratest wrapper, where the arrow
function of an earlier data set had been freed). Each entry now pins the
node it was built for and answers for that very node only; the ternary
and match capture maps keyed the same way get the same check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
…etion

isset()/empty()/?? ensure the links of their operand's chain non-null
before walking it. A link the scope cannot price from its state - a call
with arguments, a ternary, a fetch spine over one - had no type ahead of
the walk, so the ensure priced the node on demand: a second walk of a
real node, and the guard's last non-nullability site.

Such a link is now registered as pending on the ensure frame and deviced
when its own walk completes, from the type that walk produced: the
result's after-scope tracks it as non-null and its value is pinned to the
ensured type, which is what a link walked on an ensured-ahead scope
answered. The late device joins the frame's originals for the nullsafe
handlers and is reverted with the frame. Links the scope does price from
state (variables, fetch spines, argument-less instance calls, constants)
keep the ahead-of-walk device; receivers that are never null (new, array
and closure literals, scalars) skip the ensure altogether.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
An immediately invoked closure's untyped parameters take the types of
its invocation arguments, which the closure type resolver read from the
scope ahead of their walk - the last processSyntheticOnDemand() seam
under the new-world guard (and, for the pipe operator, the callee was
evaluated before its operand, unlike PHP).

The call handler now walks the arguments first, on the closure's
declared signature (ClosureTypeResolver::getDeclaredClosureType(), the
signature without invocation inference or a body walk), then the
closure, whose parameter inference consumes the stored argument results;
the call resolves from the walked closure's acceptor over the same
processed arguments (ArgsResult::withResolvedParametersAcceptor()). The
closure is thereby walked on the post-argument scope: a by-value use of
a variable an argument assigns sees the assigned value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
`$x = $this->foo(function () use (&$x) {...})` types the by-ref use from
the call being assigned - a forward reference read from inside the call's
own arguments, which priced the enclosing call on demand (a nested walk
of the closure itself). The call handlers now hand the context the
declared return type of the acceptor the call was normalized with
(template types resolved to their bounds) before processing the
arguments, and the by-ref use reads it; a closure right side still
resolves through the closure type resolver.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
…e read

resolveScopeStateType() prices an argument-less instance call from the
declared return type of its method - the shape @phpstan-assert subjects
take, synthetic nodes never stored. The arm also caught calls the walk
had processed and stored, and the declared type lacks what the walk
resolved against the arguments: a conditional return type stays a
conditional (`($asResource is true ? resource : string)` narrowed truthy
became `resource|non-falsy-string` instead of `non-falsy-string`, and
getKeepVoidType() no longer saw the `void` a `($callback is null ? void
: TReturn)` call resolves to, losing the method.void report), a template
stays a template, and on the PHP 7.4 downgrade a native `self|false`
union resolved its `false` as a class in the narrowing base.

A call with a stored result now answers through getType(), i.e. from
that result; the reflection route stays for the synthetic subjects.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
askScopeVariableStateMatches() traversed the expression's whole subtree
per result - O(depth) per chain link, recreated every loop-convergence
pass. On tests/bench/data/nullsafe-chain-walk.php the traversal was 43%
of the run (NodeTraverser::traverseNode 22x the self-cost of 2.2.x).

The names are pure syntax, so they cache on the node as an attribute
(sharing the node's lifetime) and each link composes its set from its
child's cached set in O(1) amortized. Restores a27ece6, dropped with
the consume/splice arc removal; the read set keeps the current semantics
(variables only, $this excluded, closure use() names only).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R7pqAkCx4xP6WYxBo2nMJE
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from 2aca59b to 00f2b74 Compare September 2, 2026 13:15
@ondrejmirtes
ondrejmirtes merged commit a4793f7 into 2.3.x Sep 2, 2026
780 of 790 checks passed
@ondrejmirtes
ondrejmirtes deleted the resolve-type-rewrite-2 branch September 2, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment