Skip to content

Commit b6b5518

Browse files
SanderMullerclaude
SanderMuller
and
claude
committed
Reuse the errors of rules on CollectedDataNode from the result cache
CollectedDataNode records which collectors a rule reads and whether it reads isOnlyFilesAnalysis(). The result cache stores the errors of each rule with a hash of that input. On the next run, a rule whose input is the same returns its stored errors and does not run again. The ignore extensions and the local ignores still run on every error, also on reused errors. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent d86cee3 commit b6b5518

25 files changed

Lines changed: 783 additions & 33 deletions

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,29 @@ jobs:
215215
OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan -vvv")
216216
echo "$OUTPUT"
217217
../bashunit assert contains 'Result cache restored.' "$OUTPUT"
218+
- script: |
219+
cd e2e/result-cache-collected-data-rules
220+
OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
221+
echo "$OUTPUT"
222+
../bashunit assert contains 'Caller.php:10:Call to function ResultCacheCollectedDataRules\pure() on a separate line has no effect.' "$OUTPUT"
223+
# Nothing changed, so every rule on CollectedDataNode reuses its errors from the result cache.
224+
OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
225+
echo "$OUTPUT"
226+
../bashunit assert contains 'Caller.php:10:Call to function ResultCacheCollectedDataRules\pure() on a separate line has no effect.' "$OUTPUT"
227+
echo "$OUTPUT" | grep -E 'Result cache reused the errors of ([0-9]+) of \1 rules on collected data\.'
228+
../bashunit assert contains 'Result cache was not rewritten because it is unchanged.' "$OUTPUT"
229+
# src/Other.php is analysed again, but what it collects stays the same.
230+
echo '// changed' >> src/Other.php
231+
OUTPUT=$(../bashunit assert exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
232+
echo "$OUTPUT"
233+
../bashunit assert contains 'Result cache restored. 1 file will be reanalysed.' "$OUTPUT"
234+
../bashunit assert contains 'Caller.php:10:Call to function ResultCacheCollectedDataRules\pure() on a separate line has no effect.' "$OUTPUT"
235+
echo "$OUTPUT" | grep -E 'Result cache reused the errors of ([0-9]+) of \1 rules on collected data\.'
236+
# Removing the call changes what src/Caller.php collects, so the rule runs again and the error goes away.
237+
sed -i 's/^\t\tpure();$//' src/Caller.php
238+
OUTPUT=$(../bashunit assert exit_code "0" "../../bin/phpstan analyse -vv --error-format raw")
239+
echo "$OUTPUT"
240+
../bashunit assert not_contains 'on a separate line has no effect' "$OUTPUT"
218241
- script: |
219242
cd e2e/bug-14514
220243
composer install

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ e2e/bashunit
2222
/e2e/result-cache-moved-tmpdir/tmp-a
2323
/e2e/result-cache-moved-tmpdir/tmp-b
2424
/e2e/result-cache-stale-objects/tmp
25+
/e2e/result-cache-collected-data-rules/tmp
2526
/e2e/result-cache-truncated/tmp
2627
/e2e/result-cache-worker-count/tmp
2728
e2e/restart-manager/probe.log

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
parameters:
2+
level: 8
3+
tmpDir: tmp
4+
paths:
5+
- src

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ResultCacheCollectedDataRules;
4+
5+
class Caller
6+
{
7+
8+
public function doFoo(): void
9+
{
10+
pure();
11+
}
12+
13+
}

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ResultCacheCollectedDataRules;
4+
5+
class Other
6+
{
7+
8+
public function doBar(): int
9+
{
10+
return 1;
11+
}
12+
13+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace ResultCacheCollectedDataRules;
4+
5+
function pure(): int
6+
{
7+
return 1;
8+
}

‎src/Analyser/AnalyserResultFinalizer.php‎

Lines changed: 108 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Analyser;
44

5+
use Closure;
56
use PHPStan\AnalysedCodeException;
67
use PHPStan\BetterReflection\NodeCompiler\Exception\UnableToCompileNode;
78
use PHPStan\BetterReflection\Reflection\Exception\CircularReference;
@@ -11,11 +12,16 @@
1112
use PHPStan\DependencyInjection\AutowiredService;
1213
use PHPStan\DependencyInjection\ExtensionsCollection;
1314
use PHPStan\Node\CollectedDataNode;
15+
use PHPStan\Node\CollectedDataNodeReads;
1416
use PHPStan\Rules\Registry as RuleRegistry;
1517
use Throwable;
18+
use function array_key_exists;
1619
use function array_merge;
1720
use function count;
1821
use function get_class;
22+
use function ksort;
23+
use function md5;
24+
use function serialize;
1925
use function sprintf;
2026

2127
#[AutowiredService]
@@ -38,7 +44,11 @@ public function __construct(
3844
{
3945
}
4046

41-
public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug): FinalizerResult
47+
/**
48+
* @param array<string, CollectedDataRuleResult> $previousCollectedDataRuleResults the rule results of the
49+
* previous run, from the result cache - a rule whose input did not change reuses its errors
50+
*/
51+
public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $debug, array $previousCollectedDataRuleResults = []): FinalizerResult
4252
{
4353
if (count($analyserResult->getCollectedData()) === 0) {
4454
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
@@ -50,15 +60,52 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
5060
}
5161

5262
$nodeType = CollectedDataNode::class;
53-
$node = new CollectedDataNode($analyserResult->getCollectedData(), $onlyFiles);
63+
$collectedData = $analyserResult->getCollectedData();
64+
$node = new CollectedDataNode($collectedData, $onlyFiles);
65+
66+
// Hashed once per collector type and run, in the same order however the files were merged.
67+
$collectorHashes = [];
68+
$hashCollector = static function (string $collectorType) use ($collectedData, &$collectorHashes): string {
69+
if (!array_key_exists($collectorType, $collectorHashes)) {
70+
$data = [];
71+
foreach ($collectedData as $filePath => $collectedDataPerCollector) {
72+
if (!isset($collectedDataPerCollector[$collectorType])) {
73+
continue;
74+
}
75+
76+
$data[$filePath] = $collectedDataPerCollector[$collectorType];
77+
}
78+
ksort($data);
79+
$collectorHashes[$collectorType] = md5(serialize($data));
80+
}
81+
82+
return $collectorHashes[$collectorType];
83+
};
5484

5585
$file = 'N/A';
5686
$scope = $this->scopeFactory->create(ScopeContext::create($file));
5787
$tempCollectorErrors = [];
5888
$internalErrors = $analyserResult->getInternalErrors();
89+
$collectedDataRuleResults = [];
90+
$reusedRulesCount = 0;
91+
$ruleCountsByClass = [];
5992
foreach ($this->ruleRegistry->getRules($nodeType) as $rule) {
93+
$ruleClass = get_class($rule);
94+
$ruleCountsByClass[$ruleClass] = ($ruleCountsByClass[$ruleClass] ?? 0) + 1;
95+
$ruleKey = sprintf('%s#%d', $ruleClass, $ruleCountsByClass[$ruleClass]);
96+
97+
$previousResult = $previousCollectedDataRuleResults[$ruleKey] ?? null;
98+
if ($previousResult !== null && $this->canReuse($previousResult, $onlyFiles, $hashCollector)) {
99+
$collectedDataRuleResults[$ruleKey] = $previousResult;
100+
$reusedRulesCount++;
101+
$this->addRuleErrors($previousResult->getErrors(), $node, $scope, $tempCollectorErrors);
102+
continue;
103+
}
104+
105+
$reads = new CollectedDataNodeReads();
106+
$ruleNode = new CollectedDataNode($collectedData, $onlyFiles, $reads);
60107
try {
61-
$ruleErrors = $rule->processNode($node, $scope);
108+
$ruleErrors = $rule->processNode($ruleNode, $scope);
62109
} catch (AnalysedCodeException $e) {
63110
$tempCollectorErrors[] = (new Error($e->getMessage(), $file, $node->getStartLine(), $e, tip: $e->getTip()))
64111
->withIdentifier('phpstan.internal')
@@ -98,19 +145,22 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
98145
continue;
99146
}
100147

148+
$errors = [];
101149
foreach ($ruleErrors as $ruleError) {
102-
$error = $this->ruleErrorTransformer->transform($ruleError, $scope, [], $node);
103-
104-
if ($error->canBeIgnored()) {
105-
foreach ($this->ignoreErrorExtensions->getAll() as $ignoreErrorExtension) {
106-
if ($ignoreErrorExtension->shouldIgnore($error, $node, $scope)) {
107-
continue 2;
108-
}
109-
}
110-
}
150+
$errors[] = $this->ruleErrorTransformer->transform($ruleError, $scope, [], $ruleNode);
151+
}
111152

112-
$tempCollectorErrors[] = $error;
153+
$readCollectorHashes = [];
154+
foreach ($reads->getCollectorTypes() as $collectorType) {
155+
$readCollectorHashes[$collectorType] = $hashCollector($collectorType);
113156
}
157+
$collectedDataRuleResults[$ruleKey] = new CollectedDataRuleResult(
158+
$readCollectorHashes,
159+
$reads->isOnlyFilesRead() ? $onlyFiles : null,
160+
$errors,
161+
);
162+
163+
$this->addRuleErrors($errors, $node, $scope, $tempCollectorErrors);
114164
}
115165

116166
$errors = $analyserResult->getUnorderedErrors();
@@ -158,7 +208,45 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles, bool $
158208
peakMemoryUsageBytes: $analyserResult->getPeakMemoryUsageBytes(),
159209
processedFiles: $analyserResult->getProcessedFiles(),
160210
workerCount: $analyserResult->getWorkerCount(),
161-
), $collectorErrors, $locallyIgnoredCollectorErrors);
211+
), $collectorErrors, $locallyIgnoredCollectorErrors, $collectedDataRuleResults, $reusedRulesCount);
212+
}
213+
214+
/**
215+
* @param Closure(string): string $hashCollector
216+
*/
217+
private function canReuse(CollectedDataRuleResult $previousResult, bool $onlyFiles, Closure $hashCollector): bool
218+
{
219+
$previousOnlyFiles = $previousResult->getOnlyFiles();
220+
if ($previousOnlyFiles !== null && $previousOnlyFiles !== $onlyFiles) {
221+
return false;
222+
}
223+
224+
foreach ($previousResult->getCollectorHashes() as $collectorType => $hash) {
225+
if ($hashCollector($collectorType) !== $hash) {
226+
return false;
227+
}
228+
}
229+
230+
return true;
231+
}
232+
233+
/**
234+
* @param list<Error> $errors
235+
* @param list<Error> $tempCollectorErrors
236+
*/
237+
private function addRuleErrors(array $errors, CollectedDataNode $node, Scope $scope, array &$tempCollectorErrors): void
238+
{
239+
foreach ($errors as $error) {
240+
if ($error->canBeIgnored()) {
241+
foreach ($this->ignoreErrorExtensions->getAll() as $ignoreErrorExtension) {
242+
if ($ignoreErrorExtension->shouldIgnore($error, $node, $scope)) {
243+
continue 2;
244+
}
245+
}
246+
}
247+
248+
$tempCollectorErrors[] = $error;
249+
}
162250
}
163251

