Fix phpstan/phpstan#10172: Walk through a generic array is messed up with non-empty-array type#5356
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Conversation
…te type - Skip HasOffsetValueType propagation to parent variable in specifyExpressionType when the variable has a template type - The root cause was that foreach's empty/non-empty scope split narrowed the parent variable through ArrayDimFetch, destroying the template identity - New regression test in tests/PHPStan/Analyser/nsrt/bug-10172.php
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
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 iterating over an array dim fetch like
$a['data']where$ahas a template typeT of array{data: array<mixed>}, PHPStan's foreach handling would split the scope into "entered loop" and "didn't enter loop" branches. This caused the template typeTto be replaced witharray{data: array{}}|array{data: non-empty-array<mixed>}, producing a false positive on the return type check.Changes
src/Analyser/MutatingScope.php: Added a check inspecifyExpressionType()to skipHasOffsetValueTypepropagation from anArrayDimFetchto its parent variable when the parent has aTemplateType. The dim fetch expression's type is still stored separately and resolved correctly.tests/PHPStan/Analyser/nsrt/bug-10172.php: Regression test verifying the template type is preserved after foreach iteration over an array dim fetch.Root cause
In
MutatingScope::specifyExpressionType(), when narrowing anArrayDimFetchexpression (e.g.,$a['data']toarray{}), the method propagates the narrowing to the parent variable by intersecting withHasOffsetValueType. For template types likeT of array{data: array<mixed>}, this intersection destroys the template identity, replacing it with a concrete type. After the foreach's scope merge (combining the "loop executed" and "loop didn't execute" branches), the result was a union of concrete types instead of the original template type.The fix skips this propagation when the parent variable has a template type. This is safe because the dim fetch expression's narrowed type is stored independently in the expression types map, and
getType()resolves it directly without needing the parent to be narrowed.Test
Added
tests/PHPStan/Analyser/nsrt/bug-10172.phpwhich asserts that afterforeach ($a['data'] as $i), the type of$aremainsT of array{data: array<mixed>}instead of being split intoarray{data: array{}}|array{data: non-empty-array<mixed>}.Fixes phpstan/phpstan#10172