[skip] display skips only on uncached run (#8071)

* [skip] display skips only on uncached run

* newline

Author: TomasVotruba

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;
     }
 }

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
      */

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

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

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);