164252
private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): AnalyserResult
@@ -187,15 +275,18 @@ private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): Analyse
187275
/**
188276
* @param list<Error> $collectorErrors
189277
* @param list<Error> $locallyIgnoredCollectorErrors
278+
* @param array<string, CollectedDataRuleResult> $collectedDataRuleResults
190279
*/
191280
private function addUnmatchedIgnoredErrors(
192281
AnalyserResult $analyserResult,
193282
array $collectorErrors,
194283
array $locallyIgnoredCollectorErrors,
284+
array $collectedDataRuleResults = [],
285+
int $reusedRulesCount = 0,
195286
): FinalizerResult
196287
{
197288
if (!$this->reportUnmatchedIgnoredErrors) {
198-
return new FinalizerResult($analyserResult, $collectorErrors, $locallyIgnoredCollectorErrors);
289+
return new FinalizerResult($analyserResult, $collectorErrors, $locallyIgnoredCollectorErrors, $collectedDataRuleResults, $reusedRulesCount);
199290
}
200291

201292
$errors = $analyserResult->getUnorderedErrors();
@@ -252,6 +343,8 @@ private function addUnmatchedIgnoredErrors(
252343
),
253344
$collectorErrors,
254345
$locallyIgnoredCollectorErrors,
346+
$collectedDataRuleResults,
347+
$reusedRulesCount,
255348
);
256349
}
257350

