Skip to content

Commit da0aac6

Browse files
committed
Remove inefficient caching from PhpFunctionReflection
1 parent ea07f8c commit da0aac6

File tree

5 files changed

+106
-77
lines changed

5 files changed

+106
-77
lines changed

conf/config.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,11 @@ services:
307307
tags:
308308
- phpstan.parser.richParserNodeVisitor
309309

310+
-
311+
class: PHPStan\Parser\VariadicFunctionsVisitor
312+
tags:
313+
- phpstan.parser.richParserNodeVisitor
314+
310315
-
311316
class: PHPStan\Node\Printer\ExprPrinter
312317

src/Parser/SimpleParser.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ public function __construct(
1616
private \PhpParser\Parser $parser,
1717
private NameResolver $nameResolver,
1818
private VariadicMethodsVisitor $variadicMethodsVisitor,
19+
private VariadicFunctionsVisitor $variadicFunctionsVisitor,
1920
)
2021
{
2122
}
@@ -50,6 +51,7 @@ public function parseString(string $sourceCode): array
5051
$nodeTraverser = new NodeTraverser();
5152
$nodeTraverser->addVisitor($this->nameResolver);
5253
$nodeTraverser->addVisitor($this->variadicMethodsVisitor);
54+
$nodeTraverser->addVisitor($this->variadicFunctionsVisitor);
5355

5456
/** @var array<Node\Stmt> */
5557
return $nodeTraverser->traverse($nodes);
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Parser;
4+
5+
use PhpParser\Node;
6+
use PhpParser\NodeVisitorAbstract;
7+
use PHPStan\Reflection\ParametersAcceptor;
8+
use PHPStan\TrinaryLogic;
9+
use function array_key_exists;
10+
11+
final class VariadicFunctionsVisitor extends NodeVisitorAbstract
12+
{
13+
14+
private ?Node $topNode = null;
15+
16+
private ?string $inNamespace = null;
17+
18+
/** @var array<string, TrinaryLogic> */
19+
private array $variadicFunctions = [];
20+
21+
public const ATTRIBUTE_NAME = 'variadicFunctions';
22+
23+
public function __construct(
24+
private FunctionCallStatementFinder $functionCallStatementFinder,
25+
)
26+
{
27+
}
28+
29+
public function beforeTraverse(array $nodes): ?array
30+
{
31+
$this->topNode = null;
32+
$this->variadicFunctions = [];
33+
$this->inNamespace = null;
34+
35+
return null;
36+
}
37+
38+
public function enterNode(Node $node): ?Node
39+
{
40+
if ($this->topNode === null) {
41+
$this->topNode = $node;
42+
}
43+
44+
if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) {
45+
$this->inNamespace = $node->name->toString();
46+
}
47+
48+
if ($node instanceof Node\Stmt\Function_) {
49+
$functionName = $this->inNamespace !== null ? $this->inNamespace . '\\' . $node->name->name : $node->name->name;
50+
51+
if (!array_key_exists($functionName, $this->variadicFunctions)) {
52+
$this->variadicFunctions[$functionName] = TrinaryLogic::createMaybe();
53+
}
54+
55+
$isVariadic = $this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null;
56+
$this->variadicFunctions[$functionName] = $this->variadicFunctions[$functionName]->and(TrinaryLogic::createFromBoolean($isVariadic));
57+
}
58+
59+
return null;
60+
}
61+
62+
public function leaveNode(Node $node): ?Node
63+
{
64+
if ($node instanceof Node\Stmt\Namespace_ && $node->name !== null) {
65+
$this->inNamespace = null;
66+
}
67+
68+
return null;
69+
}
70+
71+
public function afterTraverse(array $nodes): ?array
72+
{
73+
if ($this->topNode !== null && $this->variadicFunctions !== []) {
74+
$this->topNode->setAttribute(self::ATTRIBUTE_NAME, $this->variadicFunctions);
75+
}
76+
77+
return null;
78+
}
79+
80+
}

src/Reflection/Php/PhpFunctionReflection.php

Lines changed: 18 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,35 @@
22

33
namespace PHPStan\Reflection\Php;
44

5-
use PhpParser\Node;
6-
use PhpParser\Node\Stmt\Function_;
75
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionFunction;
86
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter;
9-
use PHPStan\Cache\Cache;
10-
use PHPStan\Parser\FunctionCallStatementFinder;
117
use PHPStan\Parser\Parser;
8+
use PHPStan\Parser\VariadicFunctionsVisitor;
129
use PHPStan\Reflection\Assertions;
1310
use PHPStan\Reflection\ExtendedFunctionVariant;
1411
use PHPStan\Reflection\ExtendedParameterReflection;
1512
use PHPStan\Reflection\ExtendedParametersAcceptor;
1613
use PHPStan\Reflection\FunctionReflection;
1714
use PHPStan\Reflection\InitializerExprTypeResolver;
18-
use PHPStan\Reflection\ParametersAcceptor;
1915
use PHPStan\TrinaryLogic;
2016
use PHPStan\Type\Generic\TemplateTypeMap;
2117
use PHPStan\Type\MixedType;
2218
use PHPStan\Type\Type;
2319
use PHPStan\Type\TypehintHelper;
2420
use function array_key_exists;
2521
use function array_map;
26-
use function filemtime;
22+
use function count;
2723
use function is_array;
2824
use function is_file;
29-
use function sprintf;
30-
use function time;
3125

