From acb06550497f87e4c7a42cb268f85274fbe240a8 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Wed, 28 Aug 2024 15:38:44 +0200 Subject: [PATCH 1/2] Detect function variadic-ness within top-level function_exists() --- src/Reflection/Php/PhpFunctionReflection.php | 46 ++++++------ .../CallToFunctionParametersRuleTest.php | 19 +++++ .../Rules/Functions/data/bug-11559.php | 41 +++++++++++ .../Rules/Functions/data/bug-11559b.php | 70 +++++++++++++++++++ 4 files changed, 152 insertions(+), 24 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/data/bug-11559.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-11559b.php diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index 5303d38b0f..10fb92ffa3 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -3,10 +3,7 @@ namespace PHPStan\Reflection\Php; use PhpParser\Node; -use PhpParser\Node\Stmt\ClassLike; -use PhpParser\Node\Stmt\Declare_; use PhpParser\Node\Stmt\Function_; -use PhpParser\Node\Stmt\Namespace_; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionFunction; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter; use PHPStan\Cache\Cache; @@ -27,6 +24,7 @@ use function array_key_exists; use function array_map; use function filemtime; +use function is_array; use function is_file; use function sprintf; use function time; @@ -149,12 +147,12 @@ private function isVariadic(): bool if ($modifiedTime === false) { $modifiedTime = time(); } - $variableCacheKey = sprintf('%d-v3', $modifiedTime); + $variableCacheKey = sprintf('%d-v4', $modifiedTime); $key = sprintf('variadic-function-%s-%s', $functionName, $fileName); $cachedResult = $this->cache->load($key, $variableCacheKey); if ($cachedResult === null) { $nodes = $this->parser->parseFile($fileName); - $result = $this->callsFuncGetArgs($nodes); + $result = !$this->callsFuncGetArgs($nodes)->no(); $this->cache->save($key, $variableCacheKey, $result); return $result; } @@ -167,41 +165,41 @@ private function isVariadic(): bool } /** - * @param Node[] $nodes + * @param Node[]|scalar[]|Node $node */ - private function callsFuncGetArgs(array $nodes): bool + private function callsFuncGetArgs(array|Node $node): TrinaryLogic { - foreach ($nodes as $node) { + $result = TrinaryLogic::createMaybe(); + + if ($node instanceof Node) { if ($node instanceof Function_) { $functionName = (string) $node->namespacedName; if ($functionName === $this->reflection->getName()) { - return $this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null; + // native variadic with ...$param is checked via ReflectionFunction->isVariadic() + return TrinaryLogic::createFromBoolean($this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null); } - - continue; - } - - if ($node instanceof ClassLike) { - continue; } - if ($node instanceof Namespace_) { - if ($this->callsFuncGetArgs($node->stmts)) { - return true; + foreach ($node->getSubNodeNames() as $subNodeName) { + $innerNode = $node->{$subNodeName}; + if (!$innerNode instanceof Node && !is_array($innerNode)) { + continue; } - } - if (!$node instanceof Declare_ || $node->stmts === null) { - continue; + $result = $result->and($this->callsFuncGetArgs($innerNode)); } + } elseif (is_array($node)) { + foreach ($node as $subNode) { + if (!$subNode instanceof Node) { + continue; + } - if ($this->callsFuncGetArgs($node->stmts)) { - return true; + $result = $result->and($this->callsFuncGetArgs($subNode)); } } - return false; + return $result; } private function getReturnType(): Type diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 49b56548cc..d49987e3c5 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -1742,4 +1742,23 @@ public function testBug11506(): void $this->analyse([__DIR__ . '/data/bug-11506.php'], []); } + public function testBug11559(): void + { + $this->analyse([__DIR__ . '/data/bug-11559.php'], []); + } + + public function testBug11559b(): void + { + $this->analyse([__DIR__ . '/data/bug-11559b.php'], [ + [ + 'Function Bug11559b\maybe_variadic_fn invoked with 5 parameters, 0 required.', + 14, + ], + [ + 'Function Bug11559b\maybe_variadic_fn4 invoked with 2 parameters, 0 required.', + 65, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-11559.php b/tests/PHPStan/Rules/Functions/data/bug-11559.php new file mode 100644 index 0000000000..05e4d85fff --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-11559.php @@ -0,0 +1,41 @@ + Date: Mon, 2 Sep 2024 21:25:59 +0200 Subject: [PATCH 2/2] Fix for variadic params --- src/Reflection/Php/PhpFunctionReflection.php | 26 ++++++++++++++----- .../Rules/Functions/data/bug-11559b.php | 12 +++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index 10fb92ffa3..a52f3829f8 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -152,7 +152,7 @@ private function isVariadic(): bool $cachedResult = $this->cache->load($key, $variableCacheKey); if ($cachedResult === null) { $nodes = $this->parser->parseFile($fileName); - $result = !$this->callsFuncGetArgs($nodes)->no(); + $result = !$this->containsVariadicFunction($nodes)->no(); $this->cache->save($key, $variableCacheKey, $result); return $result; } @@ -167,7 +167,7 @@ private function isVariadic(): bool /** * @param Node[]|scalar[]|Node $node */ - private function callsFuncGetArgs(array|Node $node): TrinaryLogic + private function containsVariadicFunction(array|Node $node): TrinaryLogic { $result = TrinaryLogic::createMaybe(); @@ -176,8 +176,7 @@ private function callsFuncGetArgs(array|Node $node): TrinaryLogic $functionName = (string) $node->namespacedName; if ($functionName === $this->reflection->getName()) { - // native variadic with ...$param is checked via ReflectionFunction->isVariadic() - return TrinaryLogic::createFromBoolean($this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null); + return TrinaryLogic::createFromBoolean($this->isFunctionNodeVariadic($node)); } } @@ -187,7 +186,7 @@ private function callsFuncGetArgs(array|Node $node): TrinaryLogic continue; } - $result = $result->and($this->callsFuncGetArgs($innerNode)); + $result = $result->and($this->containsVariadicFunction($innerNode)); } } elseif (is_array($node)) { foreach ($node as $subNode) { @@ -195,7 +194,7 @@ private function callsFuncGetArgs(array|Node $node): TrinaryLogic continue; } - $result = $result->and($this->callsFuncGetArgs($subNode)); + $result = $result->and($this->containsVariadicFunction($subNode)); } } @@ -301,4 +300,19 @@ public function acceptsNamedArguments(): bool return $this->acceptsNamedArguments; } + private function isFunctionNodeVariadic(Function_ $node): bool + { + foreach ($node->params as $parameter) { + if ($parameter->variadic) { + return true; + } + } + + if ($this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null) { + return true; + } + + return false; + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-11559b.php b/tests/PHPStan/Rules/Functions/data/bug-11559b.php index 498f456d32..ff5b8a919a 100644 --- a/tests/PHPStan/Rules/Functions/data/bug-11559b.php +++ b/tests/PHPStan/Rules/Functions/data/bug-11559b.php @@ -68,3 +68,15 @@ function maybe_variadic_fn4(...$values) { function variadic_fn5(...$values) { } variadic_fn5('action','asdf'); + + +if (rand(0,1)) { + function maybe_variadic_fn6($x, $y): void { + } +} else if ( ! function_exists( 'maybe_variadic_fn6' ) ) { + function maybe_variadic_fn6($y, ...$values): void { + } +} + +maybe_variadic_fn6('action','asdf'); +