Remove inefficient caching from PhpMethodReflection

phpstan · Oct 3, 2024 · 545e3d6 · 545e3d6
1 parent 1249a20
commit 545e3d6
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 81 deletions.
diff --git 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
@@ -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<Node\Stmt> */
 		return $nodeTraverser->traverse($nodes);

diff --git a/src/Parser/VariadicMethodsVisitor.php b/src/Parser/VariadicMethodsVisitor.php
@@ -0,0 +1,94 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Parser;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\NodeVisitorAbstract;
+use PHPStan\Reflection\ParametersAcceptor;
+use function array_key_exists;
+
+final class VariadicMethodsVisitor extends NodeVisitorAbstract
+{
+
+	private ?Node $topNode = null;
+
+	private ?string $inClassLike = null;
+
+	private ?string $inNamespace = null;
+
+	/** @var array<string, list<string>> */
+	private array $variadicMethods = [];
+
+	public const ATTRIBUTE_NAME = 'variadicFunctions';
+
+	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
@@ -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<ExtendedFunctionVariant>|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
@@ -68,6 +68,7 @@ public function testParse(
 			new SimpleParser(
 				new Php7(new Emulative()),
 				new NameResolver(),
+				new VariadicMethodsVisitor(new FunctionCallStatementFinder()),
 			),
 			new PhpVersion($phpVersionId),
 		);