‎src/Analyser/CollectedDataRuleResult.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Analyser;
4+
5+
/**
6+
* The errors of one rule on CollectedDataNode, with what the rule asked from the node to get them:
7+
* a hash of the data of every collector it asked for, and the value of isOnlyFilesAnalysis() if it
8+
* asked for that. While all of it is the same, the next run reuses the errors instead of running the
9+
* rule again.
10+
*/
11+
final class CollectedDataRuleResult
12+
{
13+
14+
/**
15+
* @param array<string, string> $collectorHashes collector type => hash of its data
16+
* @param list<Error> $errors before the ignore extensions and the local ignores
17+
*/
18+
public function __construct(
19+
private array $collectorHashes,
20+
private ?bool $onlyFiles,
21+
private array $errors,
22+
)
23+
{
24+
}
25+
26+
/**
27+
* @return array<string, string>
28+
*/
29+
public function getCollectorHashes(): array
30+
{
31+
return $this->collectorHashes;
32+
}
33+
34+
/**
35+
* Null when the rule did not ask.
36+
*/
37+
public function getOnlyFiles(): ?bool
38+
{
39+
return $this->onlyFiles;
40+
}
41+
42+
/**
43+
* @return list<Error>
44+
*/
45+
public function getErrors(): array
46+
{
47+
return $this->errors;
48+
}
49+
50+
/**
51+
* @param callable(string): string $transform
52+
*/
53+
public function transformPaths(callable $transform): self
54+
{
55+
$errors = [];
56+
foreach ($this->errors as $error) {
57+
$errors[] = $error->transformPaths($transform);
58+
}
59+
60+
return new self($this->collectorHashes, $this->onlyFiles, $errors);
61+
}
62+
63+
}

