[reporting] Add reportUnusedSkips() to report skips that never matched

.github/workflows/e2e.yaml

@@ -40,6 +40,7 @@ jobs:
                     - 'e2e/only-option-quote-single-bsdouble'
                     - 'e2e/only-option-quote-single-equalnone'
                     - 'e2e/parallel-custom-config'
+                    - 'e2e/parallel-unused-skips'
                     - 'e2e/parallel-reflection-resolver'
                     - 'e2e/parallel with space'
                     - 'e2e/print-new-node'
@@ -72,6 +73,11 @@ jobs:
                 working-directory: ${{ matrix.directory }}
                 if: ${{ matrix.directory == 'e2e/parallel-custom-config' }}
 
+            # tests parallel aggregation of used skips for ->reportUnusedSkips()
+            -   run: php ../e2eTestRunner.php --config custom/config/rector.php
+                working-directory: ${{ matrix.directory }}
+                if: ${{ matrix.directory == 'e2e/parallel-unused-skips' }}
+
             -   run: php ../e2eTestRunner.php
                 working-directory: ${{ matrix.directory }}
-                if: ${{ matrix.directory != 'e2e/parallel-custom-config' }}
+                if: ${{ matrix.directory != 'e2e/parallel-custom-config' && matrix.directory != 'e2e/parallel-unused-skips' }}

e2e/parallel-unused-skips/.gitignore

@@ -0,0 +1 @@
+/vendor

e2e/parallel-unused-skips/composer.json

@@ -0,0 +1,5 @@
+{
+    "require": {
+        "php": "^8.1"
+    }
+}

e2e/parallel-unused-skips/custom/config/rector.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\CodeQuality\Rector\FunctionLike\SimplifyUselessVariableRector;
+use Rector\Config\RectorConfig;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->parallel();
+
+    $rectorConfig->paths([__DIR__ . '/../../src/']);
+
+    $rectorConfig->rule(SimplifyUselessVariableRector::class);
+
+    $rectorConfig->reportUnusedSkips();
+
+    // matched in the worker - must be excluded from the unused report (proves parallel aggregation)
+    // plus a glob that never matches - must be reported as unused
+    $rectorConfig->skip([
+        SimplifyUselessVariableRector::class => ['*/src/*'],
+        '*/NonexistentUnused/*',
+    ]);
+};

e2e/parallel-unused-skips/expected-output.diff

@@ -0,0 +1,6 @@
+[OK] Rector is done!
+
+ [WARNING] This skip is unused, it never matched any element. You can remove it
+           from "->withSkip()"
+
+ * */NonexistentUnused/*

e2e/parallel-unused-skips/src/SomeClass.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace E2e\Parallel\Unused\Skips;
+
+final class SomeClass
+{
+    public function run()
+    {
+        $value = 1000;
+        return $value;
+    }
+}

src/Application/ApplicationFileProcessor.php

@@ -14,6 +14,7 @@
 use Rector\Parallel\Application\ParallelFileProcessor;
 use Rector\PhpParser\Parser\ParserErrors;
 use Rector\Reporting\MissConfigurationReporter;
+use Rector\Skipper\Skipper\UsedSkipCollector;
 use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment;
 use Rector\Util\ArrayParametersMerger;
 use Rector\ValueObject\Application\File;
@@ -49,6 +50,7 @@ public function __construct(
         private readonly FileProcessor $fileProcessor,
         private readonly ArrayParametersMerger $arrayParametersMerger,
         private readonly MissConfigurationReporter $missConfigurationReporter,
+        private readonly UsedSkipCollector $usedSkipCollector,
     ) {
     }
 
@@ -161,7 +163,7 @@ public function processFiles(
             }
         }
 
-        return new ProcessResult($systemErrors, $fileDiffs, $totalChanged);
+        return new ProcessResult($systemErrors, $fileDiffs, $totalChanged, $this->usedSkipCollector->provide());
     }
 
     private function processFile(File $file, Configuration $configuration): FileProcessResult

src/Config/RectorConfig.php

@@ -438,6 +438,11 @@ public function reportingRealPath(bool $absolute = true): void
         SimpleParameterProvider::setParameter(Option::ABSOLUTE_FILE_PATH, $absolute);
     }
 
+    public function reportUnusedSkips(bool $report = true): void
+    {
+        SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, $report);
+    }
+
     public function editorUrl(string $editorUrl): void
     {
         SimpleParameterProvider::setParameter(Option::EDITOR_URL, $editorUrl);

src/Configuration/Option.php

@@ -103,6 +103,12 @@ final class Option
      */
     public const string SKIP = 'skip';
 
+    /**
+     * @internal Use @see \Rector\Config\RectorConfig::reportUnusedSkips() instead
+     * @var string
+     */
+    public const string REPORT_UNUSED_SKIPS = 'report_unused_skips';
+
     /**
      * @internal Use RectorConfig::fileExtensions() instead
      */

src/Configuration/RectorConfigBuilder.php

