vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.57k stars 660 forks source link

Non-empty object-like array intersection inconsistencies #5561

Open AndrolGenhald opened 3 years ago

AndrolGenhald commented 3 years ago

This works: https://psalm.dev/r/32b7137fa9 This doesn't: https://psalm.dev/r/6b95034171 This is a false negative: https://psalm.dev/r/be95fcaee2

It's also described as array{...}<...> instead of non-empty-array{...}<...>: https://psalm.dev/r/abbebc2e18

I'm also not sure I like the syntax, array{...}&array<...> seems to me like it should simplify to array{...}, since we're intersecting two types, and array<...> is a supertype of array{...}. I think it would be nice to have array{...} be exhaustive (#5299) and array(non-exhaustive){...} or array(nonstrict){...} be a non-exhaustive object-like array (see #5560).

psalm-github-bot[bot] commented 3 years ago

I found these snippets:

https://psalm.dev/r/32b7137fa9 ```php $bar */ function foo(array $bar): array { return $bar; } /** @var non-empty-array&array{foo?: string, bar?: string} */ $foo = ['foobar' => 'baz']; foo(array_values($foo)); ``` ``` Psalm output (using commit d022910): No issues! ```
https://psalm.dev/r/6b95034171 ```php $bar */ function foo(array $bar): array { return $bar; } /** @var non-empty-array&array{foo?: string, bar?: string} */ $foo = ['foobar' => 'baz']; $bar = []; foreach ($foo as $val) { $bar[] = $val; } foo($bar); ``` ``` Psalm output (using commit d022910): ERROR: InvalidScalarArgument - 17:5 - Argument 1 of foo expects non-empty-list, list provided ```
https://psalm.dev/r/be95fcaee2 ```php $bar */ function foo(array $bar): array { return $bar; } /** @var array&array{foo?: string, bar?: string} */ $foo = ['foobar' => 'baz']; foo(array_values($foo)); ``` ``` Psalm output (using commit d022910): No issues! ```
https://psalm.dev/r/abbebc2e18 ```php &array{foo?: string, bar?: string} * @psalm-trace $foo */ $foo = ['foobar' => 'baz']; ``` ``` Psalm output (using commit d022910): INFO: Trace - 7:1 - $foo: array{bar?: string, foo?: string} INFO: UnusedVariable - 7:1 - $foo is never referenced or the value is not used ```
orklah commented 3 years ago

It seems that Psalm collapse non-empty-array<string, string>&array{foo?: string, bar?: string} into array{bar?: string, foo?: string}<string, string> which make it loose the non-empty part. This is probably cause by TypeCombiner. Not sure what effect it could have if we didn't. We could try.

About exhaustivity of array shapes, there's a debate about the syntax that should be used for that. I'll let PHPStan takes the lead on that and we'll follow with the syntax I think. Psalm current syntax is a bit weird and not consistent.