Skip to content

Commit d5b1a1d

Browse files
ondrejmirtesondrejmirtes
committed
Report the scanned files whose symbols changed, not every scanned file that changed
The message counted every entry the scannedFiles metadata diff produced and said the files depending on them were being re-analysed. Since the exported nodes of a non-analysed file are compared, neither half is true any more: a scanned file that changed without any of its symbols changing re-analyses nothing, a file that is gone re-analyses nothing here either, and the files depending on an edited one are reached by the dependency graph rather than by the metadata. A body-only edit of a scanned file announced that its dependents were being re-analysed and then re-analysed none. The message now counts the scanned files whose exported nodes actually differ from the cached ones, collected from both places that can tell: the metadata loop, for a file that appeared or that nothing depends on, and the loop over the non-analysed files, which compares the nodes of an edited one. Nothing changed means no message at all.
1 parent ed73837 commit d5b1a1d

2 files changed

Lines changed: 29 additions & 16 deletions

File tree

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ jobs:
245245
patch -b src/Generated/Foo.php < patch.patch
246246
OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
247247
echo "$OUTPUT"
248-
../bashunit -a contains 'Scanned files changed (1); re-analysing only the files depending on them.' "$OUTPUT"
248+
../bashunit -a contains 'Scanned files with changed symbols (1); re-analysing the files affected by them.' "$OUTPUT"
249249
../bashunit -a contains 'Result cache restored. 1 file will be reanalysed.' "$OUTPUT"
250250
../bashunit -a contains 'Instantiated class ResultCacheE2EGenerated\Foo not found.' "$OUTPUT"
251251
- script: |
@@ -308,7 +308,7 @@ jobs:
308308
mv new-file/NewlyScanned.php scanned/NewlyScanned.php
309309
OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
310310
echo "$OUTPUT"
311-
../bashunit -a contains 'Scanned files changed (1); re-analysing only the files depending on them.' "$OUTPUT"
311+
../bashunit -a contains 'Scanned files with changed symbols (1); re-analysing the files affected by them.' "$OUTPUT"
312312
../bashunit -a contains 'Result cache restored. 1 file will be reanalysed.' "$OUTPUT"
313313
../bashunit -a not_contains 'Instantiated class ResultCacheE2EScannedNewFile\NewlyScanned not found.' "$OUTPUT"
314314
../bashunit -a contains 'UsesNew.php:10:Method ResultCacheE2EScannedNewFile\UsesNew::doUsesNew() should return int but returns string.' "$OUTPUT"
@@ -326,10 +326,11 @@ jobs:
326326
OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
327327
echo "$OUTPUT"
328328
../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT"
329+
../bashunit -a not_contains 'Scanned files with changed symbols' "$OUTPUT"
329330
# The new contents are recorded: the next run has nothing to notice.
330331
OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
331332
echo "$OUTPUT"
332-
../bashunit -a not_contains 'Scanned files changed' "$OUTPUT"
333+
../bashunit -a not_contains 'Scanned files with changed symbols' "$OUTPUT"
333334
../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT"
334335
# A changed return type is an exported node change: the file depending on it is
335336
# re-analysed. A changed signature cannot make a new symbol available, so the file with

‎src/Analyser/ResultCache/ResultCacheManager.php‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
331331
$packageDependencies = $data['packageDependencies'];
332332
$packageSeededFiles = [];
333333
$notAnalysedFileSymbolsChanged = false;
334+
// Scanned files whose exported nodes differ from the ones the cache holds - the only ones that
335+
// make anything be re-analysed. A scanned file can change without any of its symbols changing,
336+
// and then nothing here has to happen at all.
337+
$scannedFilesWithChangedSymbols = [];
334338
if ($this->isMetaDifferent($data['meta'], $meta)) {
335339
$diffs = $this->getMetaKeyDifferences($data['meta'], $meta);
336340

@@ -353,12 +357,13 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
353357
// (NodeDependencies::getNonAnalysedDependencies()) along with their exported nodes, so the
354358
// loop over the not-analysed files below treats an edited one the way the loop above
355359
// treats an analysed file: the files depending on it are re-analysed only when its
356-
// exported nodes changed. What is left for the metadata to notice is a file the
357-
// dependency graph does not know: one that appeared, or an edited one nothing depends
358-
// on. Either may define a symbol that was reported as unknown somewhere - if it declares
359-
// any symbol at all - so the files with errors are re-analysed the same way a new
360-
// analysed file makes them re-analysed. The previous nodes of a file nothing depends on
361-
// are not kept, so for it "declares a symbol" stands in for "declares a new symbol".
360+
// exported nodes changed, and it is that loop which counts it as changed. What is left
361+
// for the metadata to notice is a file the dependency graph does not know: one that
362+
// appeared, or an edited one nothing depends on. Either may define a symbol that was
363+
// reported as unknown somewhere - if it declares any symbol at all - so the files with
364+
// errors are re-analysed the same way a new analysed file makes them re-analysed. The
365+
// previous nodes of a file nothing depends on are not kept, so for it "declares a symbol"
366+
// stands in for "declares a new symbol".
362367
$changedScannedFiles = $this->getChangedScannedFiles($data['meta'], $meta);
363368
foreach ($changedScannedFiles as $changedScannedFile => $change) {
364369
// The scanned files getNonAnalysedDependencies() leaves out: nothing would
@@ -392,15 +397,9 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
392397
continue;
393398
}
394399

400+
$scannedFilesWithChangedSymbols[$changedScannedFile] = true;
395401
$notAnalysedFileSymbolsChanged = true;
396402
}
397-
398-
if ($output->isVeryVerbose()) {
399-
$output->writeLineFormatted(sprintf(
400-
'Scanned files changed (%d); re-analysing only the files depending on them.',
401-
count($changedScannedFiles),
402-
));
403-
}
404403
}
405404

406405
// The generated container and the analysis are unchanged except for code coming from packages
@@ -647,6 +646,8 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
647646
}
648647
}
649648

649+
// the freshly computed metadata, so a file that stopped being scanned is not counted
650+
$scannedFiles = is_array($meta['scannedFiles'] ?? null) ? $meta['scannedFiles'] : [];
650651
foreach (array_keys($notAnalysedFiles) as $notAnalysedFile) {
651652
if (!array_key_exists($notAnalysedFile, $invertedDependencies)) {
652653
continue;
@@ -697,6 +698,10 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
697698
continue;
698699
}
699700

701+
if (array_key_exists($notAnalysedFile, $scannedFiles)) {
702+
$scannedFilesWithChangedSymbols[$notAnalysedFile] = true;
703+
}
704+
700705
if ($exportedNodesChanged) {
701706
// A symbol appeared or disappeared. The dependents are not enough then: a file that
702707
// reported the symbol as unknown - or referenced this file while it had a syntax error
@@ -729,6 +734,13 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
729734
$filesToAnalyse[] = $packageSeededFile;
730735
}
731736

737+
if (count($scannedFilesWithChangedSymbols) > 0 && $output->isVeryVerbose()) {
738+
$output->writeLineFormatted(sprintf(
739+
'Scanned files with changed symbols (%d); re-analysing the files affected by them.',
740+
count($scannedFilesWithChangedSymbols),
741+
));
742+
}
743+
732744
$filesToAnalyse = array_unique($filesToAnalyse);
733745
$filesToAnalyseCount = count($filesToAnalyse);
734746

0 commit comments

Comments
 (0)