@@ -174,6 +174,8 @@ final class RectorConfigBuilder
 
     private ?bool $reportingRealPath = null;
 
+    private ?bool $reportUnusedSkips = null;
+
     /**
      * @var string[]
      */
@@ -394,6 +396,10 @@ public function __invoke(RectorConfig $rectorConfig): void
             $rectorConfig->reportingRealPath($this->reportingRealPath);
         }
 
+        if ($this->reportUnusedSkips !== null) {
+            $rectorConfig->reportUnusedSkips($this->reportUnusedSkips);
+        }
+
         if ($this->editorUrl !== null) {
             $rectorConfig->editorUrl($this->editorUrl);
         }
@@ -1314,6 +1320,17 @@ public function withRealPathReporting(bool $absolutePath = true): self
         return $this;
     }
 
+    /**
+     * Report skips configured via withSkip() that never matched anything during the run,
+     * so they can be safely removed.
+     */
+    public function reportUnusedSkips(bool $report = true): self
+    {
+        $this->reportUnusedSkips = $report;
+
+        return $this;
+    }
+
     public function withEditorUrl(string $editorUrl): self
     {
         $this->editorUrl = $editorUrl;

src/Console/Command/ProcessCommand.php

@@ -186,6 +186,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $this->deprecatedRulesReporter->reportDeprecatedRectorUnsupportedMethods();
 
         $this->missConfigurationReporter->reportSkippedNeverRegisteredRules();
+        $this->missConfigurationReporter->reportUnusedSkips($processResult);
 
         return $this->resolveReturnCode($processResult, $configuration);
     }

src/Console/Command/WorkerCommand.php

@@ -166,6 +166,7 @@ private function runWorker(
                     Bridge::SYSTEM_ERRORS => $processResult->getSystemErrors(),
                     Bridge::SYSTEM_ERRORS_COUNT => count($processResult->getSystemErrors()),
                     Bridge::TOTAL_CHANGED => $processResult->getTotalChanged(),
+                    Bridge::USED_SKIPS => $processResult->getUsedSkips(),
                 ],
             ]);
         });

src/DependencyInjection/LazyContainerFactory.php

@@ -177,6 +177,7 @@
 use Rector\Rector\AbstractRector;
 use Rector\Reporting\DeprecatedRulesReporter;
 use Rector\Skipper\Skipper\Skipper;
+use Rector\Skipper\Skipper\UsedSkipCollector;
 use Rector\StaticTypeMapper\Contract\PhpDocParser\PhpDocTypeMapperInterface;
 use Rector\StaticTypeMapper\Contract\PhpParser\PhpParserNodeMapperInterface;
 use Rector\StaticTypeMapper\Mapper\PhpParserNodeMapper;
@@ -453,6 +454,9 @@ public function create(): RectorConfig
         $rectorConfig->singleton(FileProcessor::class);
         $rectorConfig->singleton(PostFileProcessor::class);
 
+        // shared state: collects used skips across skipper voters and the file processor
+        $rectorConfig->singleton(UsedSkipCollector::class);
+
         $rectorConfig->when(RectorNodeTraverser::class)
             ->needs('$rectors')
             ->giveTagged(RectorInterface::class);

src/Parallel/Application/ParallelFileProcessor.php

@@ -75,6 +75,9 @@ public function process(
         /** @var SystemError[] $systemErrors */
         $systemErrors = [];
 
+        /** @var array<string, true> $usedSkips */
+        $usedSkips = [];
+
         $tcpServer = new TcpServer('127.0.0.1:0', $streamSelectLoop);
         $this->processPool = new ProcessPool($tcpServer);
 
@@ -143,6 +146,7 @@ public function process(
         $processSpawner = function () use (
             &$systemErrors,
             &$fileDiffs,
+            &$usedSkips,
             &$jobs,
             $postFileCallback,
             &$systemErrorsCount,
@@ -176,6 +180,7 @@ function (array $json) use (
                     $parallelProcess,
                     &$systemErrors,
                     &$fileDiffs,
+                    &$usedSkips,
                     &$jobs,
                     $postFileCallback,
                     &$systemErrorsCount,
@@ -190,10 +195,15 @@ function (array $json) use (
                      *      system_errors: mixed[],
                      *      file_diffs: array<string, mixed>,
                      *      files_count: int,
-                     *      system_errors_count: int
+                     *      system_errors_count: int,
+                     *      used_skips: string[]
                      * } $json */
                     $totalChanged += $json[Bridge::TOTAL_CHANGED];
 
+                    foreach ($json[Bridge::USED_SKIPS] as $usedSkip) {
+                        $usedSkips[$usedSkip] = true;
+                    }
+
                     // decode arrays to objects
                     foreach ($json[Bridge::SYSTEM_ERRORS] as $jsonError) {
                         if (is_string($jsonError)) {
@@ -276,6 +286,6 @@ function ($exitCode, string $stdErr) use (&$systemErrors, $processIdentifier): v
             ));
         }
 
-        return new ProcessResult($systemErrors, $fileDiffs, $totalChanged);
+        return new ProcessResult($systemErrors, $fileDiffs, $totalChanged, array_keys($usedSkips));
     }
 }

src/Parallel/ValueObject/Bridge.php

@@ -20,4 +20,6 @@ final class Bridge
     public const string FILES_COUNT = 'files_count';
 
     public const string TOTAL_CHANGED = 'total_changed';
+
+    public const string USED_SKIPS = 'used_skips';
 }

