Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
M1ke opened this issue Jun 25, 2021 · 5 comments
Open

Comments

@M1ke
Copy link
Contributor

M1ke commented Jun 25, 2021

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
Copy link

I found these snippets:

https://psalm.dev/r/c75c90c4ec
<?php

/**
 * @template T of array<int, string>
 * @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<int, string>)|non-empty-array<int, string>' does not match the declared return type 'T:fn-takesanint as array<int, string>' for takesAnInt

ERROR: InvalidReturnType - 6:18 - The declared return type 'T:fn-takesanint as array<int, string>' for takesAnInt is incorrect, got '(T:fn-takesanint as array<int, string>)|non-empty-array<int, string>'

@M1ke
Copy link
Contributor Author

M1ke commented Jun 25, 2021

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
Copy link

psalm-github-bot bot commented Jun 25, 2021

I found these snippets:

https://psalm.dev/r/d6609968af
<?php

/**
 * @template T of array<int, string>
 * @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

/**
 * @template T of array<int, string>
 * @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<int, string>)|non-empty-array<int, string>' does not match the declared return type 'T:fn-takesanint as array<int, string>' for takesAnInt

ERROR: InvalidReturnType - 6:18 - The declared return type 'T:fn-takesanint as array<int, string>' for takesAnInt is incorrect, got '(T:fn-takesanint as array<int, string>)|non-empty-array<int, string>'

@kkmuffme
Copy link
Contributor

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
Copy link

I found these snippets:

https://psalm.dev/r/c39fbb3124
<?php

/**
 * @template T of int
 * @psalm-param array<T, string> $a
 * @psalm-return array<T, string>
 */
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

kkmuffme added a commit to kkmuffme/psalm that referenced this issue Jun 30, 2023
* remove useless replace return type provider code that returned incorrect type for array replacements and was worse duplicate of existing stubs
* require preg patterns to be non-empty-strings as otherwise it will throw a PHP notice
* improve return type of array replacements to be more correct (not fully correct due to vimeo#5994 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants