diff --git a/src/Caching/UnchangedFilesFilter.php b/src/Caching/UnchangedFilesFilter.php index d0f3860b15e..d02d27e8cc2 100644 --- a/src/Caching/UnchangedFilesFilter.php +++ b/src/Caching/UnchangedFilesFilter.php @@ -5,6 +5,8 @@ namespace Rector\Caching; use Rector\Caching\Detector\ChangedFilesDetector; +use Rector\Configuration\Option; +use Rector\Configuration\Parameter\SimpleParameterProvider; final readonly class UnchangedFilesFilter { @@ -31,6 +33,12 @@ public function filterFilePaths(array $filePaths): array $this->changedFilesDetector->invalidateFile($filePath); } + // some files were served from cache, so rules ran on a subset only - unused skip reporting + // would then flag every cached file's skip as falsely unused + if (count($changedFileInfos) < count($filePaths)) { + SimpleParameterProvider::setParameter(Option::IS_CACHED_RUN, true); + } + return $changedFileInfos; } } diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index 947e6bb1937..0fccaa3d555 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -118,6 +118,15 @@ final class Option */ public const string IS_RUN_NARROWED = 'is_run_narrowed'; + /** + * True when the unchanged-files cache dropped at least one file from the run, so rules only ran + * on the changed subset. Unused skip reporting is then disabled, as skips on cached files never + * get a chance to match and would all look falsely unused. + * + * @internal + */ + public const string IS_CACHED_RUN = 'is_cached_run'; + /** * @internal Use RectorConfig::fileExtensions() instead */ diff --git a/src/Reporting/MissConfigurationReporter.php b/src/Reporting/MissConfigurationReporter.php index f62aab65275..0b011de9b80 100644 --- a/src/Reporting/MissConfigurationReporter.php +++ b/src/Reporting/MissConfigurationReporter.php @@ -40,7 +40,10 @@ public function reportUnusedSkips(ProcessResult $processResult): void count($unusedSkips) > 1 ? 'them' : 'it' )); - $this->symfonyStyle->listing($unusedSkips); + // add a blank line between items, so grouped rule skips stay visually separated + $spacedUnusedSkips = array_map(static fn (string $unusedSkip): string => $unusedSkip . "\n", $unusedSkips); + + $this->symfonyStyle->listing($spacedUnusedSkips); } public function reportSkippedNeverRegisteredRules(): void diff --git a/src/Reporting/UnusedSkipResolver.php b/src/Reporting/UnusedSkipResolver.php index 90062f0d20b..2c52ce10570 100644 --- a/src/Reporting/UnusedSkipResolver.php +++ b/src/Reporting/UnusedSkipResolver.php @@ -43,6 +43,12 @@ public function resolve(ProcessResult $processResult): array return []; } + // a cached run only re-processes changed files, so skips on cached files never get a chance + // to match and would all look falsely unused - reporting them would be noise + if (SimpleParameterProvider::provideBoolParameter(Option::IS_CACHED_RUN, false)) { + return []; + } + // map of rule => (trackable skip path => relative display path); skips are tracked at // runtime by their path, but rule-scoped ones are printed grouped under their rule so the // user knows exactly what to remove. Skip-everywhere rule skips (null path) are forgotten diff --git a/tests/Reporting/UnusedSkipResolverTest.php b/tests/Reporting/UnusedSkipResolverTest.php index f6e9baaffef..eb926349e66 100644 --- a/tests/Reporting/UnusedSkipResolverTest.php +++ b/tests/Reporting/UnusedSkipResolverTest.php @@ -46,6 +46,7 @@ protected function tearDown(): void SimpleParameterProvider::setParameter(Option::SKIP, []); SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false); SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, false); + SimpleParameterProvider::setParameter(Option::IS_CACHED_RUN, false); } public function testResolvesUnusedSkipsAsRuleAndPath(): void @@ -113,6 +114,18 @@ public function testResolvesNothingWhenRunIsNarrowed(): void $this->assertSame([], $this->unusedSkipResolver->resolve($processResult)); } + public function testResolvesNothingWhenRunIsCached(): void + { + SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true); + SimpleParameterProvider::setParameter(Option::IS_CACHED_RUN, true); + + // a cached run only re-processes changed files, so skips on cached files never match and + // are not false positives + $processResult = new ProcessResult([], [], 0, []); + + $this->assertSame([], $this->unusedSkipResolver->resolve($processResult)); + } + public function testResolvesNothingWhenDisabled(): void { SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false);