Skip to content

Commit dda126f

Browse files
SanderMullerclaude
authored andcommitted
Rerun a rule on CollectedDataNode when what a class declares changed
A class reflection extension records with DeclarationDependencyTracker what a class declares depends on. A rule on CollectedDataNode can read such a class, but it is not known which classes it read. An extension also describes a class only once per process, so a second rule that reads the class records nothing. So a rule now gets the values of every class seen in the process. A changed value runs the rule again. A value of a class that the rule did not read causes an extra run only. Every rule now has the same values, so the finalizer asks for each current value once per run, not once per rule. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 1ccdd8a commit dda126f

12 files changed

Lines changed: 283 additions & 8 deletions

File tree

‎.github/workflows/e2e-tests.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,12 @@ jobs:
266266
OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
267267
echo "$OUTPUT"
268268
../bashunit assert contains 'Controller.php:20:View contact not found.' "$OUTPUT"
269+
# And a method that a class reflection extension describes from a file that is not
270+
# analysed, tracked through DeclarationDependencyTracker. No analysed file changes.
271+
printf '' > more-views.txt
272+
OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
273+
echo "$OUTPUT"
274+
../bashunit assert contains 'Controller.php:25:View faq not found.' "$OUTPUT"
269275
- script: |
270276
cd e2e/bug-14514
271277
composer install
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ResultCacheCollectedDataRulesTracking;
4+
5+
use PHPStan\Analyser\DeclarationDependencyTracker;
6+
use PHPStan\Reflection\ClassReflection;
7+
use PHPStan\Reflection\MethodReflection;
8+
use PHPStan\Reflection\MethodsClassReflectionExtension;
9+
use PHPStan\Type\StringType;
10+
use function file;
11+
use function in_array;
12+
13+
/**
14+
* MoreViews has a method for each view listed in a file that is not analysed. A class reflection
15+
* extension gets no Scope, so it tracks the file on the class.
16+
*/
17+
final class MoreViewsExtension implements MethodsClassReflectionExtension
18+
{
19+
20+
public function __construct(
21+
private DeclarationDependencyTracker $declarationDependencyTracker,
22+
private string $moreViewsFile,
23+
)
24+
{
25+
}
26+
27+
public function hasMethod(ClassReflection $classReflection, string $methodName): bool
28+
{
29+
if ($classReflection->getName() !== MoreViews::class) {
30+
return false;
31+
}
32+
33+
$this->declarationDependencyTracker->trackFileDependency($classReflection, $this->moreViewsFile);
34+
35+
return in_array($methodName, file($this->moreViewsFile, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES), true);
36+
}
37+
38+
public function getMethod(ClassReflection $classReflection, string $methodName): MethodReflection
39+
{
40+
return new ViewMethodReflection($classReflection, $methodName, new StringType());
41+
}
42+
43+
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ResultCacheCollectedDataRulesTracking;
4+
5+
use PHPStan\Reflection\ClassMemberReflection;
6+
use PHPStan\Reflection\ClassReflection;
7+
use PHPStan\Reflection\FunctionVariant;
8+
use PHPStan\Reflection\MethodReflection;
9+
use PHPStan\TrinaryLogic;
10+
use PHPStan\Type\Generic\TemplateTypeMap;
11+
use PHPStan\Type\Type;
12+
13+
final class ViewMethodReflection implements MethodReflection
14+
{
15+
16+
public function __construct(private ClassReflection $declaringClass, private string $name, private Type $returnType)
17+
{
18+
}
19+
20+
public function getDeclaringClass(): ClassReflection
21+
{
22+
return $this->declaringClass;
23+
}
24+
25+
public function isStatic(): bool
26+
{
27+
return false;
28+
}
29+
30+
public function isPrivate(): bool
31+
{
32+
return false;
33+
}
34+
35+
public function isPublic(): bool
36+
{
37+
return true;
38+
}
39+
40+
public function getDocComment(): ?string
41+
{
42+
return null;
43+
}
44+
45+
public function getName(): string
46+
{
47+
return $this->name;
48+
}
49+
50+
public function getPrototype(): ClassMemberReflection
51+
{
52+
return $this;
53+
}
54+
55+
public function getVariants(): array
56+
{
57+
return [new FunctionVariant(TemplateTypeMap::createEmpty(), null, [], false, $this->returnType)];
58+
}
59+
60+
public function isDeprecated(): TrinaryLogic
61+
{
62+
return TrinaryLogic::createNo();
63+
}
64+
65+
public function getDeprecatedDescription(): ?string
66+
{
67+
return null;
68+
}
69+
70+
public function isFinal(): TrinaryLogic
71+
{
72+
return TrinaryLogic::createNo();
73+
}
74+
75+
public function isInternal(): TrinaryLogic
76+
{
77+
return TrinaryLogic::createNo();
78+
}
79+
80+
public function getThrowType(): ?Type
81+
{
82+
return null;
83+
}
84+
85+
public function hasSideEffects(): TrinaryLogic
86+
{
87+
return TrinaryLogic::createNo();
88+
}
89+
90+
}

‎e2e/result-cache-collected-data-rules-tracking/extension/ViewRule.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
/**
1818
* Reports a view without a file in the views directory, unless the view is listed in a file of
1919
* ignored views or is a constant of ExtraViews. None of them is analysed, so the rule declares
20-
* all three.
20+
* all three. A view can also be a method of MoreViews, which MoreViewsExtension describes and
21+
* declares itself.
2122
*
2223
* @implements Rule<CollectedDataNode>
2324
*/
@@ -48,10 +49,11 @@ public function processNode(Node $node, Scope $scope): array
4849

4950
$ignoredViews = file($this->ignoredViewsFile, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
5051
$extraViews = $this->reflectionProvider->getClass(ExtraViews::class)->getNativeReflection()->getConstants();
52+
$moreViews = $this->reflectionProvider->getClass(MoreViews::class);
5153
$errors = [];
5254
foreach ($node->get(ViewCollector::class) as $file => $views) {
5355
foreach ($views as [$view, $line]) {
54-
if (is_file($this->viewsDirectory . '/' . $view . '.txt') || in_array($view, $ignoredViews, true) || in_array($view, $extraViews, true)) {
56+
if (is_file($this->viewsDirectory . '/' . $view . '.txt') || in_array($view, $ignoredViews, true) || in_array($view, $extraViews, true) || $moreViews->hasMethod($view)) {
5557
continue;
5658
}
5759

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ResultCacheCollectedDataRulesTracking;
4+
5+
/**
6+
* Has a method for each view in more-views.txt, from MoreViewsExtension.
7+
*/
8+
class MoreViews
9+
{
10+
11+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
faq

‎e2e/result-cache-collected-data-rules-tracking/phpstan.neon‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,9 @@ services:
2121
ignoredViewsFile: %currentWorkingDirectory%/ignored-views.txt
2222
tags:
2323
- phpstan.rules.rule
24+
-
25+
class: ResultCacheCollectedDataRulesTracking\MoreViewsExtension
26+
arguments:
27+
moreViewsFile: %currentWorkingDirectory%/more-views.txt
28+
tags:
29+
- phpstan.broker.methodsClassReflectionExtension

‎e2e/result-cache-collected-data-rules-tracking/src/Controller.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,9 @@ public function contact(): string
2020
return view('contact');
2121
}
2222

23+
public function faq(): string
24+
{
25+
return view('faq');
26+
}
27+
2328
}

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,14 @@ public function finalize(
105105
$collectedDataRuleResults = [];
106106
$reusedRulesCount = 0;
107107
$ruleCountsByClass = [];
108+
$currentValues = [];
108109
foreach ($this->ruleRegistry->getRules($nodeType) as $rule) {
109110
$ruleClass = get_class($rule);
110111
$ruleCountsByClass[$ruleClass] = ($ruleCountsByClass[$ruleClass] ?? 0) + 1;
111112
$ruleKey = sprintf('%s#%d', $ruleClass, $ruleCountsByClass[$ruleClass]);
112113

113114
$previousResult = $this->reuseCollectedDataRuleErrors ? $previousCollectedDataRuleResults[$ruleKey] ?? null : null;
114-
if ($previousResult !== null && $this->canReuse($previousResult, $onlyFiles, $hashCollector)) {
115+
if ($previousResult !== null && $this->canReuse($previousResult, $onlyFiles, $hashCollector, $currentValues)) {
115116
$collectedDataRuleResults[$ruleKey] = $previousResult;
116117
$reusedRulesCount++;
117118
$this->addRuleErrors($previousResult->getErrors(), $node, $scope, $tempCollectorErrors);
@@ -265,8 +266,9 @@ private function createRuleResult(CollectedDataNodeReads $reads, bool $onlyFiles
265266

266267
/**
267268
* @param Closure(string): string $hashCollector
269+
* @param array<string, string> $currentValues id => value, asked for once per run - every rule shares the values of the classes
268270
*/
269-
private function canReuse(CollectedDataRuleResult $previousResult, bool $onlyFiles, Closure $hashCollector): bool
271+
private function canReuse(CollectedDataRuleResult $previousResult, bool $onlyFiles, Closure $hashCollector, array &$currentValues): bool
270272
{
271273
$previousOnlyFiles = $previousResult->getOnlyFiles();
272274
if ($previousOnlyFiles !== null && $previousOnlyFiles !== $onlyFiles) {
@@ -286,7 +288,9 @@ private function canReuse(CollectedDataRuleResult $previousResult, bool $onlyFil
286288
return false;
287289
}
288290

289-
if ($extension->getValue($extension->keyFromResultCache($storedKey)) !== $value) {
291+
$id = ValueDependencyCollector::getId($extensionClass, $storedKey);
292+
$currentValues[$id] ??= $extension->getValue($extension->keyFromResultCache($storedKey));
293+
if ($currentValues[$id] !== $value) {
290294
return false;
291295
}
292296
}

‎src/Analyser/ValueDependencyCollector.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
* and reused by every file analysed after it without asking the extension again, so these are kept
5050
* for the whole process too, by class, and every file depending on the class - or on a class
5151
* extending it or implementing it - gets them as its own dependencies when its analysis finishes.
52+
* A rule for CollectedDataNode gets those of every class, because it is not known which classes
53+
* it read.
5254
*
5355
* @phpstan-type ValueDependencies = array{
5456
* values: array<string, array{string, string, string}>,
@@ -171,11 +173,23 @@ public function startCollectedDataRule(): void
171173
}
172174

173175
/**
176+
* With what every class seen in this process declares depends on, see recordForClass(): the
177+
* rule may have read any of them, and an extension describes a class only once per process.
178+
*
174179
* @return array<string, array{string, string, string}> id => [extension class, key, value]
175180
*/
176181
public function getCollectedDataRuleValues(): array
177182
{
178-
return $this->collectedDataRuleValues ?? [];
183+
if ($this->collectedDataRuleValues === null) {
184+
return [];
185+
}
186+
187+
$values = $this->collectedDataRuleValues;
188+
foreach ($this->classDeclarations as $classValues) {
189+
$values += $classValues;
190+
}
191+
192+
return $values;
179193
}
180194

181195
public function finishCollectedDataRule(): void

0 commit comments

Comments
 (0)