Fix phpstan/phpstan#4296: BleedingEdge: Offset '1234' on array<string, Test>&nonEmpty in isset() does not exist.#5174
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
… isset() - In ArrayType::hasOffsetValueType and getOffsetValueType, toArrayKey() converts numeric strings like '1234' to integers, losing the original string type info - Added check against original offset type before toArrayKey() conversion so that a string offset on a string-keyed array is not incorrectly rejected - Updated NonexistentOffsetInArrayDimFetchRuleTest to reflect that '0' on array<string, string> is no longer a false positive - New regression test in tests/PHPStan/Rules/Variables/data/bug-4296.php Closes phpstan/phpstan#4296
Contributor
|
I'm not sure we should do anything until phpstan/phpstan#6847 is fixed |
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
PHPStan incorrectly reported "Offset '1234' on array<string, Test>&nonEmpty in isset() does not exist" when checking a numeric string key against a string-keyed array. This fix ensures the original offset type (before
toArrayKey()conversion) is also checked against the array's key type.Changes
src/Type/ArrayType.php: In bothhasOffsetValueType()andgetOffsetValueType(), added a check that also compares the original offset type (beforetoArrayKey()conversion) against the array's key type. This prevents numeric string keys like'1234'from being falsely rejected when the array hasstringkeys.tests/PHPStan/Rules/Variables/data/bug-4296.php: New regression test reproducing the issue.tests/PHPStan/Rules/Variables/IssetRuleTest.php: AddedtestBug4296method.tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php: Removed false positive expectation for'0'onarray<string, string>(same root cause).Root cause
ArrayType::hasOffsetValueType()callstoArrayKey()on the offset type, which converts numeric strings (e.g.,'1234') to their integer equivalents (e.g.,1234) per PHP semantics. The subsequent check$this->getKeyType()->isSuperTypeOf($offsetType)then comparesStringTypeagainstConstantIntegerType(1234), which returnsno. The fix preserves the original offset type and checks it against the key type as well — since'1234'IS astringand the key type ISstring, the offset should not be rejected.Test
Added
tests/PHPStan/Rules/Variables/data/bug-4296.phpthat reproduces the exact scenario from the issue: building anarray<string, Test>via a loop, then checkingisset($map[$value])where$valueis the numeric string'1234'. The test expects no errors.Fixes phpstan/phpstan#4296