[reporting] Group unused skips by rule, skip reporting on narrowed runs

src/Configuration/ConfigurationFactory.php

@@ -71,6 +71,12 @@ public function createFromInput(InputInterface $input): Configuration
 
         $onlySuffix = $input->getOption(Option::ONLY_SUFFIX);
 
+        // "--only"/"--only-suffix" narrow the run, so skips outside the scope look falsely unused;
+        // mark the run as narrowed to disable unused skip reporting and avoid false positives
+        if ($onlyRule !== null || $onlySuffix !== null) {
+            SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, true);
+        }
+
         $isParallel = SimpleParameterProvider::provideBoolParameter(Option::PARALLEL);
         $parallelPort = (string) $input->getOption(Option::PARALLEL_PORT);
         $parallelIdentifier = (string) $input->getOption(Option::PARALLEL_IDENTIFIER);
@@ -144,6 +150,8 @@ private function resolvePaths(InputInterface $input): array
 
         // give priority to command line
         if ($commandLinePaths !== []) {
+            // mark the run as narrowed, so unused skip reporting can be disabled to avoid false positives
+            SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, true);
             $this->setFilesWithoutExtensionParameter($commandLinePaths);
             return $commandLinePaths;
         }

src/Configuration/Option.php

@@ -109,6 +109,15 @@ final class Option
      */
     public const string REPORT_UNUSED_SKIPS = 'report_unused_skips';
 
+    /**
+     * True when the run is narrowed on the command line - via paths argument, "--only" or
+     * "--only-suffix". Unused skip reporting is then disabled, as skips outside the narrowed scope
+     * look falsely unused and would produce many false positives.
+     *
+     * @internal
+     */
+    public const string IS_RUN_NARROWED = 'is_run_narrowed';
+
     /**
      * @internal Use RectorConfig::fileExtensions() instead
      */

src/Reporting/UnusedSkipResolver.php

@@ -25,9 +25,9 @@ public function __construct(
 
     /**
      * Resolves skips configured via "->withSkip()" that never matched any element during the run.
-     * Rule-scoped skips are returned as "rule => path" so the user knows exactly what to remove;
-     * global skips are returned as a plain path. Returns an empty array unless
-     * "->reportUnusedSkips()" is enabled.
+     * Rule-scoped skips are grouped under their rule ("rule => path", or "rule => [ path\n path ]"
+     * for multiple paths) so the user knows exactly what to remove; global skips are returned as a
+     * plain path. Returns an empty array unless "->reportUnusedSkips()" is enabled.
      *
      * @return string[]
      */
@@ -37,37 +37,66 @@ public function resolve(ProcessResult $processResult): array
             return [];
         }
 
-        // map of trackable skip path => human-readable display; skips are tracked at runtime by
-        // their path, but rule-scoped ones are printed as "rule => path" so the user knows exactly
-        // what to remove. Skip-everywhere rule skips (null path) are forgotten from the container
-        // at boot, so they never reach the skipper and cannot be tracked.
-        $skipDisplaysByPath = [];
+        // a narrowed run (cli paths, "--only" or "--only-suffix") only touches part of the codebase,
+        // so skips outside that scope look falsely unused - reporting them would be noise
+        if (SimpleParameterProvider::provideBoolParameter(Option::IS_RUN_NARROWED, 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
+        // from the container at boot, so they never reach the skipper and cannot be tracked.
+        $relativePathsByClass = [];
         foreach ($this->skippedClassResolver->resolve() as $rectorClass => $paths) {
             if ($paths === null) {
                 continue;
             }
 
             // rule-scoped paths are intentional, so they are reported even as mask paths
             foreach ($paths as $path) {
-                $skipDisplaysByPath[$path] = $rectorClass . ' => ' . $this->filePathHelper->relativePath($path);
+                $relativePathsByClass[$rectorClass][$path] = $this->filePathHelper->relativePath($path);
             }
         }
 
         // global mask paths like "*/some/*" are hard to spot and report false positives, skip them
+        $globalRelativePaths = [];
         foreach ($this->skippedPathsResolver->resolve() as $globalPath) {
             if (str_contains($globalPath, '*')) {
                 continue;
             }
 
-            $skipDisplaysByPath[$globalPath] = $this->filePathHelper->relativePath($globalPath);
+            $globalRelativePaths[$globalPath] = $this->filePathHelper->relativePath($globalPath);
         }
 
         $usedSkips = $processResult->getUsedSkips();
 
         $unusedSkips = [];
-        foreach ($skipDisplaysByPath as $path => $skipDisplay) {
+
+        // group unused rule-scoped paths under their rule, matching the "->withSkip()" config shape
+        foreach ($relativePathsByClass as $rectorClass => $relativePaths) {
+            $unusedRelativePaths = [];
+            foreach ($relativePaths as $path => $relativePath) {
+                if (! in_array($path, $usedSkips, true)) {
+                    $unusedRelativePaths[] = $relativePath;
+                }
+            }
+
+            if ($unusedRelativePaths === []) {
+                continue;
+            }
+
+            if (count($unusedRelativePaths) === 1) {
+                $unusedSkips[] = $rectorClass . ' => ' . $unusedRelativePaths[0];
+                continue;
+            }
+
+            $unusedSkips[] = $rectorClass . ' => [ ' . implode("\n    ", $unusedRelativePaths) . ' ]';
+        }
+
+        foreach ($globalRelativePaths as $path => $relativePath) {
             if (! in_array($path, $usedSkips, true)) {
-                $unusedSkips[] = $skipDisplay;
+                $unusedSkips[] = $relativePath;
             }
         }
 

tests/Reporting/UnusedSkipResolverTest.php

@@ -45,6 +45,7 @@ protected function tearDown(): void
     {
         SimpleParameterProvider::setParameter(Option::SKIP, []);
         SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false);
+        SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, false);
     }
 
     public function testResolvesUnusedSkipsAsRuleAndPath(): void
@@ -78,6 +79,37 @@ public function testReportsUnusedSkipAsRelativePath(): void
         $this->assertContains(FifthElement::class . ' => src/Reporting/UnusedSkipResolver.php', $unusedSkips);
     }
 
+    public function testGroupsMultipleUnusedPathsUnderRule(): void
+    {
+        SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);
+
+        $firstPath = getcwd() . '/src/Reporting/UnusedSkipResolver.php';
+        $secondPath = getcwd() . '/src/Reporting/MissConfigurationReporter.php';
+        SimpleParameterProvider::setParameter(Option::SKIP, [
+            FifthElement::class => [$firstPath, $secondPath],
+        ]);
+
+        $unusedSkips = $this->unusedSkipResolver->resolve(new ProcessResult([], [], 0, []));
+
+        // multiple unused paths are grouped under their rule, not repeated per line
+        $this->assertContains(
+            FifthElement::class . ' => [ src/Reporting/UnusedSkipResolver.php' . "\n    " . 'src/Reporting/MissConfigurationReporter.php ]',
+            $unusedSkips
+        );
+    }
+
+    public function testResolvesNothingWhenRunIsNarrowed(): void
+    {
+        SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);
+        SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, true);
+
+        // narrowed run (cli paths, "--only", "--only-suffix") only touches part of the codebase,
+        // so skips outside that scope 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);