3226
final class PhpFunctionReflection implements FunctionReflection
3327
{
3428

3529
/** @var list<ExtendedFunctionVariant>|null */
3630
private ?array $variants = null;
3731

32+
private ?bool $containsVariadicCalls = null;
33+
3834
/**
3935
* @param array<string, Type> $phpDocParameterTypes
4036
* @param array<string, Type> $phpDocParameterOutTypes
@@ -45,8 +41,6 @@ public function __construct(
4541
private InitializerExprTypeResolver $initializerExprTypeResolver,
4642
private ReflectionFunction $reflection,
4743
private Parser $parser,
48-
private FunctionCallStatementFinder $functionCallStatementFinder,
49-
private Cache $cache,
5044
private TemplateTypeMap $templateTypeMap,
5145
private array $phpDocParameterTypes,
5246
private ?Type $phpDocReturnType,
@@ -139,67 +133,29 @@ private function getParameters(): array
139133
private function isVariadic(): bool
140134
{
141135
$isNativelyVariadic = $this->reflection->isVariadic();
142-
if (!$isNativelyVariadic && $this->reflection
143-
->getFileName() !== false) {
144-
$fileName = $this->reflection->getFileName();
145-
if (is_file($fileName)) {
146-
$functionName = $this->reflection->getName();
147-
$modifiedTime = filemtime($fileName);
148-
if ($modifiedTime === false) {
149-
$modifiedTime = time();
150-
}
151-
$variableCacheKey = sprintf('%d-v4', $modifiedTime);
152-
$key = sprintf('variadic-function-%s-%s', $functionName, $fileName);
153-
$cachedResult = $this->cache->load($key, $variableCacheKey);
154-
if ($cachedResult === null) {
155-
$nodes = $this->parser->parseFile($fileName);
156-
$result = !$this->containsVariadicFunction($nodes)->no();
157-
$this->cache->save($key, $variableCacheKey, $result);
158-
return $result;
159-
}
136+
if (!$isNativelyVariadic && $this->reflection->getFileName() !== false) {
137+
$filename = $this->reflection->getFileName();
160138

161-
return $cachedResult;
139+
if ($this->containsVariadicCalls !== null) {
140+
return $this->containsVariadicCalls;
162141
}
163-
}
164-
165-
return $isNativelyVariadic;
166-
}
167142

168-
/**
169-
* @param Node[]|scalar[]|Node $node
170-
*/
171-
private function containsVariadicFunction(array|Node $node): TrinaryLogic
172-
{
173-
$result = TrinaryLogic::createMaybe();
174-
175-
if ($node instanceof Node) {
176-
if ($node instanceof Function_) {
177-
$functionName = (string) $node->namespacedName;
178-
179-
if ($functionName === $this->reflection->getName()) {
180-
return TrinaryLogic::createFromBoolean($this->isFunctionNodeVariadic($node));
181-
}
182-
}
143+
$nodes = $this->parser->parseFile($filename);
144+
if (count($nodes) > 0) {
145+
$variadicFunctions = $nodes[0]->getAttribute(VariadicFunctionsVisitor::ATTRIBUTE_NAME);
183146

184-
foreach ($node->getSubNodeNames() as $subNodeName) {
185-
$innerNode = $node->{$subNodeName};
186-
if (!$innerNode instanceof Node && !is_array($innerNode)) {
187-
continue;
147+
if (
148+
is_array($variadicFunctions)
149+
&& array_key_exists($this->reflection->getName(), $variadicFunctions)
150+
) {
151+
return $this->containsVariadicCalls = !$variadicFunctions[$this->reflection->getName()]->no();
188152
}
189-
190-
$result = $result->and($this->containsVariadicFunction($innerNode));
191153
}
192-
} elseif (is_array($node)) {
193-
foreach ($node as $subNode) {
194-
if (!$subNode instanceof Node) {
195-
continue;
196-
}
197154

198-
$result = $result->and($this->containsVariadicFunction($subNode));
199-
}
155+
return $this->containsVariadicCalls = false;
200156
}
201157

202-
return $result;
158+
return $isNativelyVariadic;
203159
}
204160

205161
private function getReturnType(): Type
@@ -296,19 +252,4 @@ public function acceptsNamedArguments(): TrinaryLogic
296252
return TrinaryLogic::createFromBoolean($this->acceptsNamedArguments);
297253
}
298254

299-
private function isFunctionNodeVariadic(Function_ $node): bool
300-
{
301-
foreach ($node->params as $parameter) {
302-
if ($parameter->variadic) {
303-
return true;
304-
}
305-
}
306-
307-
if ($this->functionCallStatementFinder->findFunctionCallInStatements(ParametersAcceptor::VARIADIC_FUNCTIONS, $node->getStmts()) !== null) {
308-
return true;
309-
}
310-
311-
return false;
312-
}
313-
314255
}

tests/PHPStan/Parser/CleaningParserTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public function testParse(
6969
new Php7(new Emulative()),
7070
new NameResolver(),
7171
new VariadicMethodsVisitor(new FunctionCallStatementFinder()),
72+
new VariadicFunctionsVisitor(new FunctionCallStatementFinder()),
7273
),
7374
new PhpVersion($phpVersionId),
7475
);

0 commit comments

Comments
 (0)