phpstan-src
phpstan-src copied to clipboard
add callback types for array_uintersect etc
This is a fixed version of https://github.com/phpstan/phpstan-src/pull/1571 (TK of array-key as you requested).
Fixes https://github.com/phpstan/phpstan/issues/7707
Unfortunately, it doesn't cover all variants due to the limitations of the stubs and the weird interface of the functions. E.g.
var_dump(array_uintersect(
['a' => 1, 'b' => 2],
['c' => 1, 'd' => 2],
['c' => 1, 'd' => 2],
['c' => 1, 'd' => 2],
// values comparison
static function (int $a, int $b): int {
return $a <=> $b;
}
));
results in:
Parameter #5 ...$rest of function array_uintersect expects array|(callable(mixed, mixed): int), Closure(int, int): int<-1, 1> given.
💡 • Type #2 from the union: Type int of parameter #1 $a of passed callable needs to be same or wider than parameter type mixed of accepting callable.
• Type #2 from the union: Type int of parameter #2 $b of passed callable needs to be same or wider than parameter type mixed of accepting callable.
But at least it's a step in the right direction.
Please try to open a separate PR with an alternative approach:
- Try writing FunctionParameterClosureTypeExtension for one of these functions instead of introducing generics into their stubs (we have PregReplaceCallbackClosureTypeExtension in the repo so you can see how it works)
- You can shuffle the signature parameters in ParametersAcceptorSelector so that the number of parameters will match the number of passed arrays and so that the last parameter/argument always has the correct name (so that the FunctionParameterClosureTypeExtension can simply ask about the
$value_compare_funcparameter and adjust its callable signature.
In theory this should get rid of all false positives. If this alternative PR is not successful then I'll merge this one.
@ondrejmirtes ~It turns out that neither approach works consistently out of the box.~
It seems that FunctionParameterClosureTypeExtension only narrows down the type inside the closure. It doesn't narrow it down for the purpose of CallFunctionParametersRule: https://phpstan.org/r/8ff9bc8f-4162-4ccf-bf5c-8b3d2061e68d
EDIT: I accidentally tested it without bleeding edge. So the rest of the comment is not true: https://phpstan.org/r/1bff567d-d9a1-4906-9fe0-83705f7854ab
~On the other hand, the reflection based approaches (this and V2) don't (sufficiently) narrow down the parameters inside of the closure. E.g.~
<?php namespace ParamsArrayIntersectUassoc;
use function PHPStan\Testing\assertType;
var_export(array_uintersect_assoc(
['a' => 'a', 'b' => 'b'],
['c' => 'c', 'd' => 'd'],
function (mixed $a, mixed $b): int {
assertType("'a'|'b'|'c'|'d'", $a);
assertType("'a'|'b'|'c'|'d'", $b);
return 1;
},
));
Results in:
9 Expected type 'a'|'b'|'c'|'d', actual: string
10 Expected type 'a'|'b'|'c'|'d', actual: string
So I suppose that should be fixed first. I'll try to have a look (no promises).
@ondrejmirtes I created a proof-of-concept PR which applies FunctionParameterClosureTypeExtension in FunctionCallParametersCheck. Please let me know whether this is a direction that you're comfortable with, so that I don't get too far down a wrong path again.
@ondrejmirtes I had another look at this, but I'm not convinced that FunctionParameterClosureTypeExtension is the way to go:
selectFromArgsdoes not know the function name. So it's not possible to modify the signatures of these functions there (without further changes).FunctionParameterClosureTypeExtensiondoesn't get the argument for which it's being called, so without receiving the hackedParameterReflectionit cannot do anything.
Would you be willing to move forward with the simple stub approach instead? I'd expect 2 array arguments to be the most common usage, so at least it's something (array_udiff is already handled this way).
Or perhaps even the "variadic in the middle" approach. Though you're right that it's very unexpected, so it could definitely lead to random bugs (I also forgot to look for other places which might need handling - e.g. NodeScopeResolver). On the other hand matching arguments to parameters is quite complicated in general, so probably not many people attempt to do that themselves.
This pull request has been marked as ready for review.
Thank you.