[reporting] Report unused skips in JSON output too

src/ChangesReporting/Output/Factory/JsonOutputFactory.php

@@ -15,8 +15,14 @@
  */
 final class JsonOutputFactory
 {
-    public static function create(ProcessResult $processResult, Configuration $configuration): string
-    {
+    /**
+     * @param string[] $unusedSkips
+     */
+    public static function create(
+        ProcessResult $processResult,
+        Configuration $configuration,
+        array $unusedSkips = []
+    ): string {
         $errorsJson = [
             'totals' => [
                 'changed_files' => $processResult->getTotalChanged(),
@@ -52,6 +58,10 @@ public static function create(ProcessResult $processResult, Configuration $confi
             $errorsJson['errors'] = $errorsData;
         }
 
+        if ($unusedSkips !== []) {
+            $errorsJson['unused_skips'] = $unusedSkips;
+        }
+
         return Json::encode($errorsJson, pretty: true);
     }
 

src/ChangesReporting/Output/JsonOutputFormatter.php

@@ -6,20 +6,29 @@
 
 use Rector\ChangesReporting\Contract\Output\OutputFormatterInterface;
 use Rector\ChangesReporting\Output\Factory\JsonOutputFactory;
+use Rector\Reporting\UnusedSkipResolver;
 use Rector\ValueObject\Configuration;
 use Rector\ValueObject\ProcessResult;
 
 final readonly class JsonOutputFormatter implements OutputFormatterInterface
 {
     public const string NAME = 'json';
 
+    public function __construct(
+        private UnusedSkipResolver $unusedSkipResolver
+    ) {
+    }
+
     public function getName(): string
     {
         return self::NAME;
     }
 
     public function report(ProcessResult $processResult, Configuration $configuration): void
     {
-        echo JsonOutputFactory::create($processResult, $configuration) . PHP_EOL;
+        // console output is silenced in json mode, so unused skips are surfaced in the payload
+        $unusedSkips = $this->unusedSkipResolver->resolve($processResult);
+
+        echo JsonOutputFactory::create($processResult, $configuration, $unusedSkips) . PHP_EOL;
     }
 }

src/Reporting/MissConfigurationReporter.php

@@ -8,8 +8,6 @@
 use Rector\Configuration\Parameter\SimpleParameterProvider;
 use Rector\Configuration\VendorMissAnalyseGuard;
 use Rector\PostRector\Contract\Rector\PostRectorInterface;
-use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver;
-use Rector\Skipper\SkipCriteriaResolver\SkippedPathsResolver;
 use Rector\ValueObject\ProcessResult;
 use Symfony\Component\Console\Style\SymfonyStyle;
 
@@ -21,8 +19,7 @@
     public function __construct(
         private SymfonyStyle $symfonyStyle,
         private VendorMissAnalyseGuard $vendorMissAnalyseGuard,
-        private SkippedClassResolver $skippedClassResolver,
-        private SkippedPathsResolver $skippedPathsResolver,
+        private UnusedSkipResolver $unusedSkipResolver,
     ) {
     }
 
@@ -32,44 +29,7 @@ public function __construct(
      */
     public function reportUnusedSkips(ProcessResult $processResult): void
     {
-        if (! SimpleParameterProvider::provideBoolParameter(Option::REPORT_UNUSED_SKIPS, false)) {
-            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 = [];
-        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 . ' => ' . $path;
-            }
-        }
-
-        // global mask paths like "*/some/*" are hard to spot and report false positives, skip them
-        foreach ($this->skippedPathsResolver->resolve() as $globalPath) {
-            if (str_contains($globalPath, '*')) {
-                continue;
-            }
-
-            $skipDisplaysByPath[$globalPath] = $globalPath;
-        }
-
-        $usedSkips = $processResult->getUsedSkips();
-
-        $unusedSkips = [];
-        foreach ($skipDisplaysByPath as $path => $skipDisplay) {
-            if (! in_array($path, $usedSkips, true)) {
-                $unusedSkips[] = $skipDisplay;
-            }
-        }
-
+        $unusedSkips = $this->unusedSkipResolver->resolve($processResult);
         if ($unusedSkips === []) {
             return;
         }

src/Reporting/UnusedSkipResolver.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Reporting;
+
+use Rector\Configuration\Option;
+use Rector\Configuration\Parameter\SimpleParameterProvider;
+use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver;
+use Rector\Skipper\SkipCriteriaResolver\SkippedPathsResolver;
+use Rector\ValueObject\ProcessResult;
+
+/**
+ * @see \Rector\Tests\Reporting\UnusedSkipResolverTest
+ */
+final readonly class UnusedSkipResolver
+{
+    public function __construct(
+        private SkippedClassResolver $skippedClassResolver,
+        private SkippedPathsResolver $skippedPathsResolver,
+    ) {
+    }
+
+    /**
+     * 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.
+     *
+     * @return string[]
+     */
+    public function resolve(ProcessResult $processResult): array
+    {
+        if (! SimpleParameterProvider::provideBoolParameter(Option::REPORT_UNUSED_SKIPS, false)) {
+            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 = [];
+        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 . ' => ' . $path;
+            }
+        }
+
+        // global mask paths like "*/some/*" are hard to spot and report false positives, skip them
+        foreach ($this->skippedPathsResolver->resolve() as $globalPath) {
+            if (str_contains($globalPath, '*')) {
+                continue;
+            }
+
+            $skipDisplaysByPath[$globalPath] = $globalPath;
+        }
+
+        $usedSkips = $processResult->getUsedSkips();
+
+        $unusedSkips = [];
+        foreach ($skipDisplaysByPath as $path => $skipDisplay) {
+            if (! in_array($path, $usedSkips, true)) {
+                $unusedSkips[] = $skipDisplay;
+            }
+        }
+
+        return $unusedSkips;
+    }
+}

tests/Reporting/MissConfigurationReporterTest.php

@@ -8,8 +8,7 @@
 use Rector\Configuration\Parameter\SimpleParameterProvider;
 use Rector\Configuration\VendorMissAnalyseGuard;
 use Rector\Reporting\MissConfigurationReporter;
-use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver;
-use Rector\Skipper\SkipCriteriaResolver\SkippedPathsResolver;
+use Rector\Reporting\UnusedSkipResolver;
 use Rector\Testing\PHPUnit\AbstractLazyTestCase;
 use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement;
 use Rector\Tests\Skipper\Skipper\Fixture\Element\ThreeMan;
@@ -41,8 +40,7 @@ protected function setUp(): void
         $this->missConfigurationReporter = new MissConfigurationReporter(
             $symfonyStyle,
             new VendorMissAnalyseGuard(),
-            $this->make(SkippedClassResolver::class),
-            $this->make(SkippedPathsResolver::class),
+            $this->make(UnusedSkipResolver::class),
         );
 
         SimpleParameterProvider::setParameter(Option::SKIP, [

tests/Reporting/UnusedSkipResolverTest.php

@@ -0,0 +1,74 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Reporting;
+
+use Rector\Configuration\Option;
+use Rector\Configuration\Parameter\SimpleParameterProvider;
+use Rector\Reporting\UnusedSkipResolver;
+use Rector\Testing\PHPUnit\AbstractLazyTestCase;
+use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement;
+use Rector\Tests\Skipper\Skipper\Fixture\Element\ThreeMan;
+use Rector\Tests\Skipper\Skipper\Source\AnotherClassToSkip;
+use Rector\ValueObject\ProcessResult;
+
+final class UnusedSkipResolverTest extends AbstractLazyTestCase
+{
+    private const string UNUSED_RULE_MASK = '*/dead-rule/*';
+
+    private const string USED_RULE_MASK = '*/matched-rule/*';
+
+    private const string GLOBAL_MASK = '*/global-mask/*';
+
+    private UnusedSkipResolver $unusedSkipResolver;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->unusedSkipResolver = $this->make(UnusedSkipResolver::class);
+
+        SimpleParameterProvider::setParameter(Option::SKIP, [
+            // skip-everywhere rule skip - forgotten at boot, not trackable, excluded
+            ThreeMan::class,
+            // rule-scoped path skip, never matched - intentional, reported even as mask
+            FifthElement::class => [self::UNUSED_RULE_MASK],
+            // rule-scoped path skip, matched - excluded
+            AnotherClassToSkip::class => [self::USED_RULE_MASK],
+            // global mask path skip - hard to spot, false-positive prone, excluded
+            self::GLOBAL_MASK,
+        ]);
+    }
+
+    protected function tearDown(): void
+    {
+        SimpleParameterProvider::setParameter(Option::SKIP, []);
+        SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false);
+    }
+
+    public function testResolvesUnusedSkipsAsRuleAndPath(): void
+    {
+        SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);
+
+        $processResult = new ProcessResult([], [], 0, [self::USED_RULE_MASK]);
+        $unusedSkips = $this->unusedSkipResolver->resolve($processResult);
+
+        // rule-scoped unused skip is reported as "rule => path"
+        $this->assertContains(FifthElement::class . ' => ' . self::UNUSED_RULE_MASK, $unusedSkips);
+
+        // matched rule-scoped skip, global mask and skip-everywhere rule are excluded
+        $this->assertNotContains(AnotherClassToSkip::class . ' => ' . self::USED_RULE_MASK, $unusedSkips);
+        $this->assertNotContains(self::GLOBAL_MASK, $unusedSkips);
+        $this->assertNotContains(ThreeMan::class, $unusedSkips);
+    }
+
+    public function testResolvesNothingWhenDisabled(): void
+    {
+        SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false);
+
+        $processResult = new ProcessResult([], [], 0, []);
+
+        $this->assertSame([], $this->unusedSkipResolver->resolve($processResult));
+    }
+}