Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/Reporting/UnusedSkipResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ private function resolveUnusedRuleScopedSkips(array $usedSkips): array
foreach ($this->resolveRelativePathsByClass() as $rectorClass => $relativePaths) {
$unusedRelativePaths = [];
foreach ($relativePaths as $path => $relativePath) {
if (! in_array($path, $usedSkips, true)) {
// used skips are tracked scoped to their rule as "class|path", so the same path
// skipped under another rule does not mark this one used (see SkipSkipper)
if (! in_array($rectorClass . '|' . $path, $usedSkips, true)) {
$unusedRelativePaths[] = $relativePath;
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/Skipper/Skipper/SkipSkipper.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ public function doesMatchSkip(object | string $checker, string $filePath, array
return true;
}

// mark the specific matched path used, so unused paths under the same rule are reported
// mark the specific matched path used, scoped to its rule via "class|path" - the same
// path can be skipped under multiple rules, so a path-only key would mark them all used
$matchedPath = $this->fileInfoMatcher->matchPattern($filePath, $skippedFiles);
if ($matchedPath !== null) {
$this->usedSkipCollector->markUsed($matchedPath);
$this->usedSkipCollector->markUsed($skippedClass . '|' . $matchedPath);
return true;
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Reporting/MissConfigurationReporterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ public function testReportsOnlyTrackableUnusedSkips(): void
{
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);

// matched rule-scoped path is marked used by its path, not by the rule class
$processResult = new ProcessResult([], [], 0, [self::USED_RULE_MASK]);
// matched rule-scoped path is marked used scoped to its rule as "class|path"
$processResult = new ProcessResult([], [], 0, [AnotherClassToSkip::class . '|' . self::USED_RULE_MASK]);
$this->missConfigurationReporter->reportUnusedSkips($processResult);

$output = $this->bufferedOutput->fetch();
Expand Down
25 changes: 24 additions & 1 deletion tests/Reporting/UnusedSkipResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public function testResolvesUnusedSkipsAsRuleAndPath(): void
{
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);

$processResult = new ProcessResult([], [], 0, [self::USED_RULE_MASK]);
// used skips are tracked scoped to their rule as "class|path"
$processResult = new ProcessResult([], [], 0, [AnotherClassToSkip::class . '|' . self::USED_RULE_MASK]);
$unusedSkips = $this->unusedSkipResolver->resolve($processResult);

// rule-scoped unused skip is reported as "rule:" with the path nested below
Expand All @@ -65,6 +66,28 @@ public function testResolvesUnusedSkipsAsRuleAndPath(): void
$this->assertNotContains(ThreeMan::class, $unusedSkips);
}

public function testSamePathUnderAnotherRuleDoesNotMarkSkipUsed(): void
{
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);

$sharedMask = '*/shared-mask/*';
SimpleParameterProvider::setParameter(Option::SKIP, [
// same path skipped under two rules - only the first one actually matched a file
AnotherClassToSkip::class => [$sharedMask],
FifthElement::class => [$sharedMask],
]);

// used skip is scoped to its rule, so the shared path stays unused under the other rule
$processResult = new ProcessResult([], [], 0, [AnotherClassToSkip::class . '|' . $sharedMask]);
$unusedSkips = $this->unusedSkipResolver->resolve($processResult);

// the never-matched rule still reports its shared path as unused
$this->assertContains(FifthElement::class . ':' . "\n * " . $sharedMask, $unusedSkips);

// the matched rule is excluded
$this->assertNotContains(AnotherClassToSkip::class . ':' . "\n * " . $sharedMask, $unusedSkips);
}

public function testReportsUnusedSkipAsRelativePath(): void
{
SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true);
Expand Down
6 changes: 3 additions & 3 deletions tests/Skipper/Skipper/UsedSkipCollectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ public function testCollectsMatchedPathNotRuleClassForRuleScopedSkip(): void

$usedSkips = $this->usedSkipCollector->provide();

// the specific matched path is collected, not the rule class
$this->assertContains('*/someDirectory/*', $usedSkips);
// the specific matched path is collected scoped to its rule as "class|path"
$this->assertContains(AnotherClassToSkip::class . '|' . '*/someDirectory/*', $usedSkips);
$this->assertNotContains(AnotherClassToSkip::class, $usedSkips);

// the unmatched sibling path under the same rule is never collected
$this->assertNotContains(self::UNUSED_SKIP_MARKER, $usedSkips);
$this->assertNotContains(AnotherClassToSkip::class . '|' . self::UNUSED_SKIP_MARKER, $usedSkips);
}
}
Loading