Skip to content

Commit 3893ea4

Browse files
TomasVotrubaactions-user
TomasVotruba
and
actions-user
authored
[reporting] Group unused skips by rule, skip reporting on narrowed runs (#8069)
* group paths together for skip * skip unused-skip reporting on narrowed runs, group paths by rule * [ci-review] Rector Rectify --------- Co-authored-by: GitHub Action <actions@github.com>
1 parent 580b374 commit 3893ea4

4 files changed

Lines changed: 90 additions & 12 deletions

File tree

‎src/Configuration/ConfigurationFactory.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ public function createFromInput(InputInterface $input): Configuration
7171

7272
$onlySuffix = $input->getOption(Option::ONLY_SUFFIX);
7373

74+
// "--only"/"--only-suffix" narrow the run, so skips outside the scope look falsely unused;
75+
// mark the run as narrowed to disable unused skip reporting and avoid false positives
76+
if ($onlyRule !== null || $onlySuffix !== null) {
77+
SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, true);
78+
}
79+
7480
$isParallel = SimpleParameterProvider::provideBoolParameter(Option::PARALLEL);
7581
$parallelPort = (string) $input->getOption(Option::PARALLEL_PORT);
7682
$parallelIdentifier = (string) $input->getOption(Option::PARALLEL_IDENTIFIER);
@@ -144,6 +150,8 @@ private function resolvePaths(InputInterface $input): array
144150

145151
// give priority to command line
146152
if ($commandLinePaths !== []) {
153+
// mark the run as narrowed, so unused skip reporting can be disabled to avoid false positives
154+
SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, true);
147155
$this->setFilesWithoutExtensionParameter($commandLinePaths);
148156
return $commandLinePaths;
149157
}

‎src/Configuration/Option.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,15 @@ final class Option
109109
*/
110110
public const string REPORT_UNUSED_SKIPS = 'report_unused_skips';
111111

112+
/**
113+
* True when the run is narrowed on the command line - via paths argument, "--only" or
114+
* "--only-suffix". Unused skip reporting is then disabled, as skips outside the narrowed scope
115+
* look falsely unused and would produce many false positives.
116+
*
117+
* @internal
118+
*/
119+
public const string IS_RUN_NARROWED = 'is_run_narrowed';
120+
112121
/**
113122
* @internal Use RectorConfig::fileExtensions() instead
114123
*/

‎src/Reporting/UnusedSkipResolver.php‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ public function __construct(
2525

2626
/**
2727
* Resolves skips configured via "->withSkip()" that never matched any element during the run.
28-
* Rule-scoped skips are returned as "rule => path" so the user knows exactly what to remove;
29-
* global skips are returned as a plain path. Returns an empty array unless
30-
* "->reportUnusedSkips()" is enabled.
28+
* Rule-scoped skips are grouped under their rule ("rule => path", or "rule => [ path\n path ]"
29+
* for multiple paths) so the user knows exactly what to remove; global skips are returned as a
30+
* plain path. Returns an empty array unless "->reportUnusedSkips()" is enabled.
3131
*
3232
* @return string[]
3333
*/
@@ -37,37 +37,66 @@ public function resolve(ProcessResult $processResult): array
3737
return [];
3838
}
3939

40-
// map of trackable skip path => human-readable display; skips are tracked at runtime by
41-
// their path, but rule-scoped ones are printed as "rule => path" so the user knows exactly
42-
// what to remove. Skip-everywhere rule skips (null path) are forgotten from the container
43-
// at boot, so they never reach the skipper and cannot be tracked.
44-
$skipDisplaysByPath = [];
40+
// a narrowed run (cli paths, "--only" or "--only-suffix") only touches part of the codebase,
41+
// so skips outside that scope look falsely unused - reporting them would be noise
42+
if (SimpleParameterProvider::provideBoolParameter(Option::IS_RUN_NARROWED, false)) {
43+
return [];
44+
}
45+
46+
// map of rule => (trackable skip path => relative display path); skips are tracked at
47+
// runtime by their path, but rule-scoped ones are printed grouped under their rule so the
48+
// user knows exactly what to remove. Skip-everywhere rule skips (null path) are forgotten
49+
// from the container at boot, so they never reach the skipper and cannot be tracked.
50+
$relativePathsByClass = [];
4551
foreach ($this->skippedClassResolver->resolve() as $rectorClass => $paths) {
4652
if ($paths === null) {
4753
continue;
4854
}
4955

5056
// rule-scoped paths are intentional, so they are reported even as mask paths
5157
foreach ($paths as $path) {
52-
$skipDisplaysByPath[$path] = $rectorClass . ' => ' . $this->filePathHelper->relativePath($path);
58+
$relativePathsByClass[$rectorClass][$path] = $this->filePathHelper->relativePath($path);
5359
}
5460
}
5561

