phpstan-src icon indicating copy to clipboard operation
phpstan-src copied to clipboard

Report non existent offset on non empty array

Open VincentLanglet opened this issue 5 months ago • 5 comments

Closes https://github.com/phpstan/phpstan/issues/7143

I discovered that a maybe not existent offset

  • was reported on array{'foo'?: 1, 'bar'?: 2}
  • was not reported on non-empty-array{'foo'?: 1, 'bar'?: 2}

It seems like it's because TypeUtils::flattenType does not support Intersection types so

TypeUtiles::flattenType(array{'foo'?: 1, 'bar'?: 2})
// returns [], ['foo' => 1], ['bar' => 2], ['foo' => 1, 'bar' => 2]

but

TypeUtiles::flattenType(non-empty-array{'foo'?: 1, 'bar'?: 2})
// returns `non-empty-array{'foo'?: 1, 'bar'?: 2}`
// instead of ['foo' => 1], ['bar' => 2], ['foo' => 1, 'bar' => 2]

Funny thing, fixing it is breaking some non-regression tests like

  • https://github.com/phpstan/phpstan/issues/11602: this was supposed to be fixed by https://github.com/phpstan/phpstan-src/pull/3899 but wasn't: the array shape was transformed into an intersection list&array shape, that's why the error wasn't reported anymore. (cc @staabm) => A way to see this is the fact it's reported with reportPossiblyNonexistentConstantArrayOffset https://phpstan.org/r/42af1687-c88d-4369-8c0b-7d3d1a14fcaa

  • https://github.com/phpstan/phpstan/issues/6379: this was supposed to be fixed but wasn't too: the array shape was transformed into an intersection non-empty-array&array shape, so the error was not reported anymore. => A way to see this is the fact it's reported with reportPossiblyNonexistentConstantArrayOffset https://phpstan.org/r/656d35b8-2ea0-4207-99b7-cd218f2151b9

What should I do in such case ? Removing the non-regression test and you'll open the two issues @ondrejmirtes ?

VincentLanglet avatar Oct 03 '25 07:10 VincentLanglet

What I currently do not fix:

/**
 * @param array{foo?: string, bar?: string, 1?:1, 2?:2, 3?:3, 4?:4, 5?:5, 6?:6, 7?:7, 8?:8, 9?:9}&non-empty-array $arr
 */

Because when there is more than 10 optional keys getAllArrays return [] and an array with all the existing keys ; and then the intersection with non-empty-array removes the empty array...

VincentLanglet avatar Oct 03 '25 07:10 VincentLanglet

I think the previous behaviour is intentional, as non-empty-array{'foo'?: 1, 'bar'?: 2} means at least one of the 2 offsets is set (because its non-empty) but we don't know better and therefore play it safe (to not be annoying)

staabm avatar Oct 03 '25 07:10 staabm

I think the previous behaviour is intentional, as non-empty-array{'foo'?: 1, 'bar'?: 2} means at least one of the 2 offsets is set (because its non-empty) but we don't know better and therefore play it safe

I don't think it's intentional. The "play it safe" argument is ok on level < 7, but in my mind that's exactly the purpose of "reportMaybe". Here, even if the array is non empty, we cannot be sure which key si non-optional so we have to report the key might not exists.

You have the same issue (as showed by https://phpstan.org/r/a9660963-948d-4d4b-9cb8-166af8806987) with

list{0: non-falsy-string, 1: non-empty-string, 2: numeric-string, 3: string, 4?: string, 5?: numeric-string}

which doesn't report that offset 4 might not exists BECAUSE it's a list, and as soon as you unset the 0 value it's reported https://phpstan.org/r/c4ad2970-a61a-42a4-8600-22a889a7b737

In https://github.com/phpstan/phpstan/issues/7143 the same can be reproduce with unset($arr['foo']); which starts triggering the might-not-exists error on bar.

VincentLanglet avatar Oct 03 '25 07:10 VincentLanglet

Failure

1) PHPStan\Rules\Arrays\NonexistentOffsetInArrayDimFetchRuleTest::testBug6379
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'16: Offset 'c' might not exist on non-empty-array{cr?: string, c?: string}.
 '

/home/runner/work/phpstan-src/phpstan-src/src/Testing/RuleTestCase.php:179
/home/runner/work/phpstan-src/phpstan-src/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php:390

2) PHPStan\Rules\Arrays\NonexistentOffsetInArrayDimFetchRuleTest::testBug11602
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'16: Offset 5 might not exist on list{0: non-falsy-string, 1: non-empty-string, 2: numeric-string, 3: string, 4?: string, 5?: numeric-string}.
+19: Offset 4 might not exist on list{0: non-falsy-string, 1: non-empty-string, 2: numeric-string, 3: string, 4?: string, 5?: numeric-string}.

are expected, those bug was never fixed it just stopped to be reported because the array was non-empty.

So we have to remove the tests and reopen the issues...

VincentLanglet avatar Oct 06 '25 17:10 VincentLanglet

I don't think

$type instanceof IntersectionType && $type->isConstantArray()->maybe();

is something doable... so I dunno how to cover the mutant

VincentLanglet avatar Dec 05 '25 14:12 VincentLanglet