From 88acc0a10cd1a2b162cd7ab1fe35862af567656f Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Jun 2026 00:32:05 +0200 Subject: [PATCH 1/3] group paths together for skip --- src/Reporting/UnusedSkipResolver.php | 47 ++++++++++++++++------ tests/Reporting/UnusedSkipResolverTest.php | 19 +++++++++ 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/Reporting/UnusedSkipResolver.php b/src/Reporting/UnusedSkipResolver.php index 6c66f9b5519..f0dbf982f28 100644 --- a/src/Reporting/UnusedSkipResolver.php +++ b/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,11 +37,11 @@ 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 = []; + // 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; @@ -49,25 +49,48 @@ public function resolve(ProcessResult $processResult): array // 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; } } diff --git a/tests/Reporting/UnusedSkipResolverTest.php b/tests/Reporting/UnusedSkipResolverTest.php index bb3e14ed3b8..5985d7a0ac3 100644 --- a/tests/Reporting/UnusedSkipResolverTest.php +++ b/tests/Reporting/UnusedSkipResolverTest.php @@ -78,6 +78,25 @@ 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 testResolvesNothingWhenDisabled(): void { SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false); From 07d8a3502af424b906b70fbb04dec1eb5a51ad18 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 21 Jun 2026 00:44:57 +0200 Subject: [PATCH 2/3] skip unused-skip reporting on narrowed runs, group paths by rule --- src/Configuration/ConfigurationFactory.php | 8 ++++++++ src/Configuration/Option.php | 10 ++++++++++ src/Reporting/UnusedSkipResolver.php | 6 ++++++ tests/Reporting/UnusedSkipResolverTest.php | 13 +++++++++++++ 4 files changed, 37 insertions(+) diff --git a/src/Configuration/ConfigurationFactory.php b/src/Configuration/ConfigurationFactory.php index 2128b23f11d..788370cf407 100644 --- a/src/Configuration/ConfigurationFactory.php +++ b/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; } diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index f5a756a78c1..ac41dff41c9 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -109,6 +109,16 @@ 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 + * @var string + */ + public const string IS_RUN_NARROWED = 'is_run_narrowed'; + /** * @internal Use RectorConfig::fileExtensions() instead */ diff --git a/src/Reporting/UnusedSkipResolver.php b/src/Reporting/UnusedSkipResolver.php index f0dbf982f28..841e482c43b 100644 --- a/src/Reporting/UnusedSkipResolver.php +++ b/src/Reporting/UnusedSkipResolver.php @@ -37,6 +37,12 @@ public function resolve(ProcessResult $processResult): array return []; } + // 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 diff --git a/tests/Reporting/UnusedSkipResolverTest.php b/tests/Reporting/UnusedSkipResolverTest.php index 5985d7a0ac3..027b26ef7b7 100644 --- a/tests/Reporting/UnusedSkipResolverTest.php +++ b/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 @@ -97,6 +98,18 @@ public function testGroupsMultipleUnusedPathsUnderRule(): void ); } + 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); From 20fe3d2604f874911e29aa2e200d1dbfda524dca Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sat, 20 Jun 2026 22:46:36 +0000 Subject: [PATCH 3/3] [ci-review] Rector Rectify --- src/Configuration/Option.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index ac41dff41c9..947e6bb1937 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -115,7 +115,6 @@ final class Option * look falsely unused and would produce many false positives. * * @internal - * @var string */ public const string IS_RUN_NARROWED = 'is_run_narrowed';