diff --git a/e2e/parallel-unused-skips/custom/config/rector.php b/e2e/parallel-unused-skips/custom/config/rector.php index 015e4de8c8e..a987270fa34 100644 --- a/e2e/parallel-unused-skips/custom/config/rector.php +++ b/e2e/parallel-unused-skips/custom/config/rector.php @@ -14,10 +14,12 @@ $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 + // two concrete paths under one rule: first matched in the worker (proves parallel aggregation), + // second never matches and must be reported per-path as unused $rectorConfig->skip([ - SimplifyUselessVariableRector::class => ['*/src/*'], - '*/NonexistentUnused/*', + SimplifyUselessVariableRector::class => [ + dirname(__DIR__, 2) . '/src/SomeClass.php', + dirname(__DIR__, 2) . '/src/NonexistentUnused.php', + ], ]); }; diff --git a/e2e/parallel-unused-skips/expected-output.diff b/e2e/parallel-unused-skips/expected-output.diff index de57957a29e..eeea8a9d939 100644 --- a/e2e/parallel-unused-skips/expected-output.diff +++ b/e2e/parallel-unused-skips/expected-output.diff @@ -3,4 +3,4 @@ [WARNING] This skip is unused, it never matched any element. You can remove it from "->withSkip()" - * */NonexistentUnused/* + * ./parallel-unused-skips/src/NonexistentUnused.php diff --git a/src/Reporting/MissConfigurationReporter.php b/src/Reporting/MissConfigurationReporter.php index 42deb136fdc..4d209358596 100644 --- a/src/Reporting/MissConfigurationReporter.php +++ b/src/Reporting/MissConfigurationReporter.php @@ -36,14 +36,24 @@ public function reportUnusedSkips(ProcessResult $processResult): void return; } - // only path-scoped class skips are trackable at runtime; skip-everywhere rule skips + // only concrete path-scoped 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 - )); + $skippedClassPaths = []; + foreach ($this->skippedClassResolver->resolve() as $paths) { + if ($paths === null) { + continue; + } + + $skippedClassPaths = [...$skippedClassPaths, ...$paths]; + } - $configuredSkips = [...$pathScopedClassSkips, ...$this->skippedPathsResolver->resolve()]; + $configuredSkips = [...$skippedClassPaths, ...$this->skippedPathsResolver->resolve()]; + + // skip mask paths like "*/some/*"; they are hard to spot and report false positives + $configuredSkips = array_filter( + $configuredSkips, + static fn (string $skip): bool => ! str_contains($skip, '*') + ); $unusedSkips = array_values(array_diff($configuredSkips, $processResult->getUsedSkips())); if ($unusedSkips === []) { diff --git a/src/Skipper/Matcher/FileInfoMatcher.php b/src/Skipper/Matcher/FileInfoMatcher.php index c7110345d01..0b3fde447ef 100644 --- a/src/Skipper/Matcher/FileInfoMatcher.php +++ b/src/Skipper/Matcher/FileInfoMatcher.php @@ -23,15 +23,26 @@ public function __construct( */ public function doesFileInfoMatchPatterns(string $filePath, array $filePatterns): bool { - $filePath = PathNormalizer::normalize($filePath); + return $this->matchPattern($filePath, $filePatterns) !== null; + } + + /** + * Returns the original (un-normalized) pattern that matched, so callers can report the exact + * configured path. Returns null when no pattern matches. + * + * @param string[] $filePatterns + */ + public function matchPattern(string $filePath, array $filePatterns): ?string + { + $normalizedFilePath = PathNormalizer::normalize($filePath); foreach ($filePatterns as $filePattern) { - $filePattern = PathNormalizer::normalize($filePattern); - if ($this->doesFileMatchPattern($filePath, $filePattern)) { - return true; + $normalizedFilePattern = PathNormalizer::normalize($filePattern); + if ($this->doesFileMatchPattern($normalizedFilePath, $normalizedFilePattern)) { + return $filePattern; } } - return false; + return null; } /** diff --git a/src/Skipper/Skipper/SkipSkipper.php b/src/Skipper/Skipper/SkipSkipper.php index 69233fdbc9a..bf42c8551e3 100644 --- a/src/Skipper/Skipper/SkipSkipper.php +++ b/src/Skipper/Skipper/SkipSkipper.php @@ -30,8 +30,10 @@ public function doesMatchSkip(object | string $checker, string $filePath, array return true; } - if ($this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, $skippedFiles)) { - $this->usedSkipCollector->markUsed($skippedClass); + // mark the specific matched path used, so unused paths under the same rule are reported + $matchedPath = $this->fileInfoMatcher->matchPattern($filePath, $skippedFiles); + if ($matchedPath !== null) { + $this->usedSkipCollector->markUsed($matchedPath); return true; } } diff --git a/tests/Reporting/MissConfigurationReporterTest.php b/tests/Reporting/MissConfigurationReporterTest.php index 155460810c1..fbc79c26a7e 100644 --- a/tests/Reporting/MissConfigurationReporterTest.php +++ b/tests/Reporting/MissConfigurationReporterTest.php @@ -21,7 +21,11 @@ final class MissConfigurationReporterTest extends AbstractLazyTestCase { - private const string UNUSED_GLOB = '*/UnusedGlobMarker/*'; + private const string UNUSED_ENTITY_PATH = '/app/bundles/UserBundle/Entity'; + + private const string USED_ENTITY_PATH = '/app/bundles/AdminBundle/Entity'; + + private const string MASK_PATH = '*/SomeMask/*'; private BufferedOutput $bufferedOutput; @@ -44,12 +48,12 @@ protected function setUp(): void SimpleParameterProvider::setParameter(Option::SKIP, [ // skip-everywhere rule skip - forgotten at boot, not trackable, must be excluded ThreeMan::class, - // path-scoped class skip, never matched - must be reported - FifthElement::class => ['*/some/*'], - // path-scoped class skip, matched - must not be reported - AnotherClassToSkip::class => ['*/other/*'], - // glob path skip, never matched - must be reported - self::UNUSED_GLOB, + // concrete path-scoped class skip, never matched - must be reported + FifthElement::class => [self::UNUSED_ENTITY_PATH], + // concrete path-scoped class skip, matched - must not be reported + AnotherClassToSkip::class => [self::USED_ENTITY_PATH], + // mask path skip - hard to spot, false-positive prone, must not be reported + self::MASK_PATH, ]); } @@ -63,20 +67,23 @@ public function testReportsOnlyTrackableUnusedSkips(): void { SimpleParameterProvider::setParameter(Option::REPORT_UNUSED_SKIPS, true); - $processResult = new ProcessResult([], [], 0, [AnotherClassToSkip::class]); + // matched path is marked used by its concrete path, not by the rule class + $processResult = new ProcessResult([], [], 0, [self::USED_ENTITY_PATH]); $this->missConfigurationReporter->reportUnusedSkips($processResult); $output = $this->bufferedOutput->fetch(); - // unused, trackable skips are reported - $this->assertStringContainsString('FifthElement', $output); - $this->assertStringContainsString('UnusedGlobMarker', $output); + // unused concrete path is reported by path, not rule class + $this->assertStringContainsString('UserBundle', $output); + + // matched concrete path is excluded + $this->assertStringNotContainsString('AdminBundle', $output); + + // mask path is excluded (hard to spot, false-positive prone) + $this->assertStringNotContainsString('SomeMask', $output); // skip-everywhere rule skip is excluded (not trackable at runtime) $this->assertStringNotContainsString('ThreeMan', $output); - - // matched skip is excluded - $this->assertStringNotContainsString('AnotherClassToSkip', $output); } public function testReportsNothingWhenDisabled(): void