From ea07f8c82c290905bcc9161a586958b08e71309e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 3 Oct 2024 15:55:13 +0200 Subject: [PATCH 01/15] Remove inefficient caching from PhpMethodReflection --- conf/config.neon | 5 + src/Parser/SimpleParser.php | 2 + src/Parser/VariadicMethodsVisitor.php | 94 ++++++++++++++++++ src/Reflection/Php/PhpMethodReflection.php | 100 ++++---------------- tests/PHPStan/Parser/CleaningParserTest.php | 1 + 5 files changed, 121 insertions(+), 81 deletions(-) create mode 100644 src/Parser/VariadicMethodsVisitor.php diff --git a/conf/config.neon b/conf/config.neon index 6a8f4d68970..340e7b3b3aa 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -302,6 +302,11 @@ services: tags: - phpstan.parser.richParserNodeVisitor + - + class: PHPStan\Parser\VariadicMethodsVisitor + tags: + - phpstan.parser.richParserNodeVisitor + - class: PHPStan\Node\Printer\ExprPrinter diff --git a/src/Parser/SimpleParser.php b/src/Parser/SimpleParser.php index 713c1502ef2..4afa4c76430 100644 --- a/src/Parser/SimpleParser.php +++ b/src/Parser/SimpleParser.php @@ -15,6 +15,7 @@ final class SimpleParser implements Parser public function __construct( private \PhpParser\Parser $parser, private NameResolver $nameResolver, + private VariadicMethodsVisitor $variadicMethodsVisitor, ) { } @@ -48,6 +49,7 @@ public function parseString(string $sourceCode): array $nodeTraverser = new NodeTraverser(); $nodeTraverser->addVisitor($this->nameResolver); + $nodeTraverser->addVisitor($this->variadicMethodsVisitor); /** @var array */ return $nodeTraverser->traverse($nodes); diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php new file mode 100644 index 00000000000..c3765e03100 --- /dev/null +++ b/src/Parser/VariadicMethodsVisitor.php @@ -0,0 +1,94 @@ +> */ + private array $variadicMethods = []; + + public const ATTRIBUTE_NAME = 'variadicMethods'; + + public function __construct( + private FunctionCallStatementFinder $functionCallStatementFinder, + ) + { + } + + public function beforeTraverse(array $nodes): ?array + { + $this->topNode = null; + $this->variadicMethods = []; + $this->inClassLike = null; + $this->inNamespace = null; + + return null; + } + + public function enterNode(Node $node): ?Node + { + if ($this->topNode === null) { + $this->topNode = $node; + } + + if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { + $this->inNamespace = $node->name->toString(); + } + + if ($node instanceof Node\Stmt\ClassLike && $node->name instanceof Node\Identifier) { + $this->inClassLike = $node->name->name; + } + + if ($this->inClassLike !== null && $node instanceof ClassMethod) { + if ($node->getStmts() === null) { + return null; // interface + } + + if ($this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null) { + $className = $this->inNamespace !== null ? $this->inNamespace . '\\' . $this->inClassLike : $this->inClassLike; + if (!array_key_exists($className, $this->variadicMethods)) { + $this->variadicMethods[$className] = []; + } + $this->variadicMethods[$className][] = $node->name->name; + } + } + + return null; + } + + public function leaveNode(Node $node): ?Node + { + if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { + $this->inNamespace = null; + } + + if ($node instanceof Node\Stmt\ClassLike && $node->name instanceof Node\Identifier) { + $this->inClassLike = null; + } + + return null; + } + + public function afterTraverse(array $nodes): ?array + { + if ($this->topNode !== null && $this->variadicMethods !== []) { + $this->topNode->setAttribute(self::ATTRIBUTE_NAME, $this->variadicMethods); + } + + return null; + } + +} diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index d0e008a7ed0..0307596df20 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -2,16 +2,10 @@ namespace PHPStan\Reflection\Php; -use PhpParser\Node; -use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Declare_; -use PhpParser\Node\Stmt\Function_; -use PhpParser\Node\Stmt\Namespace_; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter; -use PHPStan\Cache\Cache; -use PHPStan\Parser\FunctionCallStatementFinder; use PHPStan\Parser\Parser; +use PHPStan\Parser\VariadicMethodsVisitor; use PHPStan\Reflection\Assertions; use PHPStan\Reflection\ClassMemberReflection; use PHPStan\Reflection\ClassReflection; @@ -21,7 +15,6 @@ use PHPStan\Reflection\ExtendedParametersAcceptor; use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\Reflection\MethodPrototypeReflection; -use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Reflection\ReflectionProvider; use PHPStan\TrinaryLogic; use PHPStan\Type\ArrayType; @@ -36,14 +29,13 @@ use PHPStan\Type\TypehintHelper; use PHPStan\Type\VoidType; use ReflectionException; +use function array_key_exists; use function array_map; +use function count; use function explode; -use function filemtime; use function in_array; -use function is_bool; -use function sprintf; +use function is_array; use function strtolower; -use function time; use const PHP_VERSION_ID; /** @@ -62,6 +54,8 @@ final class PhpMethodReflection implements ExtendedMethodReflection /** @var list|null */ private ?array $variants = null; + private ?bool $containsVariadicCalls = null; + /** * @param Type[] $phpDocParameterTypes * @param Type[] $phpDocParameterOutTypes @@ -75,8 +69,6 @@ public function __construct( private ReflectionMethod $reflection, private ReflectionProvider $reflectionProvider, private Parser $parser, - private FunctionCallStatementFinder $functionCallStatementFinder, - private Cache $cache, private TemplateTypeMap $templateTypeMap, private array $phpDocParameterTypes, private ?Type $phpDocReturnType, @@ -253,81 +245,27 @@ private function isVariadic(): bool } if (!$isNativelyVariadic && $filename !== null) { - $modifiedTime = @filemtime($filename); - if ($modifiedTime === false) { - $modifiedTime = time(); - } - $key = sprintf('variadic-method-%s-%s-%s', $declaringClass->getName(), $this->reflection->getName(), $filename); - $variableCacheKey = sprintf('%d-v4', $modifiedTime); - $cachedResult = $this->cache->load($key, $variableCacheKey); - if ($cachedResult === null || !is_bool($cachedResult)) { - $nodes = $this->parser->parseFile($filename); - $result = $this->callsFuncGetArgs($declaringClass, $nodes); - $this->cache->save($key, $variableCacheKey, $result); - return $result; - } - - return $cachedResult; - } - - return $isNativelyVariadic; - } - - /** - * @param Node[] $nodes - */ - private function callsFuncGetArgs(ClassReflection $declaringClass, array $nodes): bool - { - foreach ($nodes as $node) { - if ( - $node instanceof Node\Stmt\ClassLike - ) { - if (!isset($node->namespacedName)) { - continue; - } - if ($declaringClass->getName() !== (string) $node->namespacedName) { - continue; - } - if ($this->callsFuncGetArgs($declaringClass, $node->stmts)) { - return true; - } - continue; + if ($this->containsVariadicCalls !== null) { + return $this->containsVariadicCalls; } - if ($node instanceof ClassMethod) { - if ($node->getStmts() === null) { - continue; // interface - } + $nodes = $this->parser->parseFile($filename); + if (count($nodes) > 0) { + $variadicMethods = $nodes[0]->getAttribute(VariadicMethodsVisitor::ATTRIBUTE_NAME); - $methodName = $node->name->name; - if ($methodName === $this->reflection->getName()) { - return $this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null; + if ( + is_array($variadicMethods) + && array_key_exists($declaringClass->getName(), $variadicMethods) + && in_array($this->reflection->getName(), $variadicMethods[$declaringClass->getName()], true) + ) { + return $this->containsVariadicCalls = true; } - - continue; } - if ($node instanceof Function_) { - continue; - } - - if ($node instanceof Namespace_) { - if ($this->callsFuncGetArgs($declaringClass, $node->stmts)) { - return true; - } - continue; - } - - if (!$node instanceof Declare_ || $node->stmts === null) { - continue; - } - - if ($this->callsFuncGetArgs($declaringClass, $node->stmts)) { - return true; - } + return $this->containsVariadicCalls = false; } - return false; + return $isNativelyVariadic; } public function isPrivate(): bool diff --git a/tests/PHPStan/Parser/CleaningParserTest.php b/tests/PHPStan/Parser/CleaningParserTest.php index 692ea7b6995..2a025f1ea79 100644 --- a/tests/PHPStan/Parser/CleaningParserTest.php +++ b/tests/PHPStan/Parser/CleaningParserTest.php @@ -68,6 +68,7 @@ public function testParse( new SimpleParser( new Php7(new Emulative()), new NameResolver(), + new VariadicMethodsVisitor(new FunctionCallStatementFinder()), ), new PhpVersion($phpVersionId), ); From da0aac636c51368c8851ca22f9f3d2ae2bcf944c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 3 Oct 2024 20:49:17 +0200 Subject: [PATCH 02/15] Remove inefficient caching from PhpFunctionReflection --- conf/config.neon | 5 ++ src/Parser/SimpleParser.php | 2 + src/Parser/VariadicFunctionsVisitor.php | 80 +++++++++++++++++ src/Reflection/Php/PhpFunctionReflection.php | 95 ++++---------------- tests/PHPStan/Parser/CleaningParserTest.php | 1 + 5 files changed, 106 insertions(+), 77 deletions(-) create mode 100644 src/Parser/VariadicFunctionsVisitor.php diff --git a/conf/config.neon b/conf/config.neon index 340e7b3b3aa..b0c631acdbd 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -307,6 +307,11 @@ services: tags: - phpstan.parser.richParserNodeVisitor + - + class: PHPStan\Parser\VariadicFunctionsVisitor + tags: + - phpstan.parser.richParserNodeVisitor + - class: PHPStan\Node\Printer\ExprPrinter diff --git a/src/Parser/SimpleParser.php b/src/Parser/SimpleParser.php index 4afa4c76430..8fbd1127420 100644 --- a/src/Parser/SimpleParser.php +++ b/src/Parser/SimpleParser.php @@ -16,6 +16,7 @@ public function __construct( private \PhpParser\Parser $parser, private NameResolver $nameResolver, private VariadicMethodsVisitor $variadicMethodsVisitor, + private VariadicFunctionsVisitor $variadicFunctionsVisitor, ) { } @@ -50,6 +51,7 @@ public function parseString(string $sourceCode): array $nodeTraverser = new NodeTraverser(); $nodeTraverser->addVisitor($this->nameResolver); $nodeTraverser->addVisitor($this->variadicMethodsVisitor); + $nodeTraverser->addVisitor($this->variadicFunctionsVisitor); /** @var array */ return $nodeTraverser->traverse($nodes); diff --git a/src/Parser/VariadicFunctionsVisitor.php b/src/Parser/VariadicFunctionsVisitor.php new file mode 100644 index 00000000000..2a810e43036 --- /dev/null +++ b/src/Parser/VariadicFunctionsVisitor.php @@ -0,0 +1,80 @@ + */ + private array $variadicFunctions = []; + + public const ATTRIBUTE_NAME = 'variadicFunctions'; + + public function __construct( + private FunctionCallStatementFinder $functionCallStatementFinder, + ) + { + } + + public function beforeTraverse(array $nodes): ?array + { + $this->topNode = null; + $this->variadicFunctions = []; + $this->inNamespace = null; + + return null; + } + + public function enterNode(Node $node): ?Node + { + if ($this->topNode === null) { + $this->topNode = $node; + } + + if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { + $this->inNamespace = $node->name->toString(); + } + + if ($node instanceof Node\Stmt\Function_) { + $functionName = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name; + + if (!array_key_exists($functionName, $this->variadicFunctions)) { + $this->variadicFunctions[$functionName] = TrinaryLogic::createMaybe(); + } + + $isVariadic = $this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null; + $this->variadicFunctions[$functionName] = $this->variadicFunctions[$functionName]->and(TrinaryLogic::createFromBoolean($isVariadic)); + } + + return null; + } + + public function leaveNode(Node $node): ?Node + { + if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { + $this->inNamespace = null; + } + + return null; + } + + public function afterTraverse(array $nodes): ?array + { + if ($this->topNode !== null && $this->variadicFunctions !== []) { + $this->topNode->setAttribute(self::ATTRIBUTE_NAME, $this->variadicFunctions); + } + + return null; + } + +} diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index e28f19f8792..00979c4e1b5 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -2,20 +2,16 @@ namespace PHPStan\Reflection\Php; -use PhpParser\Node; -use PhpParser\Node\Stmt\Function_; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionFunction; use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter; -use PHPStan\Cache\Cache; -use PHPStan\Parser\FunctionCallStatementFinder; use PHPStan\Parser\Parser; +use PHPStan\Parser\VariadicFunctionsVisitor; use PHPStan\Reflection\Assertions; use PHPStan\Reflection\ExtendedFunctionVariant; use PHPStan\Reflection\ExtendedParameterReflection; use PHPStan\Reflection\ExtendedParametersAcceptor; use PHPStan\Reflection\FunctionReflection; use PHPStan\Reflection\InitializerExprTypeResolver; -use PHPStan\Reflection\ParametersAcceptor; use PHPStan\TrinaryLogic; use PHPStan\Type\Generic\TemplateTypeMap; use PHPStan\Type\MixedType; @@ -23,11 +19,9 @@ use PHPStan\Type\TypehintHelper; use function array_key_exists; use function array_map; -use function filemtime; +use function count; use function is_array; use function is_file; -use function sprintf; -use function time; final class PhpFunctionReflection implements FunctionReflection { @@ -35,6 +29,8 @@ final class PhpFunctionReflection implements FunctionReflection /** @var list|null */ private ?array $variants = null; + private ?bool $containsVariadicCalls = null; + /** * @param array $phpDocParameterTypes * @param array $phpDocParameterOutTypes @@ -45,8 +41,6 @@ public function __construct( private InitializerExprTypeResolver $initializerExprTypeResolver, private ReflectionFunction $reflection, private Parser $parser, - private FunctionCallStatementFinder $functionCallStatementFinder, - private Cache $cache, private TemplateTypeMap $templateTypeMap, private array $phpDocParameterTypes, private ?Type $phpDocReturnType, @@ -139,67 +133,29 @@ private function getParameters(): array private function isVariadic(): bool { $isNativelyVariadic = $this->reflection->isVariadic(); - if (!$isNativelyVariadic && $this->reflection - ->getFileName() !== false) { - $fileName = $this->reflection->getFileName(); - if (is_file($fileName)) { - $functionName = $this->reflection->getName(); - $modifiedTime = filemtime($fileName); - if ($modifiedTime === false) { - $modifiedTime = time(); - } - $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->containsVariadicFunction($nodes)->no(); - $this->cache->save($key, $variableCacheKey, $result); - return $result; - } + if (!$isNativelyVariadic && $this->reflection->getFileName() !== false) { + $filename = $this->reflection->getFileName(); - return $cachedResult; + if ($this->containsVariadicCalls !== null) { + return $this->containsVariadicCalls; } - } - - return $isNativelyVariadic; - } - /** - * @param Node[]|scalar[]|Node $node - */ - private function containsVariadicFunction(array|Node $node): TrinaryLogic - { - $result = TrinaryLogic::createMaybe(); - - if ($node instanceof Node) { - if ($node instanceof Function_) { - $functionName = (string) $node->namespacedName; - - if ($functionName === $this->reflection->getName()) { - return TrinaryLogic::createFromBoolean($this->isFunctionNodeVariadic($node)); - } - } + $nodes = $this->parser->parseFile($filename); + if (count($nodes) > 0) { + $variadicFunctions = $nodes[0]->getAttribute(VariadicFunctionsVisitor::ATTRIBUTE_NAME); - foreach ($node->getSubNodeNames() as $subNodeName) { - $innerNode = $node->{$subNodeName}; - if (!$innerNode instanceof Node && !is_array($innerNode)) { - continue; + if ( + is_array($variadicFunctions) + && array_key_exists($this->reflection->getName(), $variadicFunctions) + ) { + return $this->containsVariadicCalls = !$variadicFunctions[$this->reflection->getName()]->no(); } - - $result = $result->and($this->containsVariadicFunction($innerNode)); } - } elseif (is_array($node)) { - foreach ($node as $subNode) { - if (!$subNode instanceof Node) { - continue; - } - $result = $result->and($this->containsVariadicFunction($subNode)); - } + return $this->containsVariadicCalls = false; } - return $result; + return $isNativelyVariadic; } private function getReturnType(): Type @@ -296,19 +252,4 @@ public function acceptsNamedArguments(): TrinaryLogic return TrinaryLogic::createFromBoolean($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/Parser/CleaningParserTest.php b/tests/PHPStan/Parser/CleaningParserTest.php index 2a025f1ea79..4bbba722ed1 100644 --- a/tests/PHPStan/Parser/CleaningParserTest.php +++ b/tests/PHPStan/Parser/CleaningParserTest.php @@ -69,6 +69,7 @@ public function testParse( new Php7(new Emulative()), new NameResolver(), new VariadicMethodsVisitor(new FunctionCallStatementFinder()), + new VariadicFunctionsVisitor(new FunctionCallStatementFinder()), ), new PhpVersion($phpVersionId), ); From 9a027751758fabe26bc110d1cf4a6c10b9fae417 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 4 Oct 2024 12:20:42 +0200 Subject: [PATCH 03/15] Remove FunctionCallStatementFinder --- conf/config.neon | 3 -- src/Parser/FunctionCallStatementFinder.php | 47 --------------------- src/Parser/VariadicFunctionsVisitor.php | 46 ++++++++++++++------ src/Parser/VariadicMethodsVisitor.php | 46 +++++++++++++------- src/Reflection/Php/PhpMethodReflection.php | 4 +- tests/PHPStan/Parser/CleaningParserTest.php | 4 +- 6 files changed, 68 insertions(+), 82 deletions(-) delete mode 100644 src/Parser/FunctionCallStatementFinder.php diff --git a/conf/config.neon b/conf/config.neon index b0c631acdbd..583cceac81e 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -644,9 +644,6 @@ services: tags: - phpstan.diagnoseExtension - - - class: PHPStan\Parser\FunctionCallStatementFinder - - class: PHPStan\Process\CpuCoreCounter diff --git a/src/Parser/FunctionCallStatementFinder.php b/src/Parser/FunctionCallStatementFinder.php deleted file mode 100644 index 9a4c1dd6bbf..00000000000 --- a/src/Parser/FunctionCallStatementFinder.php +++ /dev/null @@ -1,47 +0,0 @@ -findFunctionCallInStatements($functionNames, $statement); - if ($result !== null) { - return $result; - } - } - - if (!($statement instanceof Node)) { - continue; - } - - if ($statement instanceof FuncCall && $statement->name instanceof Name) { - if (in_array((string) $statement->name, $functionNames, true)) { - return $statement; - } - } - - $result = $this->findFunctionCallInStatements($functionNames, $statement); - if ($result !== null) { - return $result; - } - } - - return null; - } - -} diff --git a/src/Parser/VariadicFunctionsVisitor.php b/src/Parser/VariadicFunctionsVisitor.php index 2a810e43036..0b4bceb0020 100644 --- a/src/Parser/VariadicFunctionsVisitor.php +++ b/src/Parser/VariadicFunctionsVisitor.php @@ -3,10 +3,12 @@ namespace PHPStan\Parser; use PhpParser\Node; +use PhpParser\Node\Name; use PhpParser\NodeVisitorAbstract; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\TrinaryLogic; use function array_key_exists; +use function in_array; final class VariadicFunctionsVisitor extends NodeVisitorAbstract { @@ -15,22 +17,19 @@ final class VariadicFunctionsVisitor extends NodeVisitorAbstract private ?string $inNamespace = null; + private ?string $inFunction = null; + /** @var array */ private array $variadicFunctions = []; public const ATTRIBUTE_NAME = 'variadicFunctions'; - public function __construct( - private FunctionCallStatementFinder $functionCallStatementFinder, - ) - { - } - public function beforeTraverse(array $nodes): ?array { $this->topNode = null; $this->variadicFunctions = []; $this->inNamespace = null; + $this->inFunction = null; return null; } @@ -46,14 +45,32 @@ public function enterNode(Node $node): ?Node } if ($node instanceof Node\Stmt\Function_) { - $functionName = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name; - - if (!array_key_exists($functionName, $this->variadicFunctions)) { - $this->variadicFunctions[$functionName] = TrinaryLogic::createMaybe(); + $this->inFunction = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name; + + foreach ($node->params as $parameter) { + if (!$parameter->variadic) { + continue; + } + + if (!array_key_exists($this->inFunction, $this->variadicFunctions)) { + $this->variadicFunctions[$this->inFunction] = TrinaryLogic::createYes(); + } else { + $this->variadicFunctions[$this->inFunction]->and(TrinaryLogic::createYes()); + } } + } - $isVariadic = $this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null; - $this->variadicFunctions[$functionName] = $this->variadicFunctions[$functionName]->and(TrinaryLogic::createFromBoolean($isVariadic)); + if ( + $this->inFunction !== null + && $node instanceof Node\Expr\FuncCall + && $node->name instanceof Name + && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) + ) { + if (!array_key_exists($this->inFunction, $this->variadicFunctions)) { + $this->variadicFunctions[$this->inFunction] = TrinaryLogic::createYes(); + } else { + $this->variadicFunctions[$this->inFunction]->and(TrinaryLogic::createYes()); + } } return null; @@ -65,6 +82,11 @@ public function leaveNode(Node $node): ?Node $this->inNamespace = null; } + if ($node instanceof Node\Stmt\Function_ && $this->inFunction !== null) { + $this->variadicFunctions[$this->inFunction] ??= TrinaryLogic::createNo(); + $this->inFunction = null; + } + return null; } diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index c3765e03100..8691b5b3315 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -3,37 +3,37 @@ namespace PHPStan\Parser; use PhpParser\Node; +use PhpParser\Node\Name; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\NodeVisitorAbstract; use PHPStan\Reflection\ParametersAcceptor; +use PHPStan\TrinaryLogic; use function array_key_exists; +use function in_array; final class VariadicMethodsVisitor extends NodeVisitorAbstract { private ?Node $topNode = null; + private ?string $inNamespace = null; + private ?string $inClassLike = null; - private ?string $inNamespace = null; + private ?string $inMethod = null; - /** @var array> */ + /** @var array> */ private array $variadicMethods = []; public const ATTRIBUTE_NAME = 'variadicMethods'; - public function __construct( - private FunctionCallStatementFinder $functionCallStatementFinder, - ) - { - } - public function beforeTraverse(array $nodes): ?array { $this->topNode = null; $this->variadicMethods = []; - $this->inClassLike = null; $this->inNamespace = null; + $this->inClassLike = null; + $this->inMethod = null; return null; } @@ -49,7 +49,8 @@ public function enterNode(Node $node): ?Node } if ($node instanceof Node\Stmt\ClassLike && $node->name instanceof Node\Identifier) { - $this->inClassLike = $node->name->name; + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name; + $this->variadicMethods[$this->inClassLike] ??= []; } if ($this->inClassLike !== null && $node instanceof ClassMethod) { @@ -57,13 +58,21 @@ public function enterNode(Node $node): ?Node return null; // interface } - if ($this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null) { - $className = $this->inNamespace !== null ? $this->inNamespace . '\\' . $this->inClassLike : $this->inClassLike; - if (!array_key_exists($className, $this->variadicMethods)) { - $this->variadicMethods[$className] = []; - } - $this->variadicMethods[$className][] = $node->name->name; + $this->inMethod = $node->name->name; + } + + if ( + $this->inMethod !== null + && $node instanceof Node\Expr\FuncCall + && $node->name instanceof Name + && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) + ) { + if (!array_key_exists($this->inMethod, $this->variadicMethods[$this->inClassLike])) { + $this->variadicMethods[$this->inClassLike][$this->inMethod] = TrinaryLogic::createYes(); + } else { + $this->variadicMethods[$this->inClassLike][$this->inMethod]->and(TrinaryLogic::createYes()); } + } return null; @@ -79,6 +88,11 @@ public function leaveNode(Node $node): ?Node $this->inClassLike = null; } + if ($node instanceof ClassMethod && $this->inClassLike !== null && $this->inMethod !== null) { + $this->variadicMethods[$this->inClassLike][$this->inMethod] ??= TrinaryLogic::createNo(); + $this->inMethod = null; + } + return null; } diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index 0307596df20..09cbe2a2a47 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -256,9 +256,9 @@ private function isVariadic(): bool if ( is_array($variadicMethods) && array_key_exists($declaringClass->getName(), $variadicMethods) - && in_array($this->reflection->getName(), $variadicMethods[$declaringClass->getName()], true) + && array_key_exists($this->reflection->getName(), $variadicMethods[$declaringClass->getName()]) ) { - return $this->containsVariadicCalls = true; + return $this->containsVariadicCalls = !$variadicMethods[$declaringClass->getName()][$this->reflection->getName()]->no(); } } diff --git a/tests/PHPStan/Parser/CleaningParserTest.php b/tests/PHPStan/Parser/CleaningParserTest.php index 4bbba722ed1..835486fdc2c 100644 --- a/tests/PHPStan/Parser/CleaningParserTest.php +++ b/tests/PHPStan/Parser/CleaningParserTest.php @@ -68,8 +68,8 @@ public function testParse( new SimpleParser( new Php7(new Emulative()), new NameResolver(), - new VariadicMethodsVisitor(new FunctionCallStatementFinder()), - new VariadicFunctionsVisitor(new FunctionCallStatementFinder()), + new VariadicMethodsVisitor(), + new VariadicFunctionsVisitor(), ), new PhpVersion($phpVersionId), ); From b8ee0bc849639da5ef66c386bca7364f3c195f9a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 4 Oct 2024 14:23:33 +0200 Subject: [PATCH 04/15] Added *Parser unit tests --- tests/PHPStan/Parser/ParserTest.php | 69 +++++++++++++++++++ .../Parser/data/variadic-functions.php | 27 ++++++++ .../PHPStan/Parser/data/variadic-methods.php | 21 ++++++ 3 files changed, 117 insertions(+) create mode 100644 tests/PHPStan/Parser/ParserTest.php create mode 100644 tests/PHPStan/Parser/data/variadic-functions.php create mode 100644 tests/PHPStan/Parser/data/variadic-methods.php diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php new file mode 100644 index 00000000000..a9ea1df0fc3 --- /dev/null +++ b/tests/PHPStan/Parser/ParserTest.php @@ -0,0 +1,69 @@ + TrinaryLogic::createYes(), + 'VariadicFunctions\nonvariadic' => TrinaryLogic::createNo(), + 'VariadicFunctions\maybe_variadic_fn1' => TrinaryLogic::createNo(), + ], + ]; + + yield [ + __DIR__ . '/data/variadic-methods.php', + VariadicMethodsVisitor::ATTRIBUTE_NAME, + [ + 'VariadicMethod\X' => [ + 'non_variadic_fn1' => TrinaryLogic::createNo(), + 'variadic_fn1' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + ], + ], + ]; + } + + /** + * @dataProvider dataVariadicCallLikes + * @param array|array> $expectedVariadics + * @throws ParserErrorsException + */ + public function testSimpleParserVariadicCallLikes(string $file, string $attributeName, array $expectedVariadics): void + { + /** @var RichParser $parser */ + $parser = self::getContainer()->getService('currentPhpVersionSimpleParser'); + $ast = $parser->parseFile($file); + $variadics = $ast[0]->getAttribute($attributeName); + $this->assertIsArray($variadics); + $this->assertSame($expectedVariadics, $variadics); + } + + /** + * @dataProvider dataVariadicCallLikes + * @param array|array> $expectedVariadics + * @throws ParserErrorsException + */ + public function testRichParserVariadicCallLikes(string $file, string $attributeName, array $expectedVariadics): void + { + /** @var RichParser $parser */ + $parser = self::getContainer()->getService('currentPhpVersionRichParser'); + $ast = $parser->parseFile($file); + $variadics = $ast[0]->getAttribute($attributeName); + $this->assertIsArray($variadics); + $this->assertSame($expectedVariadics, $variadics); + } + +} diff --git a/tests/PHPStan/Parser/data/variadic-functions.php b/tests/PHPStan/Parser/data/variadic-functions.php new file mode 100644 index 00000000000..c07a3a34b7a --- /dev/null +++ b/tests/PHPStan/Parser/data/variadic-functions.php @@ -0,0 +1,27 @@ + Date: Sun, 6 Oct 2024 17:57:37 +0200 Subject: [PATCH 05/15] test nested classes --- src/Parser/VariadicMethodsVisitor.php | 54 ++++++++++++++----- tests/PHPStan/Parser/ParserTest.php | 28 ++++++++++ .../Parser/data/variadic-methods-in-enum.php | 16 ++++++ .../PHPStan/Parser/data/variadic-methods.php | 35 +++++++++++- 4 files changed, 117 insertions(+), 16 deletions(-) create mode 100644 tests/PHPStan/Parser/data/variadic-methods-in-enum.php diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 8691b5b3315..91411a623c8 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -6,6 +6,7 @@ use PhpParser\Node\Name; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\NodeVisitorAbstract; +use PHPStan\Node\AnonymousClassNode; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\TrinaryLogic; use function array_key_exists; @@ -20,6 +21,11 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract private ?string $inClassLike = null; + /** + * @var array + */ + private array $classStack = []; + private ?string $inMethod = null; /** @var array> */ @@ -32,6 +38,7 @@ public function beforeTraverse(array $nodes): ?array $this->topNode = null; $this->variadicMethods = []; $this->inNamespace = null; + $this->classStack = []; $this->inClassLike = null; $this->inMethod = null; @@ -48,21 +55,28 @@ public function enterNode(Node $node): ?Node $this->inNamespace = $node->name->toString(); } - if ($node instanceof Node\Stmt\ClassLike && $node->name instanceof Node\Identifier) { - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name; + if ( + $node instanceof Node\Stmt\Class_ + || $node instanceof Node\Stmt\ClassLike + ) { + if (!$node->name instanceof Node\Identifier) { + $className = 'class@anonymous'; + } else { + $className = $node->name->name; + } + + $this->classStack[] = $className; + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); $this->variadicMethods[$this->inClassLike] ??= []; } if ($this->inClassLike !== null && $node instanceof ClassMethod) { - if ($node->getStmts() === null) { - return null; // interface - } - $this->inMethod = $node->name->name; } if ( - $this->inMethod !== null + $this->inClassLike !== null + && $this->inMethod !== null && $node instanceof Node\Expr\FuncCall && $node->name instanceof Name && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) @@ -80,17 +94,29 @@ public function enterNode(Node $node): ?Node public function leaveNode(Node $node): ?Node { - if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { - $this->inNamespace = null; + if ( + $node instanceof ClassMethod + && $this->inClassLike !== null + ) { + $this->variadicMethods[$this->inClassLike][$node->name->name] ??= TrinaryLogic::createNo(); + $this->inMethod = null; } - if ($node instanceof Node\Stmt\ClassLike && $node->name instanceof Node\Identifier) { - $this->inClassLike = null; + if ( + $node instanceof Node\Stmt\Class_ + || $node instanceof Node\Stmt\ClassLike + ) { + array_pop($this->classStack); + + if ($this->classStack !== []) { + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + } else { + $this->inClassLike = null; + } } - if ($node instanceof ClassMethod && $this->inClassLike !== null && $this->inMethod !== null) { - $this->variadicMethods[$this->inClassLike][$this->inMethod] ??= TrinaryLogic::createNo(); - $this->inMethod = null; + if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { + $this->inNamespace = null; } return null; diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php index a9ea1df0fc3..d4bf2475228 100644 --- a/tests/PHPStan/Parser/ParserTest.php +++ b/tests/PHPStan/Parser/ParserTest.php @@ -31,9 +31,37 @@ public function dataVariadicCallLikes(): iterable 'VariadicMethod\X' => [ 'non_variadic_fn1' => TrinaryLogic::createNo(), 'variadic_fn1' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'implicit_variadic_fn1' => TrinaryLogic::createYes(), + ], + 'VariadicMethod\Z' => [ + 'non_variadic_fnZ' => TrinaryLogic::createNo(), + 'variadic_fnZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'implicit_variadic_fnZ' => TrinaryLogic::createYes(), + ], + 'VariadicMethod\Z\class@anonymous' => [ + 'non_variadic_fn_subZ' => TrinaryLogic::createNo(), + 'variadic_fn_subZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'implicit_variadic_subZ' => TrinaryLogic::createYes(), + ], + 'VariadicMethod\class@anonymous' => [ + 'non_variadic_fn' => TrinaryLogic::createNo(), + 'variadic_fn' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'implicit_variadic_fn' => TrinaryLogic::createYes(), ], ], ]; + + yield [ + __DIR__ . '/data/variadic-methods-in-enum.php', + VariadicMethodsVisitor::ATTRIBUTE_NAME, + [ + 'VariadicMethodEnum\X' => [ + 'non_variadic_fn1' => TrinaryLogic::createNo(), + 'variadic_fn1' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'implicit_variadic_fn1' => TrinaryLogic::createYes(), + ], + ] + ]; } /** diff --git a/tests/PHPStan/Parser/data/variadic-methods-in-enum.php b/tests/PHPStan/Parser/data/variadic-methods-in-enum.php new file mode 100644 index 00000000000..03594920e0f --- /dev/null +++ b/tests/PHPStan/Parser/data/variadic-methods-in-enum.php @@ -0,0 +1,16 @@ += 8.1 + +namespace VariadicMethodEnum; + +enum X { + + function non_variadic_fn1($v) { + } + + function variadic_fn1(...$v) { + } + + function implicit_variadic_fn1() { + $args = func_get_args(); + } +} diff --git a/tests/PHPStan/Parser/data/variadic-methods.php b/tests/PHPStan/Parser/data/variadic-methods.php index 36ce047a092..cf3d94c0318 100644 --- a/tests/PHPStan/Parser/data/variadic-methods.php +++ b/tests/PHPStan/Parser/data/variadic-methods.php @@ -9,13 +9,44 @@ function non_variadic_fn1($v) { function variadic_fn1(...$v) { } + + function implicit_variadic_fn1() { + $args = func_get_args(); + } +} + +class Z { + function non_variadic_fnZ($v) { + return $x = new class { + function non_variadic_fn_subZ($v) { + } + + function variadic_fn_subZ(...$v) { + } + + function implicit_variadic_subZ() { + $args = func_get_args(); + } + }; + } + + function variadic_fnZ(...$v) { + } + + function implicit_variadic_fnZ() { + $args = func_get_args(); + } } $x = new class { - function non_variadic_fn1($v) { + function non_variadic_fn($v) { } - function variadic_fn1(...$v) { + function variadic_fn(...$v) { + } + + function implicit_variadic_fn() { + $args = func_get_args(); } }; From c3cdb0b5fa38dda4da5abcd1afba83fcd9a947f1 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 6 Oct 2024 17:58:58 +0200 Subject: [PATCH 06/15] test functions --- src/Parser/VariadicMethodsVisitor.php | 9 ++++----- tests/PHPStan/Parser/ParserTest.php | 7 ++++--- tests/PHPStan/Parser/data/variadic-functions.php | 4 ++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 91411a623c8..366c266abf6 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -6,10 +6,11 @@ use PhpParser\Node\Name; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\NodeVisitorAbstract; -use PHPStan\Node\AnonymousClassNode; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\TrinaryLogic; use function array_key_exists; +use function array_pop; +use function implode; use function in_array; final class VariadicMethodsVisitor extends NodeVisitorAbstract @@ -21,9 +22,7 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract private ?string $inClassLike = null; - /** - * @var array - */ + /** @var array */ private array $classStack = []; private ?string $inMethod = null; @@ -60,7 +59,7 @@ public function enterNode(Node $node): ?Node || $node instanceof Node\Stmt\ClassLike ) { if (!$node->name instanceof Node\Identifier) { - $className = 'class@anonymous'; + $className = 'class@anonymous:' . $node->getStartLine(); } else { $className = $node->name->name; } diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php index d4bf2475228..6db25ce8234 100644 --- a/tests/PHPStan/Parser/ParserTest.php +++ b/tests/PHPStan/Parser/ParserTest.php @@ -21,6 +21,7 @@ public function dataVariadicCallLikes(): iterable 'VariadicFunctions\variadic_fn1' => TrinaryLogic::createYes(), 'VariadicFunctions\nonvariadic' => TrinaryLogic::createNo(), 'VariadicFunctions\maybe_variadic_fn1' => TrinaryLogic::createNo(), + 'VariadicFunctions\implicit_variadic_fn1' => TrinaryLogic::createYes(), ], ]; @@ -38,12 +39,12 @@ public function dataVariadicCallLikes(): iterable 'variadic_fnZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_fnZ' => TrinaryLogic::createYes(), ], - 'VariadicMethod\Z\class@anonymous' => [ + 'VariadicMethod\Z\class@anonymous:20' => [ 'non_variadic_fn_subZ' => TrinaryLogic::createNo(), 'variadic_fn_subZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_subZ' => TrinaryLogic::createYes(), ], - 'VariadicMethod\class@anonymous' => [ + 'VariadicMethod\class@anonymous:42' => [ 'non_variadic_fn' => TrinaryLogic::createNo(), 'variadic_fn' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_fn' => TrinaryLogic::createYes(), @@ -60,7 +61,7 @@ public function dataVariadicCallLikes(): iterable 'variadic_fn1' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_fn1' => TrinaryLogic::createYes(), ], - ] + ], ]; } diff --git a/tests/PHPStan/Parser/data/variadic-functions.php b/tests/PHPStan/Parser/data/variadic-functions.php index c07a3a34b7a..d1a572e1e09 100644 --- a/tests/PHPStan/Parser/data/variadic-functions.php +++ b/tests/PHPStan/Parser/data/variadic-functions.php @@ -25,3 +25,7 @@ function maybe_variadic_fn1(...$v) $fn2 = function ($x) use ($y) { return $x + $y; }; + +function implicit_variadic_fn1() { + $args = func_get_args(); +} From 687c50d70c0a81c65d0ea32e65e282055d538d76 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 7 Oct 2024 18:09:42 +0200 Subject: [PATCH 07/15] unique index for same-line anonymous classes --- src/Parser/VariadicMethodsVisitor.php | 16 +++++++--------- tests/PHPStan/Parser/ParserTest.php | 10 +++++++--- tests/PHPStan/Parser/data/variadic-methods.php | 6 ++++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 366c266abf6..9fdbfc4743b 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -12,6 +12,7 @@ use function array_pop; use function implode; use function in_array; +use function sprintf; final class VariadicMethodsVisitor extends NodeVisitorAbstract { @@ -30,6 +31,8 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract /** @var array> */ private array $variadicMethods = []; + private int $anonymousClassIndex = 0; + public const ATTRIBUTE_NAME = 'variadicMethods'; public function beforeTraverse(array $nodes): ?array @@ -40,6 +43,7 @@ public function beforeTraverse(array $nodes): ?array $this->classStack = []; $this->inClassLike = null; $this->inMethod = null; + $this->anonymousClassIndex = 0; return null; } @@ -54,12 +58,9 @@ public function enterNode(Node $node): ?Node $this->inNamespace = $node->name->toString(); } - if ( - $node instanceof Node\Stmt\Class_ - || $node instanceof Node\Stmt\ClassLike - ) { + if ($node instanceof Node\Stmt\ClassLike) { if (!$node->name instanceof Node\Identifier) { - $className = 'class@anonymous:' . $node->getStartLine(); + $className = sprintf('class@anonymous:%s:%s', $node->getStartLine(), ++$this->anonymousClassIndex); } else { $className = $node->name->name; } @@ -101,10 +102,7 @@ public function leaveNode(Node $node): ?Node $this->inMethod = null; } - if ( - $node instanceof Node\Stmt\Class_ - || $node instanceof Node\Stmt\ClassLike - ) { + if ($node instanceof Node\Stmt\ClassLike) { array_pop($this->classStack); if ($this->classStack !== []) { diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php index 6db25ce8234..58d3aaff9b4 100644 --- a/tests/PHPStan/Parser/ParserTest.php +++ b/tests/PHPStan/Parser/ParserTest.php @@ -39,16 +39,20 @@ public function dataVariadicCallLikes(): iterable 'variadic_fnZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_fnZ' => TrinaryLogic::createYes(), ], - 'VariadicMethod\Z\class@anonymous:20' => [ + 'VariadicMethod\Z\class@anonymous:20:1' => [ 'non_variadic_fn_subZ' => TrinaryLogic::createNo(), 'variadic_fn_subZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_subZ' => TrinaryLogic::createYes(), ], - 'VariadicMethod\class@anonymous:42' => [ + 'VariadicMethod\class@anonymous:42:2' => [ 'non_variadic_fn' => TrinaryLogic::createNo(), 'variadic_fn' => TrinaryLogic::createNo(), // variadicness later on detected via reflection 'implicit_variadic_fn' => TrinaryLogic::createYes(), ], + 'VariadicMethod\class@anonymous:54:3' => [ + 'implicit_variadic_fn' => TrinaryLogic::createYes(), + ], + 'VariadicMethod\class@anonymous:54:4' => [], ], ]; @@ -72,7 +76,7 @@ public function dataVariadicCallLikes(): iterable */ public function testSimpleParserVariadicCallLikes(string $file, string $attributeName, array $expectedVariadics): void { - /** @var RichParser $parser */ + /** @var SimpleParser $parser */ $parser = self::getContainer()->getService('currentPhpVersionSimpleParser'); $ast = $parser->parseFile($file); $variadics = $ast[0]->getAttribute($attributeName); diff --git a/tests/PHPStan/Parser/data/variadic-methods.php b/tests/PHPStan/Parser/data/variadic-methods.php index cf3d94c0318..d808c0f3ad9 100644 --- a/tests/PHPStan/Parser/data/variadic-methods.php +++ b/tests/PHPStan/Parser/data/variadic-methods.php @@ -50,3 +50,9 @@ function implicit_variadic_fn() { $args = func_get_args(); } }; + +$c = new class (new class {}) { + function implicit_variadic_fn() { + $args = func_get_args(); + } +}; From fc7e0ca5fb65aaaced16e4fcfe101fad8149e392 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Mon, 7 Oct 2024 18:20:47 +0200 Subject: [PATCH 08/15] Drop inClassLike property --- src/Parser/VariadicMethodsVisitor.php | 31 +++++++++++---------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 9fdbfc4743b..46ebb95d733 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -21,8 +21,6 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract private ?string $inNamespace = null; - private ?string $inClassLike = null; - /** @var array */ private array $classStack = []; @@ -41,7 +39,6 @@ public function beforeTraverse(array $nodes): ?array $this->variadicMethods = []; $this->inNamespace = null; $this->classStack = []; - $this->inClassLike = null; $this->inMethod = null; $this->anonymousClassIndex = 0; @@ -66,25 +63,27 @@ public function enterNode(Node $node): ?Node } $this->classStack[] = $className; - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); - $this->variadicMethods[$this->inClassLike] ??= []; + $inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + $this->variadicMethods[$inClassLike] ??= []; } - if ($this->inClassLike !== null && $node instanceof ClassMethod) { + if ($this->classStack !== [] && $node instanceof ClassMethod) { $this->inMethod = $node->name->name; } if ( - $this->inClassLike !== null + $this->classStack !== [] && $this->inMethod !== null && $node instanceof Node\Expr\FuncCall && $node->name instanceof Name && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) ) { - if (!array_key_exists($this->inMethod, $this->variadicMethods[$this->inClassLike])) { - $this->variadicMethods[$this->inClassLike][$this->inMethod] = TrinaryLogic::createYes(); + $inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + + if (!array_key_exists($this->inMethod, $this->variadicMethods[$inClassLike])) { + $this->variadicMethods[$inClassLike][$this->inMethod] = TrinaryLogic::createYes(); } else { - $this->variadicMethods[$this->inClassLike][$this->inMethod]->and(TrinaryLogic::createYes()); + $this->variadicMethods[$inClassLike][$this->inMethod]->and(TrinaryLogic::createYes()); } } @@ -96,20 +95,16 @@ public function leaveNode(Node $node): ?Node { if ( $node instanceof ClassMethod - && $this->inClassLike !== null + && $this->classStack !== [] ) { - $this->variadicMethods[$this->inClassLike][$node->name->name] ??= TrinaryLogic::createNo(); + $inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + + $this->variadicMethods[$inClassLike][$node->name->name] ??= TrinaryLogic::createNo(); $this->inMethod = null; } if ($node instanceof Node\Stmt\ClassLike) { array_pop($this->classStack); - - if ($this->classStack !== []) { - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); - } else { - $this->inClassLike = null; - } } if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { From 13b1067a42ce5d15590e3434ba4f77c791096db4 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 8 Oct 2024 09:07:29 +0200 Subject: [PATCH 09/15] Revert "Drop inClassLike property" This reverts commit 083aaf3b64b2a539ac11db1bb5ebbc0574af9967. --- src/Parser/VariadicMethodsVisitor.php | 31 ++++++++++++++++----------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 46ebb95d733..9fdbfc4743b 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -21,6 +21,8 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract private ?string $inNamespace = null; + private ?string $inClassLike = null; + /** @var array */ private array $classStack = []; @@ -39,6 +41,7 @@ public function beforeTraverse(array $nodes): ?array $this->variadicMethods = []; $this->inNamespace = null; $this->classStack = []; + $this->inClassLike = null; $this->inMethod = null; $this->anonymousClassIndex = 0; @@ -63,27 +66,25 @@ public function enterNode(Node $node): ?Node } $this->classStack[] = $className; - $inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); - $this->variadicMethods[$inClassLike] ??= []; + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + $this->variadicMethods[$this->inClassLike] ??= []; } - if ($this->classStack !== [] && $node instanceof ClassMethod) { + if ($this->inClassLike !== null && $node instanceof ClassMethod) { $this->inMethod = $node->name->name; } if ( - $this->classStack !== [] + $this->inClassLike !== null && $this->inMethod !== null && $node instanceof Node\Expr\FuncCall && $node->name instanceof Name && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) ) { - $inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); - - if (!array_key_exists($this->inMethod, $this->variadicMethods[$inClassLike])) { - $this->variadicMethods[$inClassLike][$this->inMethod] = TrinaryLogic::createYes(); + if (!array_key_exists($this->inMethod, $this->variadicMethods[$this->inClassLike])) { + $this->variadicMethods[$this->inClassLike][$this->inMethod] = TrinaryLogic::createYes(); } else { - $this->variadicMethods[$inClassLike][$this->inMethod]->and(TrinaryLogic::createYes()); + $this->variadicMethods[$this->inClassLike][$this->inMethod]->and(TrinaryLogic::createYes()); } } @@ -95,16 +96,20 @@ public function leaveNode(Node $node): ?Node { if ( $node instanceof ClassMethod - && $this->classStack !== [] + && $this->inClassLike !== null ) { - $inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); - - $this->variadicMethods[$inClassLike][$node->name->name] ??= TrinaryLogic::createNo(); + $this->variadicMethods[$this->inClassLike][$node->name->name] ??= TrinaryLogic::createNo(); $this->inMethod = null; } if ($node instanceof Node\Stmt\ClassLike) { array_pop($this->classStack); + + if ($this->classStack !== []) { + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + } else { + $this->inClassLike = null; + } } if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { From c9583672ddf73d33922b6f34619de5925f28df24 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 8 Oct 2024 09:17:06 +0200 Subject: [PATCH 10/15] fix anonymous class detection --- src/Parser/VariadicMethodsVisitor.php | 11 +++++------ src/Reflection/Php/PhpMethodReflection.php | 12 +++++++++--- tests/PHPStan/Parser/ParserTest.php | 16 ++++++++-------- .../Rules/Methods/CallMethodsRuleTest.php | 15 +++++++++++++++ tests/PHPStan/Rules/Methods/data/bug-11559c.php | 14 ++++++++++++++ 5 files changed, 51 insertions(+), 17 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-11559c.php diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 9fdbfc4743b..626ad2d8a5c 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -31,8 +31,6 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract /** @var array> */ private array $variadicMethods = []; - private int $anonymousClassIndex = 0; - public const ATTRIBUTE_NAME = 'variadicMethods'; public function beforeTraverse(array $nodes): ?array @@ -43,7 +41,6 @@ public function beforeTraverse(array $nodes): ?array $this->classStack = []; $this->inClassLike = null; $this->inMethod = null; - $this->anonymousClassIndex = 0; return null; } @@ -60,13 +57,15 @@ public function enterNode(Node $node): ?Node if ($node instanceof Node\Stmt\ClassLike) { if (!$node->name instanceof Node\Identifier) { - $className = sprintf('class@anonymous:%s:%s', $node->getStartLine(), ++$this->anonymousClassIndex); + $className = sprintf('class@anonymous:%s:%s', $node->getStartLine(), $node->getEndLine()); + $this->classStack[] = $className; + $this->inClassLike = $className; // anonymous classes are in global namespace } else { $className = $node->name->name; + $this->classStack[] = $className; + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); } - $this->classStack[] = $className; - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); $this->variadicMethods[$this->inClassLike] ??= []; } diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index 09cbe2a2a47..93c9d713549 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -35,6 +35,7 @@ use function explode; use function in_array; use function is_array; +use function sprintf; use function strtolower; use const PHP_VERSION_ID; @@ -253,12 +254,17 @@ private function isVariadic(): bool if (count($nodes) > 0) { $variadicMethods = $nodes[0]->getAttribute(VariadicMethodsVisitor::ATTRIBUTE_NAME); + $className = $declaringClass->getName(); + if ($declaringClass->isAnonymous()) { + $className = sprintf('class@anonymous:%s:%s', $declaringClass->getNativeReflection()->getStartLine(), $declaringClass->getNativeReflection()->getEndLine()); + } + if ( is_array($variadicMethods) - && array_key_exists($declaringClass->getName(), $variadicMethods) - && array_key_exists($this->reflection->getName(), $variadicMethods[$declaringClass->getName()]) + && array_key_exists($className, $variadicMethods) + && array_key_exists($this->reflection->getName(), $variadicMethods[$className]) ) { - return $this->containsVariadicCalls = !$variadicMethods[$declaringClass->getName()][$this->reflection->getName()]->no(); + return $this->containsVariadicCalls = !$variadicMethods[$className][$this->reflection->getName()]->no(); } } diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php index 58d3aaff9b4..75742128958 100644 --- a/tests/PHPStan/Parser/ParserTest.php +++ b/tests/PHPStan/Parser/ParserTest.php @@ -31,28 +31,28 @@ public function dataVariadicCallLikes(): iterable [ 'VariadicMethod\X' => [ 'non_variadic_fn1' => TrinaryLogic::createNo(), - 'variadic_fn1' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'variadic_fn1' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection 'implicit_variadic_fn1' => TrinaryLogic::createYes(), ], 'VariadicMethod\Z' => [ 'non_variadic_fnZ' => TrinaryLogic::createNo(), - 'variadic_fnZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'variadic_fnZ' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection 'implicit_variadic_fnZ' => TrinaryLogic::createYes(), ], - 'VariadicMethod\Z\class@anonymous:20:1' => [ + 'class@anonymous:20:30' => [ 'non_variadic_fn_subZ' => TrinaryLogic::createNo(), - 'variadic_fn_subZ' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'variadic_fn_subZ' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection 'implicit_variadic_subZ' => TrinaryLogic::createYes(), ], - 'VariadicMethod\class@anonymous:42:2' => [ + 'class@anonymous:42:52' => [ 'non_variadic_fn' => TrinaryLogic::createNo(), - 'variadic_fn' => TrinaryLogic::createNo(), // variadicness later on detected via reflection + 'variadic_fn' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection 'implicit_variadic_fn' => TrinaryLogic::createYes(), ], - 'VariadicMethod\class@anonymous:54:3' => [ + 'class@anonymous:54:58' => [ 'implicit_variadic_fn' => TrinaryLogic::createYes(), ], - 'VariadicMethod\class@anonymous:54:4' => [], + 'class@anonymous:54:54' => [], ], ]; diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index 7342c880fec..478064f39ba 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3412,4 +3412,19 @@ public function testBug1953(): void ]); } + public function testBug11559c(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->checkExplicitMixed = true; + + $this->analyse([__DIR__ . '/data/bug-11559c.php'], [ + [ + 'Method class@anonymous/tests/PHPStan/Rules/Methods/data/bug-11559c.php:5:1::regular_fn() invoked with 3 parameters, 1 required.', + 14, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-11559c.php b/tests/PHPStan/Rules/Methods/data/bug-11559c.php new file mode 100644 index 00000000000..f6a002f66b6 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-11559c.php @@ -0,0 +1,14 @@ +implicit_variadic_fn(1, 2, 3); +$c->regular_fn(1, 2, 3); From f3fb80a3a04b094d794572c3f39bfe883aad8f22 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 8 Oct 2024 09:21:56 +0200 Subject: [PATCH 11/15] put test out of global namspace --- .../Rules/Methods/CallMethodsRuleTest.php | 4 ++-- .../PHPStan/Rules/Methods/data/bug-11559c.php | 20 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index 478064f39ba..3ddc7db072d 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -3421,8 +3421,8 @@ public function testBug11559c(): void $this->analyse([__DIR__ . '/data/bug-11559c.php'], [ [ - 'Method class@anonymous/tests/PHPStan/Rules/Methods/data/bug-11559c.php:5:1::regular_fn() invoked with 3 parameters, 1 required.', - 14, + 'Method class@anonymous/tests/PHPStan/Rules/Methods/data/bug-11559c.php:6:1::regular_fn() invoked with 3 parameters, 1 required.', + 15, ], ]); } diff --git a/tests/PHPStan/Rules/Methods/data/bug-11559c.php b/tests/PHPStan/Rules/Methods/data/bug-11559c.php index f6a002f66b6..54e3ba27b05 100644 --- a/tests/PHPStan/Rules/Methods/data/bug-11559c.php +++ b/tests/PHPStan/Rules/Methods/data/bug-11559c.php @@ -2,13 +2,15 @@ namespace Bug11559c; -$c = new class (new class {}) { - function implicit_variadic_fn() { - $args = func_get_args(); - } - function regular_fn(int $i) { - } -}; +function doFoo() { + $c = new class (new class {}) { + function implicit_variadic_fn() { + $args = func_get_args(); + } + function regular_fn(int $i) { + } + }; -$c->implicit_variadic_fn(1, 2, 3); -$c->regular_fn(1, 2, 3); + $c->implicit_variadic_fn(1, 2, 3); + $c->regular_fn(1, 2, 3); +} From 676d4409a665f7596e5d2dc537b8d4eb329ec035 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 8 Oct 2024 10:00:12 +0200 Subject: [PATCH 12/15] fix wrong inClassLike after stack pop --- src/Parser/VariadicMethodsVisitor.php | 14 ++++++++++++-- tests/PHPStan/Parser/ParserTest.php | 5 +++++ tests/PHPStan/Parser/data/variadic-methods.php | 10 ++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 626ad2d8a5c..1fcef407648 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -10,9 +10,11 @@ use PHPStan\TrinaryLogic; use function array_key_exists; use function array_pop; +use function count; use function implode; use function in_array; use function sprintf; +use function str_contains; final class VariadicMethodsVisitor extends NodeVisitorAbstract { @@ -33,6 +35,8 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract public const ATTRIBUTE_NAME = 'variadicMethods'; + private const ANONYMOUS_CLASS_PREFIX = 'class@anonymous'; + public function beforeTraverse(array $nodes): ?array { $this->topNode = null; @@ -57,7 +61,7 @@ public function enterNode(Node $node): ?Node if ($node instanceof Node\Stmt\ClassLike) { if (!$node->name instanceof Node\Identifier) { - $className = sprintf('class@anonymous:%s:%s', $node->getStartLine(), $node->getEndLine()); + $className = sprintf('%s:%s:%s', self::ANONYMOUS_CLASS_PREFIX, $node->getStartLine(), $node->getEndLine()); $this->classStack[] = $className; $this->inClassLike = $className; // anonymous classes are in global namespace } else { @@ -105,7 +109,13 @@ public function leaveNode(Node $node): ?Node array_pop($this->classStack); if ($this->classStack !== []) { - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + $lastClass = $this->classStack[count($this->classStack) - 1]; + + if (str_contains($lastClass, self::ANONYMOUS_CLASS_PREFIX)) { + $this->inClassLike = $lastClass; + } else { + $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + } } else { $this->inClassLike = null; } diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php index 75742128958..627226e0d22 100644 --- a/tests/PHPStan/Parser/ParserTest.php +++ b/tests/PHPStan/Parser/ParserTest.php @@ -53,6 +53,11 @@ public function dataVariadicCallLikes(): iterable 'implicit_variadic_fn' => TrinaryLogic::createYes(), ], 'class@anonymous:54:54' => [], + 'class@anonymous:61:68' => [ + 'nestedClass' => TrinaryLogic::createNo(), + 'implicit_variadic_fn' => TrinaryLogic::createYes(), + ], + 'class@anonymous:63:63' => [], ], ]; diff --git a/tests/PHPStan/Parser/data/variadic-methods.php b/tests/PHPStan/Parser/data/variadic-methods.php index d808c0f3ad9..da6135b9671 100644 --- a/tests/PHPStan/Parser/data/variadic-methods.php +++ b/tests/PHPStan/Parser/data/variadic-methods.php @@ -56,3 +56,13 @@ function implicit_variadic_fn() { $args = func_get_args(); } }; + + +$c = new class () { + function nestedClass() { + $nested = new class () {}; + } + function implicit_variadic_fn() { + $args = func_get_args(); + } +}; From d4d5c1a79fc497dc17729fb790ce4a4f6c461776 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 8 Oct 2024 10:59:00 +0200 Subject: [PATCH 13/15] My vision --- src/Parser/VariadicFunctionsVisitor.php | 28 +++------- src/Parser/VariadicMethodsVisitor.php | 57 ++++++-------------- src/Reflection/Php/PhpFunctionReflection.php | 2 +- src/Reflection/Php/PhpMethodReflection.php | 4 +- tests/PHPStan/Parser/ParserTest.php | 50 ++++++++--------- 5 files changed, 48 insertions(+), 93 deletions(-) diff --git a/src/Parser/VariadicFunctionsVisitor.php b/src/Parser/VariadicFunctionsVisitor.php index 0b4bceb0020..f4a34415e84 100644 --- a/src/Parser/VariadicFunctionsVisitor.php +++ b/src/Parser/VariadicFunctionsVisitor.php @@ -6,7 +6,7 @@ use PhpParser\Node\Name; use PhpParser\NodeVisitorAbstract; use PHPStan\Reflection\ParametersAcceptor; -use PHPStan\TrinaryLogic; +use function array_filter; use function array_key_exists; use function in_array; @@ -19,7 +19,7 @@ final class VariadicFunctionsVisitor extends NodeVisitorAbstract private ?string $inFunction = null; - /** @var array */ + /** @var array */ private array $variadicFunctions = []; public const ATTRIBUTE_NAME = 'variadicFunctions'; @@ -46,18 +46,6 @@ public function enterNode(Node $node): ?Node if ($node instanceof Node\Stmt\Function_) { $this->inFunction = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name; - - foreach ($node->params as $parameter) { - if (!$parameter->variadic) { - continue; - } - - if (!array_key_exists($this->inFunction, $this->variadicFunctions)) { - $this->variadicFunctions[$this->inFunction] = TrinaryLogic::createYes(); - } else { - $this->variadicFunctions[$this->inFunction]->and(TrinaryLogic::createYes()); - } - } } if ( @@ -65,12 +53,9 @@ public function enterNode(Node $node): ?Node && $node instanceof Node\Expr\FuncCall && $node->name instanceof Name && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) + && !array_key_exists($this->inFunction, $this->variadicFunctions) ) { - if (!array_key_exists($this->inFunction, $this->variadicFunctions)) { - $this->variadicFunctions[$this->inFunction] = TrinaryLogic::createYes(); - } else { - $this->variadicFunctions[$this->inFunction]->and(TrinaryLogic::createYes()); - } + $this->variadicFunctions[$this->inFunction] = true; } return null; @@ -83,7 +68,7 @@ public function leaveNode(Node $node): ?Node } if ($node instanceof Node\Stmt\Function_ && $this->inFunction !== null) { - $this->variadicFunctions[$this->inFunction] ??= TrinaryLogic::createNo(); + $this->variadicFunctions[$this->inFunction] ??= false; $this->inFunction = null; } @@ -93,7 +78,8 @@ public function leaveNode(Node $node): ?Node public function afterTraverse(array $nodes): ?array { if ($this->topNode !== null && $this->variadicFunctions !== []) { - $this->topNode->setAttribute(self::ATTRIBUTE_NAME, $this->variadicFunctions); + $functions = array_filter($this->variadicFunctions, static fn (bool $variadic) => $variadic); + $this->topNode->setAttribute(self::ATTRIBUTE_NAME, $functions); } return null; diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 1fcef407648..59be3e2011d 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -7,43 +7,37 @@ use PhpParser\Node\Stmt\ClassMethod; use PhpParser\NodeVisitorAbstract; use PHPStan\Reflection\ParametersAcceptor; -use PHPStan\TrinaryLogic; use function array_key_exists; use function array_pop; use function count; -use function implode; use function in_array; use function sprintf; -use function str_contains; final class VariadicMethodsVisitor extends NodeVisitorAbstract { + public const ATTRIBUTE_NAME = 'variadicMethods'; + + public const ANONYMOUS_CLASS_PREFIX = 'class@anonymous'; + private ?Node $topNode = null; private ?string $inNamespace = null; - private ?string $inClassLike = null; - /** @var array */ private array $classStack = []; private ?string $inMethod = null; - /** @var array> */ + /** @var array> */ private array $variadicMethods = []; - public const ATTRIBUTE_NAME = 'variadicMethods'; - - private const ANONYMOUS_CLASS_PREFIX = 'class@anonymous'; - public function beforeTraverse(array $nodes): ?array { $this->topNode = null; $this->variadicMethods = []; $this->inNamespace = null; $this->classStack = []; - $this->inClassLike = null; $this->inMethod = null; return null; @@ -63,31 +57,30 @@ public function enterNode(Node $node): ?Node if (!$node->name instanceof Node\Identifier) { $className = sprintf('%s:%s:%s', self::ANONYMOUS_CLASS_PREFIX, $node->getStartLine(), $node->getEndLine()); $this->classStack[] = $className; - $this->inClassLike = $className; // anonymous classes are in global namespace } else { $className = $node->name->name; - $this->classStack[] = $className; - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); + $this->classStack[] = $this->inNamespace !== null ? $this->inNamespace . '\\' . $className : $className; } - - $this->variadicMethods[$this->inClassLike] ??= []; } - if ($this->inClassLike !== null && $node instanceof ClassMethod) { + if ($node instanceof ClassMethod) { $this->inMethod = $node->name->name; } if ( - $this->inClassLike !== null - && $this->inMethod !== null + $this->inMethod !== null && $node instanceof Node\Expr\FuncCall && $node->name instanceof Name && in_array((string) $node->name, ParametersAcceptor::VARIADIC_FUNCTIONS, true) ) { - if (!array_key_exists($this->inMethod, $this->variadicMethods[$this->inClassLike])) { - $this->variadicMethods[$this->inClassLike][$this->inMethod] = TrinaryLogic::createYes(); - } else { - $this->variadicMethods[$this->inClassLike][$this->inMethod]->and(TrinaryLogic::createYes()); + $lastClass = $this->classStack[count($this->classStack) - 1] ?? null; + if ($lastClass !== null) { + if ( + !array_key_exists($lastClass, $this->variadicMethods) + || !array_key_exists($this->inMethod, $this->variadicMethods[$lastClass]) + ) { + $this->variadicMethods[$lastClass][$this->inMethod] = true; + } } } @@ -97,28 +90,12 @@ public function enterNode(Node $node): ?Node public function leaveNode(Node $node): ?Node { - if ( - $node instanceof ClassMethod - && $this->inClassLike !== null - ) { - $this->variadicMethods[$this->inClassLike][$node->name->name] ??= TrinaryLogic::createNo(); + if ($node instanceof ClassMethod) { $this->inMethod = null; } if ($node instanceof Node\Stmt\ClassLike) { array_pop($this->classStack); - - if ($this->classStack !== []) { - $lastClass = $this->classStack[count($this->classStack) - 1]; - - if (str_contains($lastClass, self::ANONYMOUS_CLASS_PREFIX)) { - $this->inClassLike = $lastClass; - } else { - $this->inClassLike = $this->inNamespace !== null ? $this->inNamespace . '\\' . implode('\\', $this->classStack) : implode('\\', $this->classStack); - } - } else { - $this->inClassLike = null; - } } if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) { diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index 00979c4e1b5..c0260f36cb5 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -148,7 +148,7 @@ private function isVariadic(): bool is_array($variadicFunctions) && array_key_exists($this->reflection->getName(), $variadicFunctions) ) { - return $this->containsVariadicCalls = !$variadicFunctions[$this->reflection->getName()]->no(); + return $this->containsVariadicCalls = $variadicFunctions[$this->reflection->getName()]; } } diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index 93c9d713549..f5c6e54e3e8 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -256,7 +256,7 @@ private function isVariadic(): bool $className = $declaringClass->getName(); if ($declaringClass->isAnonymous()) { - $className = sprintf('class@anonymous:%s:%s', $declaringClass->getNativeReflection()->getStartLine(), $declaringClass->getNativeReflection()->getEndLine()); + $className = sprintf('%s:%s:%s', VariadicMethodsVisitor::ANONYMOUS_CLASS_PREFIX, $declaringClass->getNativeReflection()->getStartLine(), $declaringClass->getNativeReflection()->getEndLine()); } if ( @@ -264,7 +264,7 @@ private function isVariadic(): bool && array_key_exists($className, $variadicMethods) && array_key_exists($this->reflection->getName(), $variadicMethods[$className]) ) { - return $this->containsVariadicCalls = !$variadicMethods[$className][$this->reflection->getName()]->no(); + return $this->containsVariadicCalls = $variadicMethods[$className][$this->reflection->getName()]; } } diff --git a/tests/PHPStan/Parser/ParserTest.php b/tests/PHPStan/Parser/ParserTest.php index 627226e0d22..94fe9a406b8 100644 --- a/tests/PHPStan/Parser/ParserTest.php +++ b/tests/PHPStan/Parser/ParserTest.php @@ -3,7 +3,7 @@ namespace PHPStan\Parser; use PHPStan\Testing\PHPStanTestCase; -use PHPStan\TrinaryLogic; +use function count; /** * @covers \PHPStan\Parser\RichParser @@ -18,10 +18,7 @@ public function dataVariadicCallLikes(): iterable __DIR__ . '/data/variadic-functions.php', VariadicFunctionsVisitor::ATTRIBUTE_NAME, [ - 'VariadicFunctions\variadic_fn1' => TrinaryLogic::createYes(), - 'VariadicFunctions\nonvariadic' => TrinaryLogic::createNo(), - 'VariadicFunctions\maybe_variadic_fn1' => TrinaryLogic::createNo(), - 'VariadicFunctions\implicit_variadic_fn1' => TrinaryLogic::createYes(), + 'VariadicFunctions\implicit_variadic_fn1' => true, ], ]; @@ -30,34 +27,23 @@ public function dataVariadicCallLikes(): iterable VariadicMethodsVisitor::ATTRIBUTE_NAME, [ 'VariadicMethod\X' => [ - 'non_variadic_fn1' => TrinaryLogic::createNo(), - 'variadic_fn1' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection - 'implicit_variadic_fn1' => TrinaryLogic::createYes(), + 'implicit_variadic_fn1' => true, ], 'VariadicMethod\Z' => [ - 'non_variadic_fnZ' => TrinaryLogic::createNo(), - 'variadic_fnZ' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection - 'implicit_variadic_fnZ' => TrinaryLogic::createYes(), + 'implicit_variadic_fnZ' => true, ], 'class@anonymous:20:30' => [ - 'non_variadic_fn_subZ' => TrinaryLogic::createNo(), - 'variadic_fn_subZ' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection - 'implicit_variadic_subZ' => TrinaryLogic::createYes(), + 'implicit_variadic_subZ' => true, ], 'class@anonymous:42:52' => [ - 'non_variadic_fn' => TrinaryLogic::createNo(), - 'variadic_fn' => TrinaryLogic::createNo(), // native variadicness later on detected via reflection - 'implicit_variadic_fn' => TrinaryLogic::createYes(), + 'implicit_variadic_fn' => true, ], 'class@anonymous:54:58' => [ - 'implicit_variadic_fn' => TrinaryLogic::createYes(), + 'implicit_variadic_fn' => true, ], - 'class@anonymous:54:54' => [], 'class@anonymous:61:68' => [ - 'nestedClass' => TrinaryLogic::createNo(), - 'implicit_variadic_fn' => TrinaryLogic::createYes(), + 'implicit_variadic_fn' => true, ], - 'class@anonymous:63:63' => [], ], ]; @@ -66,9 +52,7 @@ public function dataVariadicCallLikes(): iterable VariadicMethodsVisitor::ATTRIBUTE_NAME, [ 'VariadicMethodEnum\X' => [ - 'non_variadic_fn1' => TrinaryLogic::createNo(), - 'variadic_fn1' => TrinaryLogic::createNo(), // variadicness later on detected via reflection - 'implicit_variadic_fn1' => TrinaryLogic::createYes(), + 'implicit_variadic_fn1' => true, ], ], ]; @@ -76,7 +60,7 @@ public function dataVariadicCallLikes(): iterable /** * @dataProvider dataVariadicCallLikes - * @param array|array> $expectedVariadics + * @param array|array> $expectedVariadics * @throws ParserErrorsException */ public function testSimpleParserVariadicCallLikes(string $file, string $attributeName, array $expectedVariadics): void @@ -86,12 +70,16 @@ public function testSimpleParserVariadicCallLikes(string $file, string $attribut $ast = $parser->parseFile($file); $variadics = $ast[0]->getAttribute($attributeName); $this->assertIsArray($variadics); - $this->assertSame($expectedVariadics, $variadics); + $this->assertCount(count($expectedVariadics), $variadics); + foreach ($expectedVariadics as $key => $expectedVariadic) { + $this->assertArrayHasKey($key, $variadics); + $this->assertSame($expectedVariadic, $variadics[$key]); + } } /** * @dataProvider dataVariadicCallLikes - * @param array|array> $expectedVariadics + * @param array|array> $expectedVariadics * @throws ParserErrorsException */ public function testRichParserVariadicCallLikes(string $file, string $attributeName, array $expectedVariadics): void @@ -101,7 +89,11 @@ public function testRichParserVariadicCallLikes(string $file, string $attributeN $ast = $parser->parseFile($file); $variadics = $ast[0]->getAttribute($attributeName); $this->assertIsArray($variadics); - $this->assertSame($expectedVariadics, $variadics); + $this->assertCount(count($expectedVariadics), $variadics); + foreach ($expectedVariadics as $key => $expectedVariadic) { + $this->assertArrayHasKey($key, $variadics); + $this->assertSame($expectedVariadic, $variadics[$key]); + } } } From d1a8cbdcac629badbf97e513fcf651f914a05bcf Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 8 Oct 2024 12:05:19 +0200 Subject: [PATCH 14/15] Optimization --- src/Reflection/Php/PhpFunctionReflection.php | 2 +- src/Reflection/Php/PhpMethodReflection.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index c0260f36cb5..7b8cc5b0924 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -133,7 +133,7 @@ private function getParameters(): array private function isVariadic(): bool { $isNativelyVariadic = $this->reflection->isVariadic(); - if (!$isNativelyVariadic && $this->reflection->getFileName() !== false) { + if (!$isNativelyVariadic && $this->reflection->getFileName() !== false && !$this->isBuiltin()) { $filename = $this->reflection->getFileName(); if ($this->containsVariadicCalls !== null) { diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index f5c6e54e3e8..4b36c2de263 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -245,7 +245,7 @@ private function isVariadic(): bool $filename = $this->declaringTrait->getFileName(); } - if (!$isNativelyVariadic && $filename !== null) { + if (!$isNativelyVariadic && $filename !== null && !$this->declaringClass->isBuiltin()) { if ($this->containsVariadicCalls !== null) { return $this->containsVariadicCalls; } From cfbadfdf1e2e855dbbdfd309adbd565d39269220 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 8 Oct 2024 12:14:34 +0200 Subject: [PATCH 15/15] Another optimization --- src/Parser/VariadicFunctionsVisitor.php | 6 ++++++ src/Parser/VariadicMethodsVisitor.php | 8 ++++++++ src/Reflection/Php/PhpFunctionReflection.php | 4 ++++ src/Reflection/Php/PhpMethodReflection.php | 17 ++++++++++++----- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Parser/VariadicFunctionsVisitor.php b/src/Parser/VariadicFunctionsVisitor.php index f4a34415e84..5276d0eb47d 100644 --- a/src/Parser/VariadicFunctionsVisitor.php +++ b/src/Parser/VariadicFunctionsVisitor.php @@ -19,6 +19,9 @@ final class VariadicFunctionsVisitor extends NodeVisitorAbstract private ?string $inFunction = null; + /** @var array */ + public static array $cache = []; + /** @var array */ private array $variadicFunctions = []; @@ -78,6 +81,9 @@ public function leaveNode(Node $node): ?Node public function afterTraverse(array $nodes): ?array { if ($this->topNode !== null && $this->variadicFunctions !== []) { + foreach ($this->variadicFunctions as $name => $variadic) { + self::$cache[$name] = $variadic; + } $functions = array_filter($this->variadicFunctions, static fn (bool $variadic) => $variadic); $this->topNode->setAttribute(self::ATTRIBUTE_NAME, $functions); } diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php index 59be3e2011d..50882efc549 100644 --- a/src/Parser/VariadicMethodsVisitor.php +++ b/src/Parser/VariadicMethodsVisitor.php @@ -29,6 +29,9 @@ final class VariadicMethodsVisitor extends NodeVisitorAbstract private ?string $inMethod = null; + /** @var array> */ + public static array $cache = []; + /** @var array> */ private array $variadicMethods = []; @@ -108,6 +111,11 @@ public function leaveNode(Node $node): ?Node public function afterTraverse(array $nodes): ?array { if ($this->topNode !== null && $this->variadicMethods !== []) { + foreach ($this->variadicMethods as $class => $methods) { + foreach ($methods as $name => $variadic) { + self::$cache[$class][$name] = $variadic; + } + } $this->topNode->setAttribute(self::ATTRIBUTE_NAME, $this->variadicMethods); } diff --git a/src/Reflection/Php/PhpFunctionReflection.php b/src/Reflection/Php/PhpFunctionReflection.php index 7b8cc5b0924..e8fbc2e8245 100644 --- a/src/Reflection/Php/PhpFunctionReflection.php +++ b/src/Reflection/Php/PhpFunctionReflection.php @@ -140,6 +140,10 @@ private function isVariadic(): bool return $this->containsVariadicCalls; } + if (array_key_exists($this->reflection->getName(), VariadicFunctionsVisitor::$cache)) { + return $this->containsVariadicCalls = VariadicFunctionsVisitor::$cache[$this->reflection->getName()]; + } + $nodes = $this->parser->parseFile($filename); if (count($nodes) > 0) { $variadicFunctions = $nodes[0]->getAttribute(VariadicFunctionsVisitor::ATTRIBUTE_NAME); diff --git a/src/Reflection/Php/PhpMethodReflection.php b/src/Reflection/Php/PhpMethodReflection.php index 4b36c2de263..53f5d800544 100644 --- a/src/Reflection/Php/PhpMethodReflection.php +++ b/src/Reflection/Php/PhpMethodReflection.php @@ -250,15 +250,22 @@ private function isVariadic(): bool return $this->containsVariadicCalls; } + $className = $declaringClass->getName(); + if ($declaringClass->isAnonymous()) { + $className = sprintf('%s:%s:%s', VariadicMethodsVisitor::ANONYMOUS_CLASS_PREFIX, $declaringClass->getNativeReflection()->getStartLine(), $declaringClass->getNativeReflection()->getEndLine()); + } + if (array_key_exists($className, VariadicMethodsVisitor::$cache)) { + if (array_key_exists($this->reflection->getName(), VariadicMethodsVisitor::$cache[$className])) { + return $this->containsVariadicCalls = VariadicMethodsVisitor::$cache[$className][$this->reflection->getName()]; + } + + return $this->containsVariadicCalls = false; + } + $nodes = $this->parser->parseFile($filename); if (count($nodes) > 0) { $variadicMethods = $nodes[0]->getAttribute(VariadicMethodsVisitor::ATTRIBUTE_NAME); - $className = $declaringClass->getName(); - if ($declaringClass->isAnonymous()) { - $className = sprintf('%s:%s:%s', VariadicMethodsVisitor::ANONYMOUS_CLASS_PREFIX, $declaringClass->getNativeReflection()->getStartLine(), $declaringClass->getNativeReflection()->getEndLine()); - } - if ( is_array($variadicMethods) && array_key_exists($className, $variadicMethods)