Narrow parent constant array type when type-checking an array offset#5590
Closed
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Closed
Narrow parent constant array type when type-checking an array offset#5590phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
phpstan-bot wants to merge 2 commits intophpstan:2.1.xfrom
Conversation
When is_string($arr['key']) narrows the offset type, also propagate a sureNotType on the parent array variable to remove constant array members whose offset value is fully covered by the narrowing. This prevents false positives when BooleanAnd's intersectWith drops entries for different expression keys. Also extend the coalesce simplification in createForExpr's false context to handle non-null type checks (e.g. is_string($x ?? null) in the falsey branch can be simplified to just narrowing $x). Closes phpstan/phpstan#14566
c4a74c8 to
39ed27d
Compare
Contributor
VincentLanglet
left a comment
There was a problem hiding this comment.
Explains why
/**
* @param array{}|array{hi: 'hello'}|array{hi: array{0: 42, 1?: 42}} $test
*/
function foo(array $test): void {
if (!isset($test['hi'])) {
return;
}
if (\is_string($test['hi'])) {
return;
}
$test['hi'][] = 42;
}
/**
* @param array{}|array{hi: 'hello'}|array{hi: array{0: 42, 1?: 42}} $test
*/
function foo2(array $test): void {
if (!\is_array($test['hi'] ?? null)) {
return;
}
$test['hi'][] = 42;
}
already works fine
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Author
|
Done. Added the two equivalent patterns from VincentLanglet's review as additional test cases ( |
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
is_string($arr['key'])(or similar type-check functions) narrows an array offset's type, also propagate asureNotTypeon the parent array variable. This removes constant array members whose offset value is fully covered by the narrowing, preventing false positives whenBooleanAnd'sintersectWithdrops entries for different expression keys.createForExpr's false context so thatis_string($x ?? null)in the falsey branch simplifies to narrowing$x(since string is non-null, the?? nullfallback is irrelevant).Test plan
tests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest::testBug14566— regression test with bothisset && is_stringandis_string($x ?? null)patternstests/PHPStan/Rules/Arrays/OffsetAccessAssignmentRuleTest::testBug9004— existing test updated (was asserting buggy behavior)make phpstan)make cs-fix)Closes phpstan/phpstan#14566