src/Reporting/MissConfigurationReporter.php

@@ -8,16 +8,57 @@
 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;
 
+/**
+ * @see \Rector\Tests\Reporting\MissConfigurationReporterTest
+ */
 final readonly class MissConfigurationReporter
 {
     public function __construct(
         private SymfonyStyle $symfonyStyle,
         private VendorMissAnalyseGuard $vendorMissAnalyseGuard,
+        private SkippedClassResolver $skippedClassResolver,
+        private SkippedPathsResolver $skippedPathsResolver,
     ) {
     }
 
+    /**
+     * Reports skips configured via "->withSkip()" that never matched any element during the run.
+     * Enabled with "->reportUnusedSkips()".
+     */
+    public function reportUnusedSkips(ProcessResult $processResult): void
+    {
+        if (! SimpleParameterProvider::provideBoolParameter(Option::REPORT_UNUSED_SKIPS, false)) {
+            return;
+        }
+
+        // only path-scoped class skips are trackable at runtime; skip-everywhere rule skips
+        // (null path) are forgotten from the container at boot, so they never reach the skipper
+        $pathScopedClassSkips = array_keys(array_filter(
+            $this->skippedClassResolver->resolve(),
+            static fn (?array $paths): bool => $paths !== null
+        ));
+
+        $configuredSkips = [...$pathScopedClassSkips, ...$this->skippedPathsResolver->resolve()];
+
+        $unusedSkips = array_values(array_diff($configuredSkips, $processResult->getUsedSkips()));
+        if ($unusedSkips === []) {
+            return;
+        }
+
+        $this->symfonyStyle->warning(sprintf(
+            '%s never matched any element. You can remove %s from "->withSkip()"',
+            count($unusedSkips) > 1 ? 'These skips are unused, they' : 'This skip is unused, it',
+            count($unusedSkips) > 1 ? 'them' : 'it'
+        ));
+
+        $this->symfonyStyle->listing($unusedSkips);
+    }
+
     public function reportSkippedNeverRegisteredRules(): void
     {
         $registeredRules = SimpleParameterProvider::provideArrayParameter(Option::REGISTERED_RECTOR_RULES);

src/Skipper/Skipper/PathSkipper.php

@@ -11,13 +11,20 @@
 {
     public function __construct(
         private FileInfoMatcher $fileInfoMatcher,
-        private SkippedPathsResolver $skippedPathsResolver
+        private SkippedPathsResolver $skippedPathsResolver,
+        private UsedSkipCollector $usedSkipCollector
     ) {
     }
 
     public function shouldSkip(string $filePath): bool
     {
-        $skippedPaths = $this->skippedPathsResolver->resolve();
-        return $this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, $skippedPaths);
+        foreach ($this->skippedPathsResolver->resolve() as $skippedPath) {
+            if ($this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, [$skippedPath])) {
+                $this->usedSkipCollector->markUsed($skippedPath);
+                return true;
+            }
+        }
+
+        return false;
     }
 }

src/Skipper/Skipper/SkipSkipper.php

@@ -9,7 +9,8 @@
 final readonly class SkipSkipper
 {
     public function __construct(
-        private FileInfoMatcher $fileInfoMatcher
+        private FileInfoMatcher $fileInfoMatcher,
+        private UsedSkipCollector $usedSkipCollector
     ) {
     }
 
@@ -25,10 +26,12 @@ public function doesMatchSkip(object | string $checker, string $filePath, array
 
             // skip everywhere
             if (! is_array($skippedFiles)) {
+                $this->usedSkipCollector->markUsed($skippedClass);
                 return true;
             }
 
             if ($this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, $skippedFiles)) {
+                $this->usedSkipCollector->markUsed($skippedClass);
                 return true;
             }
         }

src/Skipper/Skipper/UsedSkipCollector.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Skipper\Skipper;
+
+/**
+ * Collects skip elements (rule classes and paths) that actually matched during the run,
+ * so unused skips can be reported and removed.
+ *
+ * @see \Rector\Tests\Skipper\Skipper\UsedSkipCollectorTest
+ */
+final class UsedSkipCollector
+{
+    /**
+     * @var array<string, true>
+     */
+    private array $usedSkips = [];
+
+    public function markUsed(string $skip): void
+    {
+        $this->usedSkips[$skip] = true;
+    }
+
+    /**
+     * @return string[]
+     */
+    public function provide(): array
+    {
+        return array_keys($this->usedSkips);
+    }
+}