Fix #14177: array_key_exists false positive#5024
Closed
phpstan-bot wants to merge 1 commit into2.1.xfrom
Closed
Conversation
…ing keys When `ConstantArrayType::unsetOffset()` removed an optional trailing key from a list type, it unconditionally set `isList` to `No`. This caused `TypeCombinator::remove()` to produce `NeverType` when re-intersecting with `AccessoryArrayListType`, since the list accessor rejects non-list arrays. The `NeverType` falsy branch then collapsed scope merging after conditionals like ternaries, propagating the narrowed (truthy) type beyond the conditional and triggering false "always true" reports on subsequent `array_key_exists()` calls. The fix preserves the original `isList` value when unsetting an optional key whose removal leaves the remaining keys as consecutive integers starting from 0. Fixes phpstan/phpstan#14177
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.
This PR fixes phpstan/phpstan#14177.
Summary
When using
array_key_exists()on a list type with optional trailing keys (e.g. frompreg_replace_callbackwith optional capture groups), PHPStan incorrectly reported subsequentarray_key_exists()calls as "always true".Root cause
ConstantArrayType::unsetOffset()unconditionally setisListtoTrinaryLogic::createNo()when removing a key. When this method was called during type narrowing (viatryRemove(HasOffsetType)in the falsy branch ofarray_key_exists), the resultingConstantArrayTypewithisList=Nowas re-intersected withAccessoryArrayListTypebyIntersectionType::tryRemove(). SinceAccessoryArrayListType::isSuperTypeOf()returnsNofor non-list types,TypeCombinator::intersect()collapsed the result toNeverType.The
NeverTypefalsy branch caused scope merging after conditionals (ternaries, if/else) to use only the truthy branch type, propagating incorrect type narrowing beyond the conditional.Fix
In
ConstantArrayType::unsetOffset(), when the key being removed is optional and the original type hasisListasYesorMaybe, check whether the remaining keys still form consecutive integers starting from 0. If so, preserve the originalisListvalue instead of forcing it toNo.The check is limited to optional keys only, so actual
unset()operations on required keys continue to setisList=Noas before.Test plan
tests/PHPStan/Rules/Comparison/data/bug-14177.phpreproducing the original issue withpreg_replace_callbackand optional capture groupstests/PHPStan/Analyser/nsrt/bug-14177.phpverifying correct type narrowing in both branches and after mergearray-is-list-unset.php