‎src/Analyser/Error.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ private function transformTraitContexts(callable $transformPath): array
470470
{
471471
$result = [];
472472
foreach ($this->traitContexts as $contextFilePath => $contextFile) {
473-
$result[$transformPath($contextFilePath)] = $contextFile;
473+
$result[$transformPath($contextFilePath)] = $transformPath($contextFile);
474474
}
475475

476476
return $result;

‎src/Analyser/FinalizerResult.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@ final class FinalizerResult
88
/**
99
* @param list<Error> $collectorErrors
1010
* @param list<Error> $locallyIgnoredCollectorErrors
11+
* @param array<string, CollectedDataRuleResult> $collectedDataRuleResults
1112
*/
1213
public function __construct(
1314
private AnalyserResult $analyserResult,
1415
private array $collectorErrors,
1516
private array $locallyIgnoredCollectorErrors,
17+
private array $collectedDataRuleResults = [],
18+
private int $reusedRulesCount = 0,
1619
)
1720
{
1821
}
@@ -46,4 +49,20 @@ public function getLocallyIgnoredCollectorErrors(): array
4649
return $this->locallyIgnoredCollectorErrors;
4750
}
4851

52+
/**
53+
* @return array<string, CollectedDataRuleResult>
54+
*/
55+
public function getCollectedDataRuleResults(): array
56+
{
57+
return $this->collectedDataRuleResults;
58+
}
59+
60+
/**
61+
* How many of the rules on CollectedDataNode reused their errors from the result cache.
62+
*/
63+
public function getReusedRulesCount(): int
64+
{
65+
return $this->reusedRulesCount;
66+
}
67+
4968
}

0 commit comments

Comments
 (0)