Skip to content

Commit

Permalink
Fix problem with closure parameter and generics
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed May 22, 2024
1 parent e3df582 commit da4fd7a
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 25 deletions.
20 changes: 16 additions & 4 deletions src/Rules/FunctionCallParametersCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr;
use PHPStan\Analyser\MutatingScope;
use PHPStan\Analyser\Scope;
use PHPStan\Php\PhpVersion;
use PHPStan\Reflection\ParameterReflection;
Expand Down Expand Up @@ -79,7 +80,7 @@ public function check(
$functionParametersMaxCount = -1;
}

/** @var array<int, array{Expr, Type, bool, string|null, int}> $arguments */
/** @var array<int, array{Expr, Type|null, bool, string|null, int}> $arguments */
$arguments = [];
/** @var array<int, Node\Arg> $args */
$args = $funcCall->getArgs();
Expand Down Expand Up @@ -172,7 +173,7 @@ public function check(

$arguments[] = [
$arg->value,
$type,
null,
false,
$argumentName,
$arg->getStartLine(),
Expand Down Expand Up @@ -277,6 +278,17 @@ public function check(
continue;
}

if ($argumentValueType === null) {
if ($scope instanceof MutatingScope) {
$scope = $scope->pushInFunctionCall(null, $parameter);
}
$argumentValueType = $scope->getType($argumentValue);

if ($scope instanceof MutatingScope) {
$scope = $scope->popInFunctionCall();
}
}

if ($this->checkArgumentTypes) {
$parameterType = TypeUtils::resolveLateResolvableTypes($parameter->getType());

Expand Down Expand Up @@ -464,8 +476,8 @@ static function (Type $type, callable $traverse) use (&$returnTemplateTypes): Ty
}

/**
* @param array<int, array{Expr, Type, bool, string|null, int}> $arguments
* @return array{list<IdentifierRuleError>, array<int, array{Expr, Type, bool, (string|null), int, (ParameterReflection|null), (ParameterReflection|null)}>}
* @param array<int, array{Expr, Type|null, bool, string|null, int}> $arguments
* @return array{list<IdentifierRuleError>, array<int, array{Expr, Type|null, bool, (string|null), int, (ParameterReflection|null), (ParameterReflection|null)}>}
*/
private function processArguments(
ParametersAcceptor $parametersAcceptor,
Expand Down
38 changes: 19 additions & 19 deletions tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public function testCallToFunctionWithoutParameters(): void
public function testCallToFunctionWithIncorrectParameters(): void
{
$setErrorHandlerError = PHP_VERSION_ID < 80000
? 'Parameter #1 $callback of function set_error_handler expects (callable(int, string, string, int, array): bool)|null, Closure(mixed, mixed, mixed, mixed): void given.'
: 'Parameter #1 $callback of function set_error_handler expects (callable(int, string, string, int): bool)|null, Closure(mixed, mixed, mixed, mixed): void given.';
? 'Parameter #1 $callback of function set_error_handler expects (callable(int, string, string, int, array): bool)|null, Closure(int, string, string, int): void given.'
: 'Parameter #1 $callback of function set_error_handler expects (callable(int, string, string, int): bool)|null, Closure(int, string, string, int): void given.';

require_once __DIR__ . '/data/incorrect-call-to-function-definition.php';
$this->analyse([__DIR__ . '/data/incorrect-call-to-function.php'], [
Expand Down Expand Up @@ -558,14 +558,14 @@ public function testArrayReduceCallback(): void
5,
],
[
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(string, int): non-empty-string given.',
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(non-empty-string, 1|2|3): non-falsy-string given.',
13,
'Type string of parameter #1 $foo of passed callable needs to be same or wider than parameter type string|null of accepting callable.',
'Type non-empty-string of parameter #1 $foo of passed callable needs to be same or wider than parameter type non-empty-string|null of accepting callable.',
],
[
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(string, int): non-empty-string given.',
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(non-empty-string, 1|2|3): non-falsy-string given.',
22,
'Type string of parameter #1 $foo of passed callable needs to be same or wider than parameter type string|null of accepting callable.',
'Type non-empty-string of parameter #1 $foo of passed callable needs to be same or wider than parameter type non-empty-string|null of accepting callable.',
],
]);
}
Expand All @@ -578,14 +578,14 @@ public function testArrayReduceArrowFunctionCallback(): void
5,
],
[
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(string, int): non-empty-string given.',
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(non-empty-string, 1|2|3): non-falsy-string given.',
11,
'Type string of parameter #1 $foo of passed callable needs to be same or wider than parameter type string|null of accepting callable.',
'Type non-empty-string of parameter #1 $foo of passed callable needs to be same or wider than parameter type non-empty-string|null of accepting callable.',
],
[
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(string, int): non-empty-string given.',
'Parameter #2 $callback of function array_reduce expects callable(non-empty-string|null, 1|2|3): (non-empty-string|null), Closure(non-empty-string, 1|2|3): non-falsy-string given.',
18,
'Type string of parameter #1 $foo of passed callable needs to be same or wider than parameter type string|null of accepting callable.',
'Type non-empty-string of parameter #1 $foo of passed callable needs to be same or wider than parameter type non-empty-string|null of accepting callable.',
],
]);
}
Expand All @@ -598,11 +598,11 @@ public function testArrayWalkCallback(): void
6,
],
[
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\', \'extra\'): mixed, Closure(int, string, int): \'\' given.',
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\', \'extra\'): mixed, Closure(1|2, \'bar\'|\'foo\', int): \'\' given.',
14,
],
[
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\'): mixed, Closure(int, string, int): \'\' given.',
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\'): mixed, Closure(1|2, \'bar\'|\'foo\', int): \'\' given.',
23,
'Parameter #3 $extra of passed callable is required but accepting callable does not have that parameter. It will be called without it.',
],
Expand All @@ -617,11 +617,11 @@ public function testArrayWalkArrowFunctionCallback(): void
6,
],
[
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\', \'extra\'): mixed, Closure(int, string, int): \'\' given.',
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\', \'extra\'): mixed, Closure(1|2, \'bar\'|\'foo\', int): \'\' given.',
12,
],
[
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\'): mixed, Closure(int, string, int): \'\' given.',
'Parameter #2 $callback of function array_walk expects callable(1|2, \'bar\'|\'foo\'): mixed, Closure(1|2, \'bar\'|\'foo\', int): \'\' given.',
19,
'Parameter #3 $extra of passed callable is required but accepting callable does not have that parameter. It will be called without it.',
],
Expand All @@ -636,7 +636,7 @@ public function testArrayUdiffCallback(): void
6,
],
[
'Parameter #3 $data_comp_func of function array_udiff expects callable(1|2|3|4|5|6, 1|2|3|4|5|6): int, Closure(int, int): non-falsy-string given.',
'Parameter #3 $data_comp_func of function array_udiff expects callable(1|2|3|4|5|6, 1|2|3|4|5|6): int, Closure(1|2|3|4|5|6, 1|2|3|4|5|6): (literal-string&non-falsy-string) given.',
14,
],
[
Expand Down Expand Up @@ -666,7 +666,7 @@ public function testPregReplaceCallback(): void
13,
],
[
'Parameter #2 $callback of function preg_replace_callback expects callable(array<int|string, string>): string, Closure(array): void given.',
'Parameter #2 $callback of function preg_replace_callback expects callable(array<int|string, string>): string, Closure(array<int|string, string>): void given.',
20,
],
[
Expand All @@ -688,7 +688,7 @@ public function testMbEregReplaceCallback(): void
13,
],
[
'Parameter #2 $callback of function mb_ereg_replace_callback expects callable(array<int|string, string>): string, Closure(array): void given.',
'Parameter #2 $callback of function mb_ereg_replace_callback expects callable(array<int|string, string>): string, Closure(array<int|string, string>): void given.',
20,
],
[
Expand Down Expand Up @@ -1617,11 +1617,11 @@ public function testDiscussion10454(): void
{
$this->analyse([__DIR__ . '/data/discussion-10454.php'], [
[
"Parameter #2 \$callback of function array_filter expects (callable('bar'|'baz'|'foo'|'quux'|'qux'): bool)|null, Closure(string): stdClass given.",
"Parameter #2 \$callback of function array_filter expects (callable('bar'|'baz'|'foo'|'quux'|'qux'): bool)|null, Closure('bar'|'baz'|'foo'|'quux'|'qux'): stdClass given.",
13,
],
[
"Parameter #2 \$callback of function array_filter expects (callable('bar'|'baz'|'foo'|'quux'|'qux'): bool)|null, Closure(string): stdClass given.",
"Parameter #2 \$callback of function array_filter expects (callable('bar'|'baz'|'foo'|'quux'|'qux'): bool)|null, Closure('bar'|'baz'|'foo'|'quux'|'qux'): stdClass given.",
23,
],
]);
Expand Down
14 changes: 12 additions & 2 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ public function testCallMethods(): void
1512,
],
[
'Parameter #1 $a of method Test\\CallableWithMixedArray::doBar() expects callable(array<string>): array<string>, Closure(array): (array{\'foo\'}|null) given.',
'Parameter #1 $a of method Test\\CallableWithMixedArray::doBar() expects callable(array<string>): array<string>, Closure(array<string>): (array{\'foo\'}|null) given.',
1533,
],
[
Expand Down Expand Up @@ -828,7 +828,7 @@ public function testCallMethodsOnThisOnly(): void
1512,
],
[
'Parameter #1 $a of method Test\\CallableWithMixedArray::doBar() expects callable(array<string>): array<string>, Closure(array): (array{\'foo\'}|null) given.',
'Parameter #1 $a of method Test\\CallableWithMixedArray::doBar() expects callable(array<string>): array<string>, Closure(array<string>): (array{\'foo\'}|null) given.',
1533,
],
[
Expand Down Expand Up @@ -3300,4 +3300,14 @@ public function testPureCallable(): void
]);
}

public function testClosureParameterGenerics(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->checkExplicitMixed = true;

$this->analyse([__DIR__ . '/data/closure-parameter-generics.php'], []);
}

}
38 changes: 38 additions & 0 deletions tests/PHPStan/Rules/Methods/data/closure-parameter-generics.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

namespace ClosureParameterGenerics;

use Closure;

class Transaction
{

}

class RetryableTransaction extends Transaction
{

}

class Foo
{

/**
* @param Closure(RetryableTransaction $transaction): T $callback
* @return T
*
* @template T
*/
public function retryableTransaction(Closure $callback)
{

}

public function doFoo(): void
{
$this->retryableTransaction(function (Transaction $tr) {
return $tr;
});
}

}

0 comments on commit da4fd7a

Please sign in to comment.