From c612c94c6a4b5a4df36ebdd096187aa174736a47 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Thu, 2 Jan 2025 02:31:47 +0900 Subject: [PATCH 1/6] Fix analysis on `array_map` with named arguments --- src/Parser/ArrayMapArgVisitor.php | 46 ++++++++++++++----- .../CallToFunctionParametersRuleTest.php | 11 +++++ .../Rules/Functions/data/bug-12317.php | 26 +++++++++++ 3 files changed, 71 insertions(+), 12 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-12317.php diff --git a/src/Parser/ArrayMapArgVisitor.php b/src/Parser/ArrayMapArgVisitor.php index a495d6e770..9fe01290bd 100644 --- a/src/Parser/ArrayMapArgVisitor.php +++ b/src/Parser/ArrayMapArgVisitor.php @@ -6,7 +6,7 @@ use PhpParser\Node; use PhpParser\NodeVisitorAbstract; use PHPStan\DependencyInjection\AutowiredService; -use function array_slice; +use function array_splice; use function count; #[AutowiredService] @@ -18,19 +18,41 @@ final class ArrayMapArgVisitor extends NodeVisitorAbstract #[Override] public function enterNode(Node $node): ?Node { - if ($node instanceof Node\Expr\FuncCall && $node->name instanceof Node\Name && !$node->isFirstClassCallable()) { - $functionName = $node->name->toLowerString(); - if ($functionName === 'array_map') { - $args = $node->getArgs(); - if (isset($args[0])) { - $slicedArgs = array_slice($args, 1); - if (count($slicedArgs) > 0) { - $args[0]->value->setAttribute(self::ATTRIBUTE_NAME, $slicedArgs); - } - } - } + if (!$this->isArrayMapCall($node)) { + return null; } + + $args = $node->getArgs(); + if (count($args) < 2) { + return null; + } + + $callbackPos = 0; + if ($args[1]->name !== null && $args[1]->name->name === 'callback') { + $callbackPos = 1; + } + [$callback] = array_splice($args, $callbackPos, 1); + $callback->value->setAttribute(self::ATTRIBUTE_NAME, $args); + return null; } + /** + * @phpstan-assert-if-true Node\Expr\FuncCall $node + */ + private function isArrayMapCall(Node $node): bool + { + if (!$node instanceof Node\Expr\FuncCall) { + return false; + } + if (!$node->name instanceof Node\Name) { + return false; + } + if ($node->isFirstClassCallable()) { + return false; + } + + return $node->name->toLowerString() === 'array_map'; + } + } diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index aaba0745cc..2705cd75e4 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2182,4 +2182,15 @@ public function testBug13065(): void $this->analyse([__DIR__ . '/data/bug-13065.php'], $errors); } + public function testBug12317(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $this->checkExplicitMixed = true; + $this->checkImplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-12317.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-12317.php b/tests/PHPStan/Rules/Functions/data/bug-12317.php new file mode 100644 index 0000000000..798d244442 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-12317.php @@ -0,0 +1,26 @@ +uuid; } +} + +class HelloWorld +{ + /** + * @param list $a + * + * @return list + */ + public function sayHello(array $a): array + { + $b = array_map( + array: $a, + callback: static fn(Uuid $c): string => (string) $c, + ); + + return $b; + } +} From 46cd1a629ae8712b97398a7d788f58f7f0921194 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Thu, 2 Jan 2025 09:56:51 +0900 Subject: [PATCH 2/6] More tests for `bug-12317.php` --- .../PHPStan/Rules/Functions/data/bug-12317.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/PHPStan/Rules/Functions/data/bug-12317.php b/tests/PHPStan/Rules/Functions/data/bug-12317.php index 798d244442..f64e154b4a 100644 --- a/tests/PHPStan/Rules/Functions/data/bug-12317.php +++ b/tests/PHPStan/Rules/Functions/data/bug-12317.php @@ -10,17 +10,16 @@ public function __toString() { return $this->uuid; } class HelloWorld { /** - * @param list $a - * - * @return list + * @param list $arr */ - public function sayHello(array $a): array + public function sayHello(array $arr): void { - $b = array_map( - array: $a, - callback: static fn(Uuid $c): string => (string) $c, - ); + $callback = static fn(Uuid $uuid): string => (string) $uuid; - return $b; + array_map(array: $arr, callback: $callback); + array_map(callback: $callback, array: $arr); + array_map($callback, $arr); + array_map($callback, array: $arr); + array_map(static fn (Uuid $u1, Uuid $u2): string => (string) $u1, $arr, $arr); } } From bd60051050d2391d5b2fb57a4548f3b1db438060 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Thu, 2 Jan 2025 20:20:22 +0900 Subject: [PATCH 3/6] Add test cases that produce errors --- src/Reflection/ParametersAcceptorSelector.php | 4 ++++ .../Functions/CallToFunctionParametersRuleTest.php | 11 ++++++++++- tests/PHPStan/Rules/Functions/data/bug-12317.php | 6 ++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index 81bc3a2a99..c1f13ce218 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -80,6 +80,10 @@ public static function selectFromArgs( && count($parametersAcceptors) > 0 ) { $arrayMapArgs = $args[0]->value->getAttribute(ArrayMapArgVisitor::ATTRIBUTE_NAME); + if ($arrayMapArgs === null && isset($args[1])) { + // callback argument of array_map() may be the second one (named argument) + $arrayMapArgs = $args[1]->value->getAttribute(ArrayMapArgVisitor::ATTRIBUTE_NAME); + } if ($arrayMapArgs !== null) { $acceptor = $parametersAcceptors[0]; $parameters = $acceptor->getParameters(); diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 2705cd75e4..4a40af04ad 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2190,7 +2190,16 @@ public function testBug12317(): void $this->checkExplicitMixed = true; $this->checkImplicitMixed = true; - $this->analyse([__DIR__ . '/data/bug-12317.php'], []); + $this->analyse([__DIR__ . '/data/bug-12317.php'], [ + [ + 'Parameter #1 $callback of function array_map expects (callable(Bug12317\Uuid): mixed)|null, Closure(string): string given.', + 28, + ], + [ + 'Parameter $callback of function array_map expects (callable(Bug12317\Uuid): mixed)|null, Closure(string): string given.', + 29, + ], + ]); } } diff --git a/tests/PHPStan/Rules/Functions/data/bug-12317.php b/tests/PHPStan/Rules/Functions/data/bug-12317.php index f64e154b4a..2443fefc09 100644 --- a/tests/PHPStan/Rules/Functions/data/bug-12317.php +++ b/tests/PHPStan/Rules/Functions/data/bug-12317.php @@ -16,10 +16,16 @@ public function sayHello(array $arr): void { $callback = static fn(Uuid $uuid): string => (string) $uuid; + // ok array_map(array: $arr, callback: $callback); array_map(callback: $callback, array: $arr); array_map($callback, $arr); array_map($callback, array: $arr); array_map(static fn (Uuid $u1, Uuid $u2): string => (string) $u1, $arr, $arr); + + // should be reported + $invalidCallback = static fn(string $uuid): string => $uuid; + array_map($invalidCallback, $arr); + array_map(array: $arr, callback: $invalidCallback); } } From 14b19888ede240911b6a307451c05ac6c5d8a53e Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Thu, 2 Jan 2025 20:36:57 +0900 Subject: [PATCH 4/6] Refactor --- src/Parser/ArrayMapArgVisitor.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Parser/ArrayMapArgVisitor.php b/src/Parser/ArrayMapArgVisitor.php index 9fe01290bd..884338d9df 100644 --- a/src/Parser/ArrayMapArgVisitor.php +++ b/src/Parser/ArrayMapArgVisitor.php @@ -31,8 +31,10 @@ public function enterNode(Node $node): ?Node if ($args[1]->name !== null && $args[1]->name->name === 'callback') { $callbackPos = 1; } - [$callback] = array_splice($args, $callbackPos, 1); - $callback->value->setAttribute(self::ATTRIBUTE_NAME, $args); + $callbackArg = $args[$callbackPos]; + $arrayArgs = $args; + array_splice($arrayArgs, $callbackPos, 1); + $callbackArg->value->setAttribute(self::ATTRIBUTE_NAME, $arrayArgs); return null; } From c5586bcccdac7c49fff401ed78aa139dbedddbf6 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Sun, 20 Jul 2025 12:26:48 +0900 Subject: [PATCH 5/6] Use `#[RequiresPhp]` --- .../Rules/Functions/CallToFunctionParametersRuleTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 4a40af04ad..82522288fd 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -2182,12 +2182,9 @@ public function testBug13065(): void $this->analyse([__DIR__ . '/data/bug-13065.php'], $errors); } + #[RequiresPhp('>= 8.0')] public function testBug12317(): void { - if (PHP_VERSION_ID < 80000) { - $this->markTestSkipped('Test requires PHP 8.0.'); - } - $this->checkExplicitMixed = true; $this->checkImplicitMixed = true; $this->analyse([__DIR__ . '/data/bug-12317.php'], [ From 32edfb35ca706c4be499e283c75a480c0c78cca9 Mon Sep 17 00:00:00 2001 From: Takuya Aramaki Date: Sun, 20 Jul 2025 16:20:31 +0900 Subject: [PATCH 6/6] Normalize `array_map` args in `ArrayMapArgVisitor` --- src/Parser/ArrayMapArgVisitor.php | 32 ++++++++++++++----- src/Reflection/ParametersAcceptorSelector.php | 4 --- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/Parser/ArrayMapArgVisitor.php b/src/Parser/ArrayMapArgVisitor.php index 884338d9df..c9e5b8a358 100644 --- a/src/Parser/ArrayMapArgVisitor.php +++ b/src/Parser/ArrayMapArgVisitor.php @@ -6,7 +6,6 @@ use PhpParser\Node; use PhpParser\NodeVisitorAbstract; use PHPStan\DependencyInjection\AutowiredService; -use function array_splice; use function count; #[AutowiredService] @@ -27,14 +26,31 @@ public function enterNode(Node $node): ?Node return null; } - $callbackPos = 0; - if ($args[1]->name !== null && $args[1]->name->name === 'callback') { - $callbackPos = 1; + $callbackArg = null; + $arrayArgs = []; + foreach ($args as $i => $arg) { + if ($callbackArg === null) { + if ($arg->name === null && $i === 0) { + $callbackArg = $arg; + continue; + } + if ($arg->name !== null && $arg->name->toString() === 'callback') { + $callbackArg = $arg; + continue; + } + } + + $arrayArgs[] = $arg; + } + + if ($callbackArg !== null) { + $callbackArg->value->setAttribute(self::ATTRIBUTE_NAME, $arrayArgs); + return new Node\Expr\FuncCall( + $node->name, + [$callbackArg, ...$arrayArgs], + $node->getAttributes(), + ); } - $callbackArg = $args[$callbackPos]; - $arrayArgs = $args; - array_splice($arrayArgs, $callbackPos, 1); - $callbackArg->value->setAttribute(self::ATTRIBUTE_NAME, $arrayArgs); return null; } diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index c1f13ce218..81bc3a2a99 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -80,10 +80,6 @@ public static function selectFromArgs( && count($parametersAcceptors) > 0 ) { $arrayMapArgs = $args[0]->value->getAttribute(ArrayMapArgVisitor::ATTRIBUTE_NAME); - if ($arrayMapArgs === null && isset($args[1])) { - // callback argument of array_map() may be the second one (named argument) - $arrayMapArgs = $args[1]->value->getAttribute(ArrayMapArgVisitor::ATTRIBUTE_NAME); - } if ($arrayMapArgs !== null) { $acceptor = $parametersAcceptors[0]; $parameters = $acceptor->getParameters();