[skip] fix match class + path matching in reporting of unused skips

Author: TomasVotruba

src/Reporting/UnusedSkipResolver.php

@@ -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;
                 }
             }

src/Skipper/Skipper/SkipSkipper.php

@@ -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;
             }
         }

tests/Reporting/MissConfigurationReporterTest.php

@@ -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();

tests/Reporting/UnusedSkipResolverTest.php

@@ -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
@@ -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);

tests/Skipper/Skipper/UsedSkipCollectorTest.php

@@ -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);
     }
 }