Description
The result cache can keep an error reported by a rule built on collected data after the reason for it is gone, so the cached run and a run with a cleared cache disagree.
Rules registered for CollectedDataNode run in AnalyserResultFinalizer, which happens after ResultCacheManager::process() has already written the cache. Their errors are therefore never in the cached error map. That map is what restore() sweeps when a new file appears, which is the recovery path for an error that could record no dependency of its own - an error about a symbol that did not exist has nothing to point at. A file whose only error comes from collected data is never in that sweep, so nothing re-analyses it, its collected data stays as it was, and the rule keeps reporting from stale input.
A failing E2E test is in phpstan/phpstan-src#6358.
Reproduction
Three analysed files, level 4.
src/globalfns.php
<?php declare(strict_types = 1);
function helper(): int
{
return 1;
}
src/Caller.php - its only error comes from a collector-backed rule. The unqualified call falls back to the global helper(), which is pure, so CallToFunctionStatementWithoutImpurePointsRule reports function.resultUnused.
<?php declare(strict_types = 1);
namespace App;
function caller(): void
{
helper();
}
src/PlainCaller.php - an ordinary error, so that the sweep can be seen to run at all.
<?php declare(strict_types = 1);
namespace App;
class PlainCaller
{
public function m(): ?Missing
{
return null;
}
}
Run PHPStan: two errors. Then create src/nsfns.php, which defines both missing symbols at once - an impure App\helper() that the unqualified call now resolves to, and App\Missing:
<?php declare(strict_types = 1);
namespace App;
function helper(): int
{
echo 'side effect';
return 1;
}
class Missing
{
}
Run PHPStan again.
Expected output
No errors. Both errors are about symbols the new file defines.
Actual output
One error, and PHPStan exits 1:
src/Caller.php:7: Call to function helper() on a separate line has no effect.
PlainCaller.php's error is gone, which shows the sweep ran - -vv says Result cache restored. 2 files will be reanalysed. Clearing the cache and running again reports nothing at all, so the cached result is wrong rather than merely conservative.
Note
Collector rules do recompute globally from the merged collected data on every run, which is why this is not visible more often. What is stale is the merged data of a file that was not re-analysed, not the rule's own bookkeeping.
Found while auditing the result cache for this class of hole; the other five holes found in the same pass are fixed in phpstan/phpstan-src#6357.
Description
The result cache can keep an error reported by a rule built on collected data after the reason for it is gone, so the cached run and a run with a cleared cache disagree.
Rules registered for
CollectedDataNoderun inAnalyserResultFinalizer, which happens afterResultCacheManager::process()has already written the cache. Their errors are therefore never in the cached error map. That map is whatrestore()sweeps when a new file appears, which is the recovery path for an error that could record no dependency of its own - an error about a symbol that did not exist has nothing to point at. A file whose only error comes from collected data is never in that sweep, so nothing re-analyses it, its collected data stays as it was, and the rule keeps reporting from stale input.A failing E2E test is in phpstan/phpstan-src#6358.
Reproduction
Three analysed files, level 4.
src/globalfns.phpsrc/Caller.php- its only error comes from a collector-backed rule. The unqualified call falls back to the globalhelper(), which is pure, soCallToFunctionStatementWithoutImpurePointsRulereportsfunction.resultUnused.src/PlainCaller.php- an ordinary error, so that the sweep can be seen to run at all.Run PHPStan: two errors. Then create
src/nsfns.php, which defines both missing symbols at once - an impureApp\helper()that the unqualified call now resolves to, andApp\Missing:Run PHPStan again.
Expected output
No errors. Both errors are about symbols the new file defines.
Actual output
One error, and PHPStan exits 1:
PlainCaller.php's error is gone, which shows the sweep ran --vvsaysResult cache restored. 2 files will be reanalysed.Clearing the cache and running again reports nothing at all, so the cached result is wrong rather than merely conservative.Note
Collector rules do recompute globally from the merged collected data on every run, which is why this is not visible more often. What is stale is the merged data of a file that was not re-analysed, not the rule's own bookkeeping.
Found while auditing the result cache for this class of hole; the other five holes found in the same pass are fixed in phpstan/phpstan-src#6357.