Intersect stored FuncCall expression types with dynamically computed types in MutatingScope::resolveType()#5454
Open
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Open
Conversation
…d types in `MutatingScope::resolveType()` - When a FuncCall expression (e.g. `count($arr)`) has a stored expression type from a prior condition, `resolveType()` now also computes the dynamic type via the ExprHandler and returns the intersection of both. - This prevents stale stored types from overriding the correct type when function arguments have been narrowed after scope merging. - The bug occurred when `count($arr)` was used in a comparison (e.g. `count($arr) > 2`), storing `count($arr) = int<0, max>` after merge, and then `$arr` was narrowed to non-empty by a different condition. - The fix also covers analogous cases: `sizeof()`, `strlen()`, `mb_strlen()`, and any other FuncCall with stored expression types.
staabm
reviewed
Apr 14, 2026
Comment on lines
+976
to
+994
| $storedType = $this->expressionTypes[$exprString]->getType(); | ||
|
|
||
| if ($node instanceof FuncCall) { | ||
| // Stored expression types for function calls can become stale | ||
| // when arguments are narrowed after scope merging. | ||
| // Intersect with the dynamically computed type to stay correct. | ||
| $this->resolvedTypes[$exprString] = $storedType; | ||
| foreach ($this->container->getServicesByTag(ExprHandler::EXTENSION_TAG) as $exprHandler) { | ||
| if (!$exprHandler->supports($node)) { | ||
| continue; | ||
| } | ||
| $dynamicType = $exprHandler->resolveType($this, $node); | ||
| unset($this->resolvedTypes[$exprString]); | ||
| return TypeCombinator::intersect($storedType, $dynamicType); | ||
| } | ||
| unset($this->resolvedTypes[$exprString]); | ||
| } | ||
|
|
||
| return $storedType; |
Contributor
There was a problem hiding this comment.
instead of re-calculating types at type-resolving time, couldn't we make sure expressionTypes are properly updated when evaluating the AST?
Collaborator
Author
There was a problem hiding this comment.
Done. Here's a summary of the changes made to address staabm's review:
Review feedback: Instead of re-calculating types at type-resolving time, update expressionTypes during AST evaluation.
What changed:
-
Reverted
resolveType()back to its original behavior - no more special FuncCall handling at type resolution time. -
Added FuncCall type updating in
filterBySpecifiedTypes()- when type narrowing is applied from conditions (e.g.,$arr !== []narrows$arr), the method now detects stored FuncCall expression types whose arguments were actually narrowed and updates them by intersecting the stored type with the freshly computed dynamic type.
Key design decisions:
- Intersection, not invalidation: Simply removing stale FuncCall types would lose precision from conditions on the function result (e.g.,
count($arr) === 3gives3, notint<1, max>). Intersecting preserves narrowing from both sources. - Type change guard: Only updates FuncCall entries when the argument's type actually changed, avoiding unnecessary recomputation when narrowing is redundant.
- Skip specified/conditional entries: FuncCall expressions that were directly specified or updated by conditional expressions are left alone.
Test results: All 11,855 tests pass, PHPStan self-analysis passes with no errors.
Address review feedback: instead of re-calculating FuncCall types at type-resolving time, update stored expression types during AST evaluation when their arguments are narrowed. In filterBySpecifiedTypes(), after processing type specifications and conditional expressions, detect FuncCall entries whose arguments' types actually changed. For each, temporarily remove the stored type, compute the dynamic type from the narrowed arguments, then store the intersection. This preserves narrowing from both sources: conditions on the function result (e.g. count($arr) === 3) and conditions on the arguments (e.g. $arr !== []). Co-Authored-By: Claude Opus 4.6 <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.
Summary
When
count($arr)was used in a comparison likecount($arr) > 2, the TypeSpecifier stored the narrowedcount($arr)type in the scope's expression types for both the truthy and falsey branches. After scope merging, the stored type becameint<0, max>. If$arrwas subsequently narrowed tonon-empty-arrayby a different condition (e.g.count($arr, COUNT_RECURSIVE) > 2or$arr !== []), the stale storedcount($arr)=int<0, max>was returned instead of being recomputed from the narrowed array type.Changes
MutatingScope::resolveType()insrc/Analyser/MutatingScope.php: when aFuncCallexpression has a stored expression type (with YES certainty), also compute the type dynamically via the ExprHandler and return the intersection of the stored and dynamic types.resolvedTypesas a recursion guard before computing the dynamic type.tests/PHPStan/Analyser/nsrt/bug-13750.phpcovering:count($arr)aftercount($arr) > 2thencount($arr, COUNT_RECURSIVE) > 2count($arr)aftercount($arr) > 2then$arr !== []sizeof()(alias for count)strlen()afterstrlen() > 5then$str !== ''Root cause
The bug was in
MutatingScope::resolveType(). When a non-variable expression (likecount($arr)) had a stored expression type with YES certainty,resolveType()returned it directly without consulting the ExprHandler/DynamicReturnTypeExtension. This is correct when the stored type is still valid, but becomes stale when the function's arguments are narrowed by a subsequent condition.The conditional expressions mechanism (created during scope merging) normally handles this by linking derived expressions to their constituent variables. However, when using
count($arr) > Nwith N >= 1, the falsey branch leaves$arrunchanged (array), which is the same as the merged type. This means$arris not identified as a "type guard", so no conditional expression linkingcount($arr)to$arris created. The stalecount($arr)type then persists uncorrected.The fix intersects the stored type with the dynamically computed type for all
FuncCallexpressions. This is always correct: the stored type narrows based on conditions, the dynamic type narrows based on current argument types, and their intersection is the most specific correct type.Test
tests/PHPStan/Analyser/nsrt/bug-13750.php: NSRT test withassertType()calls verifying correctcount(),sizeof(), andstrlen()return types after scope merging and subsequent variable narrowing.Fixes phpstan/phpstan#13750