Fix phpstan/phpstan#14347: Regression: Setting array-key in loop doesn't persist with other actions in between#5287
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
… different key - When an array dim fetch assignment (e.g. $arr['all']++) reassigns the root variable, expression types for dynamic array dim fetches (e.g. $arr[$card->value]) are now preserved - Added getDynamicArrayDimFetchExpressionTypes() and restoreExpressionTypes() to MutatingScope - New regression test in tests/PHPStan/Rules/Arrays/data/bug-14347.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 initializing an array entry with
??=in a loop, modifying a different array key in between would cause PHPStan to lose track of the initialized key, resulting in a false positiveoffsetAccess.notFounderror.Changes
getDynamicArrayDimFetchExpressionTypes()method tosrc/Analyser/MutatingScope.phpto collect expression types for array dim fetches with non-scalar (dynamic) dim expressionsrestoreExpressionTypes()method tosrc/Analyser/MutatingScope.phpto directly restore expression type holders without triggering parent-type side effectssrc/Analyser/ExprHandler/AssignHandler.phpto save and restore dynamic array dim fetch expression types aroundassignVariablecalls during array dim fetch assignmentstests/PHPStan/Rules/Arrays/data/bug-14347.phpRoot cause
When processing an array dim fetch assignment like
$arr['all']++, PHPStan computes the new array type and callsassignVariable(), which internally callsinvalidateExpression()on the variable. This invalidation removes ALL expression types containing that variable, including$arr[$dynamicKey]which was set by a prior??=operation.The
NonexistentOffsetInArrayDimFetchRulechecks$scope->hasExpressionType($node)at line 72 to skip the error when the expression was previously assigned. With the expression type invalidated, this check fails and the rule reports a false positive.The fix preserves expression types for dynamic (non-scalar) array dim fetches before the
assignVariablecall and restores them afterward. Only dynamic dim fetches are preserved to avoid interfering with condition-narrowed types on constant keys (like those fromarray_key_existschecks).Test
Added
tests/PHPStan/Rules/Arrays/data/bug-14347.phpwith two functions:countCards(working order) andcountCardsBroken(broken order where??=comes before modifying another key). Both should analyze cleanly withreportPossiblyNonexistentConstantArrayOffset = true.Fixes phpstan/phpstan#14347