Intersect cached count()/strlen() expression types with current argument type constraints#5702
Intersect cached count()/strlen() expression types with current argument type constraints#5702phpstan-bot wants to merge 2 commits into
count()/strlen() expression types with current argument type constraints#5702Conversation
…rgument type constraints - In `MutatingScope::resolveType()`, when returning a cached expression type for `count()`/`sizeof()` calls, intersect it with the current `getArraySize()` of the argument type. This ensures the cached type reflects any subsequent type narrowing of the argument. - Apply the same fix for `strlen()`/`mb_strlen()`: when the argument has been narrowed to `non-empty-string`, intersect the cached type with `int<1, max>`. - Guard against first-class callable syntax (`count(...)`) which does not have args.
| if ( | ||
| $node instanceof FuncCall | ||
| && $node->name instanceof Name | ||
| && !$node->isFirstClassCallable() | ||
| && count($node->getArgs()) >= 1 | ||
| ) { | ||
| $funcName = $node->name->toLowerString(); | ||
| if (in_array($funcName, ['count', 'sizeof'], true)) { | ||
| $argType = $this->getType($node->getArgs()[0]->value); | ||
| $type = TypeCombinator::intersect($type, $argType->getArraySize()); | ||
| } elseif ( | ||
| in_array($funcName, ['strlen', 'mb_strlen'], true) | ||
| && $this->getType($node->getArgs()[0]->value)->isNonEmptyString()->yes() | ||
| ) { | ||
| $type = TypeCombinator::intersect($type, IntegerRangeType::fromInterval(1, null)); | ||
| } | ||
| } |
There was a problem hiding this comment.
why do we need to patch the types after the fact, instead of remembering the proper types in the first place?
There was a problem hiding this comment.
Pushed. Here's a summary of what changed:
The reviewer's concern: Why patch cached types on retrieval instead of storing the correct types from the start?
The fix: Moved the refinement logic from resolveType() (retrieval time) to specifyExpressionType() (storage time). When a variable's type is narrowed via specifyExpressionType, the new refineDependentFuncCallTypes method scans expressionTypes for count()/sizeof()/strlen()/mb_strlen() FuncCall entries whose first argument matches the narrowed variable, and intersects their cached types with the constraint derived from the new variable type. This ensures cached expression types are always correct when retrieved, rather than being patched after the fact.
The resolveType() method is back to simply returning the cached type directly.
make phpstan— no errorsmake tests— all 12,105 tests pass- Verified test fails without any fix, passes with the storage-time fix
…l time Move the count()/sizeof()/strlen()/mb_strlen() cached expression type refinement from resolveType() (retrieval time) into specifyExpressionType() (storage time). When a variable's type is narrowed, dependent FuncCall expression types are now intersected with the new constraint immediately, so the cached types are always correct when retrieved. This addresses the review feedback: types are remembered properly from the start, rather than being patched after the fact on each retrieval. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Summary
When
count($arr)appeared in a comparison (e.g.,count($arr) > 2), the TypeSpecifier cached the resulting integer range in the scope'sexpressionTypes. After the conditional block merged its branches, the cachedcount($arr)becameint<0, max>. If a subsequent condition narrowed$arrtonon-empty-arraythrough a different mechanism (e.g.,count($arr, COUNT_RECURSIVE) > 2), the stalecount($arr) = int<0, max>was returned from the cache instead of being recomputed asint<1, max>.Changes
src/Analyser/MutatingScope.php,resolveType(): when returning a cached expression type for acount()/sizeof()FuncCall, intersect it with the argument's currentgetArraySize(). This correctly refines the cached type when the argument has been narrowed (e.g., fromarraytonon-empty-array).strlen()/mb_strlen()— when the argument has been narrowed tonon-empty-string, intersect the cached type withint<1, max>.count(...)) which has no args.Analogous cases probed
sizeof()— alias forcount(), same fix applies automatically via the['count', 'sizeof']check. Tested.strlen()/mb_strlen()— same stale-type pattern. Fixed and tested.preg_match()— also mentioned alongside count/strlen in the TypeSpecifier comparison handler, but its return type does not depend on the argument type narrowing, so not affected.count($arr) = int<3, max>inside an outerif (count($arr) > 2)is NOT degraded when an inner condition narrows$arrwithout changing its type. ThegetArraySize()intersection only tightens, never widens.Root cause
The TypeSpecifier's comparison handler (lines 486–528 of
TypeSpecifier.php) caches integer range types for both sides of</<=/>/>=comparisons as expression type holders. These cached types persist across scope transitions (branch merges, nested conditions). When a variable appearing as a function argument is subsequently narrowed by a different condition, the stale function-call expression type was returned verbatim from the cache inresolveType(), bypassing the dynamic return type extension that would have computed the correct type from the narrowed argument.The fix intersects the cached type with a lower bound derived from the current argument type, ensuring the cached type is at least as precise as what the current argument type implies.
Test
tests/PHPStan/Analyser/nsrt/bug-13750.php— regression test covering:count($arr)aftercount($arr, COUNT_RECURSIVE) > 2narrowingsizeof()analogcount($arr)after$arr !== []narrowingstrlen()after non-empty string narrowingmb_strlen()after non-empty string narrowingFixes phpstan/phpstan#13750