vimeo / psalm

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

Invalid return type for template where param and return are the same array #5994

Open M1ke opened 3 years ago

M1ke commented 3 years ago

https://psalm.dev/r/c75c90c4ec

The issue here is that we're wanting the docblock to declare that the function must return the input param. Psalm instead requires us to permit the function to return either the input value, or an alternative array that shares a similar type description.

This means that the common goal of "a function that takes an array, modifies it and returns it" can't be represented without also permitting the function to take an array, discard it and return something that looks like it.

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

I found these snippets:

https://psalm.dev/r/c75c90c4ec ```php * @psalm-param T $a * @psalm-return T */ function takesAnInt(array $a) { for ($i=random_int(0,10); $i<10; $i++){ $a[$i] = md5((string) $i); } return $a; } ``` ``` Psalm output (using commit ee1c36c): ERROR: InvalidReturnStatement - 13:12 - The inferred type '(T:fn-takesanint as array)|non-empty-array' does not match the declared return type 'T:fn-takesanint as array' for takesAnInt ERROR: InvalidReturnType - 6:18 - The declared return type 'T:fn-takesanint as array' for takesAnInt is incorrect, got '(T:fn-takesanint as array)|non-empty-array' ```
M1ke commented 3 years ago

If we unset the first key instead then Psalm has no issues with modifying it

https://psalm.dev/r/d6609968af

However if we modify an internal item (even if we explicitly check it exists first) the same issue is thrown.

https://psalm.dev/r/c75c90c4ec

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

I found these snippets:

https://psalm.dev/r/d6609968af ```php * @psalm-param T $a * @psalm-return T */ function takesAnInt(array $a) { unset($a[random_int(0,10)]); return $a; } ``` ``` Psalm output (using commit ee1c36c): No issues! ```
https://psalm.dev/r/c75c90c4ec ```php * @psalm-param T $a * @psalm-return T */ function takesAnInt(array $a) { for ($i=random_int(0,10); $i<10; $i++){ $a[$i] = md5((string) $i); } return $a; } ``` ``` Psalm output (using commit ee1c36c): ERROR: InvalidReturnStatement - 13:12 - The inferred type '(T:fn-takesanint as array)|non-empty-array' does not match the declared return type 'T:fn-takesanint as array' for takesAnInt ERROR: InvalidReturnType - 6:18 - The declared return type 'T:fn-takesanint as array' for takesAnInt is incorrect, got '(T:fn-takesanint as array)|non-empty-array' ```
kkmuffme commented 1 year ago

class-string-map https://psalm.dev/docs/annotating_code/type_syntax/utility_types/#class-string-mapt-of-foo-t does exactly this for class instances. We would need a more generic type to be used for cases of "normal" arrays (and lists) To get this to work correctly too https://psalm.dev/r/c39fbb3124

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/c39fbb3124 ```php $a * @psalm-return array */ function takesAnInt($a) { foreach ( $a as $key => $value ) { $a[ $key ] = 'hello' . $value; } return $a; } $x = takesAnInt( array( 10 => 'bar', 15 => 'world' ) ); /** @psalm-trace $x */; ``` ``` Psalm output (using commit 086b943): INFO: Trace - 17:23 - $x: array<10|15, string> INFO: UnusedVariable - 16:1 - $x is never referenced or the value is not used ```