Valid array property type error not reported since phpstan v1.4

[mvorisek]

Bug report

in phpstan v1.3, the type error was correctly reported

Code snippet that reproduces the problem

https://phpstan.org/r/b46d101c-7e40-4483-a65a-7a532dc8247b

Expected output

 ------ ------------------------------------------------------------------- 
  Line   src/Persistence/Static_.php                                        
 ------ ------------------------------------------------------------------- 
-  29 | No error to ignore is reported on line 29. 
 ------ ------------------------------------------------------------------- 

[mvorisek]

I updated the description, it is reproducible with level 6. The issue is that phpstan since v1.4 unions the type to be assigned with the array type instead of checking the type against the type for the given key. And since level 6 is weak on union types, no error is reported. I belive the original error is however valid and should continue to be reported.

l8 demo https://phpstan.org/r/ddc84a3c-4edb-4029-aba3-c61471e6317e (notice the whole prop type is compared)

[ondrejmirtes]

This is kind of expected. The new way of verifying this compares the expected property type against the assigned one.

And the assignment changes the type from array<string, Table> to array<string, bool|Table>. That's why it's reported on level 7 with union types.

This refactoring fixed some errors that previously escaped PHPStan (#5804, #6333, #4906, #3703). I don't know how to achieve the same result so that PHPStan would see Table -> bool instead of Table -> bool|table.

[mvorisek]

I don't know how to achieve the same result so that PHPStan would see Table -> bool instead of Table -> bool|table.

when assigning an array element, what about checking the key and the value type individually?

[ondrejmirtes]

That's what was done before and it didn't work so well. Also there's the gray zone of "maybe an array" type that complicates things, other offset-accessible types like string and ArrayAccess, and also all the other checks we achieved with the last refactoring - like checking the property type when doing array destructuring - [$this–>foo] = $array;... I made all of this work at a small price of some errors now being reported on level 7 instead of sooner...

[mvorisek]

I fully understand the reasonds behind, but this change * effectively allows to assign any value to any array *, even when the array is defined with a simple type (eg. checked by much lower level). I belive this needs to be fixed, can you please keep this issue open?

[ondrejmirtes]

Not really, this is only a problem with ArrayDimFetch assignments like $this->foo[0] = .... Simple assignments like $this->foo = $foo; are still checked on a lower level. So if the type is always wrong, not just a partially wrong union type, it's gonna be reported on level 3: https://phpstan.org/r/877edd88-12e0-457f-8794-6aedbd00a3ef

[mvorisek]

sure, but any array property update/append (common usecases in builder/registry design patters) is now effectively unchecked https://phpstan.org/r/22e2d1ec-419b-48f0-ae23-34cc05c70ef0

[ondrejmirtes]

You always need to account for the fact that unless you're on the highest level, you're missing out on bugs.