Skip to content

Commit 5586806

Browse files
SanderMullerclaude
authored andcommitted
Run a rule on CollectedDataNode every time when it does not call get()
Such a rule works with something other than the collected data, for example a tool that it runs over the whole project. Its errors were reused on every warm run. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 034afd3 commit 5586806

7 files changed

Lines changed: 96 additions & 3 deletions

File tree

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ public function __construct(
5555
* @param Closure(): void|null $postRuleCallback called when the rule's processNode() has returned or thrown, before its errors are processed
5656
* @param array<string, CollectedDataRuleResult> $previousCollectedDataRuleResults the rule results of the
5757
* previous run, from the result cache - a rule whose input did not change reuses its errors.
58-
* Only with the reuseCollectedDataRuleErrors feature toggle, otherwise every rule runs.
58+
* Only with the reuseCollectedDataRuleErrors feature toggle, otherwise every rule runs. A rule that
59+
* does not call get() on the node always runs.
5960
*/
6061
public function finalize(
6162
AnalyserResult $analyserResult,
@@ -179,7 +180,8 @@ public function finalize(
179180
$errors[] = $this->ruleErrorTransformer->transform($ruleError, $scope, [], $ruleNode);
180181
}
181182

182-
if ($this->reuseCollectedDataRuleErrors) {
183+
// a rule that never asked for collected data works with something else, so it runs every time
184+
if ($this->reuseCollectedDataRuleErrors && $reads->getCollectorTypes() !== []) {
183185
$collectedDataRuleResults[$ruleKey] = $this->createRuleResult($reads, $onlyFiles, $hashCollector, $errors, $ruleValueDependencies);
184186
}
185187

‎src/Analyser/DependencyTracker.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
* the result cache re-analyses the file when that changes - see ResultCacheValueExtension.
2626
* In a rule for CollectedDataNode, it tracks what the errors of the rule depend on: with the
2727
* reuseCollectedDataRuleErrors feature toggle, the result cache reuses the errors of such a rule
28-
* while its collected data and the tracked values stay the same.
28+
* while its collected data and the tracked values stay the same. A rule that does not call get() on
29+
* the node runs every time.
2930
*
3031
* Extensions that get no Scope because they describe a class - class reflection extensions - use
3132
* DeclarationDependencyTracker instead.

‎tests/PHPStan/Analyser/CollectedDataReuse/DescribedClassRule.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ public function processNode(Node $node, Scope $scope): array
3434
{
3535
$this->runs++;
3636

37+
// a rule that does not ask for collected data runs every time
38+
$node->get(FirstCollector::class);
3739
$contents = file_get_contents($this->file);
3840

3941
return [
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser\CollectedDataReuse;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\DependencyTracker;
7+
use PHPStan\Analyser\Scope;
8+
use PHPStan\Node\CollectedDataNode;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use function file_get_contents;
12+
use function sprintf;
13+
14+
/**
15+
* Asks only whether the analysis is files-only, and reads a file, like a rule that runs a tool over
16+
* the whole project. It tracks the file when told to.
17+
*
18+
* @implements Rule<CollectedDataNode>
19+
*/
20+
final class NoCollectedDataRule implements Rule
21+
{
22+
23+
public int $runs = 0;
24+
25+
public function __construct(private string $file, private bool $track)
26+
{
27+
}
28+
29+
public function getNodeType(): string
30+
{
31+
return CollectedDataNode::class;
32+
}
33+
34+
/**
35+
* @param Scope&DependencyTracker $scope
36+
*/
37+
public function processNode(Node $node, Scope $scope): array
38+
{
39+
$this->runs++;
40+
41+
if ($node->isOnlyFilesAnalysis()) {
42+
return [];
43+
}
44+
45+
if ($this->track) {
46+
$scope->trackFileDependency($this->file);
47+
}
48+
49+
$contents = file_get_contents($this->file);
50+
51+
return [
52+
RuleErrorBuilder::message(sprintf('Tool says %s.', $contents === false ? 'nothing' : $contents))
53+
->file(__FILE__)
54+
->line(1)
55+
->identifier('tests.noCollectedData')
56+
->build(),
57+
];
58+
}
59+
60+
}

‎tests/PHPStan/Analyser/CollectedDataReuse/OnlyFilesRule.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ public function processNode(Node $node, Scope $scope): array
2525
{
2626
$this->runs++;
2727

28+
// a rule that does not ask for collected data runs every time
29+
$node->get(FirstCollector::class);
2830
if ($node->isOnlyFilesAnalysis()) {
2931
return [];
3032
}

‎tests/PHPStan/Analyser/CollectedDataReuse/TrackingRule.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ public function processNode(Node $node, Scope $scope): array
4141
{
4242
$this->runs++;
4343

44+
// a rule that does not ask for collected data runs every time
45+
$node->get(FirstCollector::class);
4446
$scope->trackFileDependency($this->file);
4547
$scope->trackDirectoryDependency($this->directory, '*.txt');
4648
$scope->trackValueDependency(FileResultCacheValueExtension::class, $this->valueFile);

‎tests/PHPStan/Analyser/CollectedDataRuleReuseTest.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPStan\Analyser\CollectedDataReuse\DescribedClassRule;
66
use PHPStan\Analyser\CollectedDataReuse\EarlyReturnRule;
77
use PHPStan\Analyser\CollectedDataReuse\FirstCollector;
8+
use PHPStan\Analyser\CollectedDataReuse\NoCollectedDataRule;
89
use PHPStan\Analyser\CollectedDataReuse\OnlyFilesRule;
910
use PHPStan\Analyser\CollectedDataReuse\SecondCollector;
1011
use PHPStan\Analyser\CollectedDataReuse\ThrowingRule;
@@ -118,6 +119,29 @@ public function testOnlyFilesAnalysis(): void
118119
$this->assertSame([], $this->getMessages($result));
119120
}
120121

122+
public function testRuleThatDoesNotAskForCollectedDataRunsEveryTime(): void
123+
{
124+
$file = sys_get_temp_dir() . '/phpstan-collected-data-reuse-' . uniqid() . '.txt';
125+
file_put_contents($file, 'one');
126+
$collectedData = [
127+
self::A => [FirstCollector::class => ['a1']],
128+
];
129+
130+
// tracking a value is not enough, the rule has to ask for collected data
131+
foreach ([false, true] as $track) {
132+
$rule = new NoCollectedDataRule($file, $track);
133+
$result = $this->finalize([$rule], $collectedData, false, []);
134+
$this->assertSame([], $result->getCollectedDataRuleResults());
135+
136+
$result = $this->finalize([$rule], $collectedData, false, $result->getCollectedDataRuleResults());
137+
$this->assertSame(2, $rule->runs);
138+
$this->assertSame(0, $result->getReusedRulesCount());
139+
$this->assertSame(['Tool says one.'], $this->getMessages($result));
140+
}
141+
142+
unlink($file);
143+
}
144+
121145
public function testRuleThatThrowsIsNotReused(): void
122146
{
123147
$rule = new ThrowingRule();

0 commit comments

Comments
 (0)