Single-pass expression analysis groundwork - answer type questions from ExpressionResults#5857
Open
ondrejmirtes wants to merge 22 commits into
Open
Single-pass expression analysis groundwork - answer type questions from ExpressionResults#5857ondrejmirtes wants to merge 22 commits into
ondrejmirtes wants to merge 22 commits into
Conversation
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
2 times, most recently
from
c5de212 to
ebb19a0
Compare
Not all ExprHandlers will be TypeResolvingExprHandler coming into the future. Instead of resolveType+specifyTypes, they will pass callbacks into ExpressionResult doing similar job.
…esults Old-world consumers (TypeSpecifier dispatcher, extensions, rules below PHP 8.1, unconverted handlers' resolveType) keep working for nodes whose handler no longer implements TypeResolvingExprHandler: every scope shares the ExpressionResultStorageStack created by its internal scope factory, NodeScopeResolver pushes the storage of the analysis in progress through MutatingScope::pushExpressionResultStorage() (always popped in finally), and MutatingScope resolves such nodes from the stored result - or by processing a synthetic node on demand. Also adds MutatingScope::applySpecifiedTypes (filterBySpecifiedTypes without Scope::getType()) and the specifyTypesCallback slot on ExpressionResult consulted by getTruthyScope()/getFalseyScope(). The cycle collector is disabled in bin/phpstan - scopes deliberately never reference a storage directly, only the stack. Popping severs the stack->storage edge when an analysis ends, so retained scopes do not pin the whole result graph. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each item type is captured at its own evaluation point in the sequence,
so [$b = 1, $b + 1, $c = $b, $c + 2, $c++, $c] infers
array{1, 2, 1, 3, 1, 2} - the old world resolves all items on a single
scope and cannot do this.
Until the item handlers (BinaryOp, inc/dec, Assign) migrate themselves,
the items resolve as their own results' before-scope evaluation instead
of cascading getTypeForScope(). Narrowing stays on the fallback path -
it is identical to the removed specifyDefaultTypes body.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An unbalanced push/pop is the one way the ambient storage design can still be misused - fail immediately instead of silently answering later type questions from the wrong storage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
from
ebb19a0 to
457689b
Compare
…sionResults Both handlers stop implementing TypeResolvingExprHandler and wire typeCallback + specifyTypesCallback, so their truthy/falsey scopes flow through MutatingScope::applySpecifiedTypes - the first real exercise of the new-world narrowing machinery. The old-world TypeSpecifier dispatcher answers nodes of converted handlers from the stored ExpressionResult (MutatingScope::specifyTypesOfNewWorldHandlerNode), processes synthetic nodes on demand (the 'foo' === $a::class rewrite builds synthetic Instanceof_ nodes), and falls back to specifyDefaultTypes when the result carries no callback - which is exactly what such handlers used to implement. Exercising the machinery surfaced two gaps in applySpecifiedTypes: - Expressions not tracked in the scope lost their sureNot narrowing ($var->name instanceof Identifier ? ... : ... stopped narrowing the else branch). The current type to intersect with or subtract from is now priced from the stored ExpressionResult (getCurrentTypesOfSpecifiedExpr) instead of Scope::getType(). - Only Yes-certainty holders hold the current type of an expression. A Maybe-certainty holder holds the when-defined type (falsy after an or-merge in nsrt/bug-pr-339.php), which the certainty-aware Scope::getType() never returned - narrowing against it produced NeverType and mis-fired conditional expression holders. AssignHandler's placeholder result for an assignment target now carries VariableHandler::createTypeCallback - every stored result for a converted handler's node type must answer type questions itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes phpstan/phpstan#13944 Closes phpstan/phpstan#12207 Closes phpstan/phpstan#7155 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes phpstan/phpstan#2032 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
staabm
reviewed
Jun 12, 2026
| return $this->withFlavor(false); | ||
| } | ||
|
|
||
| private function withFlavor(bool $fiber): self |
Contributor
There was a problem hiding this comment.
should this read withFiber?
A sure specification (e.g. is_string($a)) can only hold for a defined variable, so it still makes the variable defined inside the branch - one error at the test site, no cascade. Removing a type from a certainly-undefined variable proves nothing about its definedness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…:create() How a type constraint on a node translates into narrowing entries is the producing handler's knowledge, declared on its ExpressionResult - never re-derived by unwrapping the AST elsewhere. DefaultNarrowingHelper (recreated from the first rewrite attempt) carries the default boolean-context narrowing (one sureNot entry, no type ask, no nullsafe chain-walking) and createSubjectTypes(): ask the subject result's createTypesCallback, fall back to a single sure/sureNot entry for the subject node. No purity gates, no structural unwrapping. AssignHandler fans a type constraint out to the assigned variable and the assigned expression - nested assignments compose through the assigned expression's own result, which is what will delete unwrapAssign. CoalesceHandler delegates to its left side when the type rules the right side in or out, so ($e ?? null) instanceof Foo narrows $e. AssignHandler also wires specifyTypesCallback: the assigned variable narrows by the boolean outcome, plus the $arr[$key] inference after $key = array_key_first/array_key_last/array_search/array_find_key. The null-context inferences stay in the old-world specifyTypes() - result-based asks are always truthy or falsey. specifyTypesCallbacks no longer touch TypeSpecifier: VariableHandler uses DefaultNarrowingHelper, InstanceofHandler narrows its subject through createSubjectTypes(). TypeSpecifierTest's assign-in-instanceof expectations hold unchanged - the new channel reproduces create()'s emission exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ingExprHandler The composite handlers wire typeCallback and specifyTypesCallback composed from their operands' results. This deletes the founding pathology of the rewrite: BooleanAndHandler::resolveType's re-walk of the left operand on a throwaway storage, BOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH and both flattened-chain code paths - deep chains compose through nested results. Child narrowing flows through DefaultNarrowingHelper::getChildSpecifiedTypes(): the child result's specifyTypesCallback first, bridged through the old-world dispatcher for unmigrated children (the dispatcher answers converted handlers from stored results, so the bridge terminates; it dies in 3.0). The ternary still rewrites itself as (cond && if) || (!cond && else) - the synthetic takes the on-demand path where its real subnodes answer from stored results. Lessons the conversion forced out of the engine: - A handler must never ask the scope about its own node mid-processing - no stored result exists yet, so the ask takes the on-demand path and recurses infinitely (CoalesceHandler's filterByFalseyValue($expr) for the right-side scope hung the suite). The equivalent narrowing is built directly from the left result instead. - Composite typeCallbacks evaluate later operands on their captured processing scopes. Re-filtering the asking scope loses the left side's side effects (by-ref writes, inline assignments); the child result's own point breaks synthetic compositions (min()'s $a < $b ? $a : $b reuses stored results of the real arg nodes, predating the synthetic's branch narrowing). The captured scope has both; native asks flavor it with doNotTreatPhpDocTypesAsCertain(). - ExpressionResult::getType()/getNativeType()/getTypeForScope() consult tracked expression holders before the typeCallback, mirroring the early return in MutatingScope::resolveType() - that is how the nullsafe handlers' ensured non-nullability of ($x ?? null) reaches type asks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…perties Closes phpstan/phpstan#10786 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The engine never processes on the rule-facing FiberScope - its type asks suspend, which crashes outside a fiber. One can reach processExprNode through a stored result's memoized truthy/falsey scope (first computed inside a rule fiber) consumed by a composite handler for a child's processing scope: phpstan-phpunit's assertEmpty() extension builds a synthetic BooleanOr, the converted BooleanOrHandler processes it on demand, and the right arm's scope comes from the left arm's stored result. Convert to the mutating flavor at the processExprNode boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
from
8eeba94 to
be672de
Compare
Replaces the blanket processExprNode conversion with the root cause: the
hooks are the boundary between the rule-facing world and the engine.
Rules hold FiberScopes and feed them straight into the old-world
dispatcher (ImpossibleCheckTypeHelper passes the rule's scope to
specifyTypesInCondition, phpstan-phpunit's assert extension builds a
synthetic BooleanOr there), so resolveTypeOfNewWorldHandlerNode() and
specifyTypesOfNewWorldHandlerNode() can run with $this being a
FiberScope. They now call toMutatingScope() - identity on a plain scope,
a state-preserving copy on a FiberScope - before invoking result
callbacks and on-demand processing.
Without the conversion the engine processes synthetic nodes on the
rule-facing scope, whose type asks suspend: wasteful inside a rule
fiber, fatal outside one ("Cannot suspend outside of a fiber"). Found
by an ExpressionResult creation tripwire after the CI-only crash never
reproduced locally.
Co-Authored-By: Claude Fable 5 <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
2 participants
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.
Groundwork for the "new world" where an expression is traversed once: after
processExpr, itsExpressionResultknows the before/after scopes, the type (typeCallback) and the narrowing (specifyTypesCallback), composed from child results instead of re-walking subtrees. Handlers then stop implementingTypeResolvingExprHandler; the old entry points (MutatingScope::resolveType, theTypeSpecifierdispatcher) are guarded behindNewWorld::disableOldWorld()and get mass-deleted in PHPStan 3.0.What's on the branch, bottom up:
ExpressionResultFactory: old-world type resolution entry points throw whenNewWorld::disableOldWorld()is flipped (the migration meter); allExpressionResultconstruction goes through a generated factory.ExpressionResultcarriesbeforeScope,expr,typeCallback,specifyTypesCallbackand is stored per node inExpressionResultStorage(layered O(1)duplicate()), replacing the stored before-Scope.ExprHandler/TypeResolvingExprHandlersplit:resolveType/specifyTypesmove 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;NodeScopeResolverpushes the storage of the analysis in progress throughMutatingScope::pushExpressionResultStorage()(always popped infinally, throwing on imbalance), andMutatingScopeanswers 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 inbin/phpstan. Also addsMutatingScope::applySpecifiedTypes-filterBySpecifiedTypeswithoutScope::getType().ScalarHandlerandArrayHandlerno longer implementTypeResolvingExprHandler. 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]infersarray{1, 2, 1, 3, 1, 2}.Verified: full test suite green,
make phpstanclean, and analysis memory back at baseline (no leak from the result graph despitegc_disable()).Closes phpstan/phpstan#13944
Closes phpstan/phpstan#12207
Closes phpstan/phpstan#7155
Closes phpstan/phpstan#2032
Closes phpstan/phpstan#10786
🤖 Generated with Claude Code