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

False Positive with array declaration & array_merge #1516

Closed
mikaelBrosset opened this issue Oct 16, 2018 · 14 comments
Closed

False Positive with array declaration & array_merge #1516

mikaelBrosset opened this issue Oct 16, 2018 · 14 comments

Comments

@mikaelBrosset
Copy link

mikaelBrosset commented Oct 16, 2018

phpstan with array_merge throws an error while declaring a new array;

$out[$k] = [];
$out[$k] = array_merge($out[$k], ['foo' => 'bar']);

// This comes in a foreach loop. Thus this might be the reason why.

This throws

Parameter #1 $arr1 of function array_merge expects array, array|string given.

But this should not show 'array|string' if I am correct !

Here is the link to the test : https://phpstan.org/r/ec3f94a5a68d29d017491a320190ca99

Thank you

Good day

@ondrejmirtes
Copy link
Member

Please follow the instructions and add a link to phpstan.org reproduction.

@mikaelBrosset
Copy link
Author

Link added for the test. Thank you very much.

@ondrejmirtes
Copy link
Member

The $out[$k] is still array|string because the original array is (array|string)[]. We don't track the $out[$k] because it's hard - $k might have changed meanwhile.

@mikaelBrosset
Copy link
Author

All right, thank you very much for the quick answer :)

@ondrejmirtes
Copy link
Member

Please don’t close it, it’s still a valid bug waiting for a solution 😊

@ondrejmirtes ondrejmirtes added this to the 0.10 milestone Oct 20, 2018
@dereuromark
Copy link
Contributor

dereuromark commented Aug 21, 2020

I have a different false positive here:

// Returns array as documented with keys that each contain an assoc array
$modules = static::releaseTypedModuleList($releaseGroup);

return $modules['major'] + $modules['minor'] + $modules['patch'];

PHPStan wrongly thinks too much here:

Method App\Model\Entity\ReleaseGroup::moduleList() should return array but returns (float|int).

Those are nested arrays, and the + operation on the assoc arrays are valid.

Manually applying casting seems to workaround it for now:

return (array)$modules['major'] + (array)$modules['minor'] + (array)$modules['patch'];

@ondrejmirtes
Copy link
Member

@dereuromark You should check what PHPStan thinks that $modules['major'], $modules['minor'], $modules['patch'] are, by comparing them to some non-sense value like === 1. You can take advantage of the array shapes (https://phpstan.org/writing-php-code/phpdoc-types#array-shapes) to tell PHPStan precisely what it needs to think about those types.

@dereuromark
Copy link
Contributor

dereuromark commented Aug 21, 2020

I argue from the other direction
If the typehint and docblock states cleary just "array", then phpstan must NOT assume it to be more precise than "mixed[]"
Right now it does that, and that is invalid IMO.
Making it array-shaped can sure help, but the root cause here is phpstan overstepping the inspection of such a mixed case.

To be clear: + operator is not only for int/float, but also valid for array merge op.
If PHPStan cannot be 100% sure about it being the first, it should not report any issue.

@ondrejmirtes
Copy link
Member

PHPStan knows that mixed + mixed can only ever be array|float|int so it uses that information.

Why it thinks in your case that it's just float|int - I don't know that, I'd need phpstan.org reproduction.

@dereuromark
Copy link
Contributor

dereuromark commented Aug 21, 2020

https://phpstan.org/r/7d71e14b-2165-4dd2-b25f-89228a4dfe8d shows it
We use it on level 5, so the other 2 errors about the shapes are not present anyway.

@ondrejmirtes
Copy link
Member

Yeah, so this is a separate bug. It shows that (array|float|int) + mixed is (float|int) which is wrong.

@ondrejmirtes
Copy link
Member

#3759

@ondrejmirtes
Copy link
Member

This one is fixed already.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants