Fix analysis on array_map with named arguments
#3763
Conversation
| $b = array_map( | ||
| array: $a, | ||
| callback: static fn(Uuid $c): string => (string) $c, | ||
| ); |
There was a problem hiding this comment.
please add another test like:
$b = array_map(
callback: static fn(Uuid $c): string => (string) $c,
array: $a,
);
(parameters in different order, which is allowed for named arguments).
I think there is a another case with more then 2 args which needs testing
There was a problem hiding this comment.
I added some more test cases.
|
it might make sense to re-use ArgumentsNormalizer for this fix |
|
I wouldn't probably do that, we could spin in an infinite recursion maybe. |
|
I'm not sure why integration test fails... |
There was a problem hiding this comment.
What we typically do to normalize args like this is to:
- Add
$callback = null; $array = null;. - Iterate over
$argsand assign these variables either based on$icounter in the foreach, or by arg name. - Recreate
$argsby doing[$callback, $array]in case both of them were found.
Thanks.
src/Parser/ArrayMapArgVisitor.php
Outdated
| if ($args[1]->name !== null && $args[1]->name->name === 'callback') { | ||
| $callbackPos = 1; | ||
| } | ||
| [$callback] = array_splice($args, $callbackPos, 1); |
There was a problem hiding this comment.
I have no idea what's going on at this line. Please make the code easier to read.
There was a problem hiding this comment.
I made some refactors.
|
|
||
| $this->checkExplicitMixed = true; | ||
| $this->checkImplicitMixed = true; | ||
| $this->analyse([__DIR__ . '/data/bug-12317.php'], []); |
There was a problem hiding this comment.
Would be a good idea to add an example that actually produces an error, to verify the args still look like they should.
There was a problem hiding this comment.
Thank you for your suggestion. I added examples and fixed a bug caught by them.
There was a problem hiding this comment.
I'm sorry but it seems like you haven't followed my comment (#3763 (review)) at all. There's no foreach in the diff so it's wrong. Code using array_slice/splice is very hard to read and I don't know if it's correct.
Also I'd like the array from the visitor to always look the same way (callback first, array 2nd), so that no other code has to be adjusted for named arguments.
Closes phpstan/phpstan#12317