5662
// global mask paths like "*/some/*" are hard to spot and report false positives, skip them
63+
$globalRelativePaths = [];
5764
foreach ($this->skippedPathsResolver->resolve() as $globalPath) {
5865
if (str_contains($globalPath, '*')) {
5966
continue;
6067
}
6168

62-
$skipDisplaysByPath[$globalPath] = $this->filePathHelper->relativePath($globalPath);
69+
$globalRelativePaths[$globalPath] = $this->filePathHelper->relativePath($globalPath);
6370
}
6471

6572
$usedSkips = $processResult->getUsedSkips();
6673

6774
$unusedSkips = [];
68-
foreach ($skipDisplaysByPath as $path => $skipDisplay) {
75+
76+
// group unused rule-scoped paths under their rule, matching the "->withSkip()" config shape
77+
foreach ($relativePathsByClass as $rectorClass => $relativePaths) {
78+
$unusedRelativePaths = [];
79+
foreach ($relativePaths as $path => $relativePath) {
80+
if (! in_array($path, $usedSkips, true)) {
81+
$unusedRelativePaths[] = $relativePath;
82+
}
83+
}
84+
85+
if ($unusedRelativePaths === []) {
86+
continue;
87+
}
88+
89+
if (count($unusedRelativePaths) === 1) {
90+
$unusedSkips[] = $rectorClass . ' => ' . $unusedRelativePaths[0];
91+
continue;
92+
}
93+
94+
$unusedSkips[] = $rectorClass . ' => [ ' . implode("\n ", $unusedRelativePaths) . ' ]';
95+
}
96+
97+
foreach ($globalRelativePaths as $path => $relativePath) {
6998
if (! in_array($path, $usedSkips, true)) {
70-
$unusedSkips[] = $skipDisplay;
99+
$unusedSkips[] = $relativePath;
71100
}
72101
}
73102

‎tests/Reporting/UnusedSkipResolverTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ protected function tearDown(): void
4545
{
4646
SimpleParameterProvider::setParameter(Option::SKIP, []);
4747
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false);
48+
SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, false);
4849
}
4950

5051
public function testResolvesUnusedSkipsAsRuleAndPath(): void
@@ -78,6 +79,37 @@ public function testReportsUnusedSkipAsRelativePath(): void
7879
$this->assertContains(FifthElement::class . ' => src/Reporting/UnusedSkipResolver.php', $unusedSkips);
7980
}
8081

82+
public function testGroupsMultipleUnusedPathsUnderRule(): void
83+
{
84+
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);
85+
86+
$firstPath = getcwd() . '/src/Reporting/UnusedSkipResolver.php';
87+
$secondPath = getcwd() . '/src/Reporting/MissConfigurationReporter.php';
88+
SimpleParameterProvider::setParameter(Option::SKIP, [
89+
FifthElement::class => [$firstPath, $secondPath],
90+
]);
91+
92+
$unusedSkips = $this->unusedSkipResolver->resolve(new ProcessResult([], [], 0, []));
93+
94+
// multiple unused paths are grouped under their rule, not repeated per line
95+
$this->assertContains(
96+
FifthElement::class . ' => [ src/Reporting/UnusedSkipResolver.php' . "\n " . 'src/Reporting/MissConfigurationReporter.php ]',
97+
$unusedSkips
98+
);
99+
}
100+
101+
public function testResolvesNothingWhenRunIsNarrowed(): void
102+
{
103+
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);
104+
SimpleParameterProvider::setParameter(Option::IS_RUN_NARROWED, true);
105+
106+
// narrowed run (cli paths, "--only", "--only-suffix") only touches part of the codebase,
107+
// so skips outside that scope are not false positives
108+
$processResult = new ProcessResult([], [], 0, []);
109+
110+
$this->assertSame([], $this->unusedSkipResolver->resolve($processResult));
111+
}
112+
81113
public function testResolvesNothingWhenDisabled(): void
82114
{
83115
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, false);

0 commit comments

Comments
 (0)