diff --git a/src/Skipper/FileSystem/FnMatchPathNormalizer.php b/src/Skipper/FileSystem/FnMatchPathNormalizer.php deleted file mode 100644 index dc1872b2646..00000000000 --- a/src/Skipper/FileSystem/FnMatchPathNormalizer.php +++ /dev/null @@ -1,29 +0,0 @@ -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. @@ -58,7 +40,7 @@ private function doesFileMatchPattern(string $filePath, string $ignoredPath): bo return true; } - $ignoredPath = $this->fnMatchPathNormalizer->normalizeForFnmatch($ignoredPath); + $ignoredPath = $this->normalizeForFnmatch($ignoredPath); if ($ignoredPath === '') { return false; } @@ -71,10 +53,63 @@ private function doesFileMatchPattern(string $filePath, string $ignoredPath): bo return true; } - if ($this->fnmatcher->match($ignoredPath, $filePath)) { + if ($this->matchFnmatch($ignoredPath, $filePath)) { + return true; + } + + return $this->matchRealpath($ignoredPath, $filePath); + } + + private function normalizeForFnmatch(string $path): string + { + if (str_ends_with($path, '*') || str_starts_with($path, '*')) { + return '*' . trim($path, '*') . '*'; + } + + if (str_contains($path, '..')) { + $realPath = realpath($path); + if ($realPath === false) { + return ''; + } + + return PathNormalizer::normalize($realPath); + } + + return $path; + } + + private function matchFnmatch(string $matchingPath, string $filePath): bool + { + if (fnmatch($matchingPath, $filePath)) { + return true; + } + + // in case of relative compare + return fnmatch('*/' . $matchingPath, $filePath); + } + + private function matchRealpath(string $matchingPath, string $filePath): bool + { + $realPathMatchingPath = realpath($matchingPath); + if ($realPathMatchingPath === false) { + return false; + } + + $realpathFilePath = realpath($filePath); + if ($realpathFilePath === false) { + return false; + } + + $normalizedMatchingPath = PathNormalizer::normalize($realPathMatchingPath); + $normalizedFilePath = PathNormalizer::normalize($realpathFilePath); + + // skip define direct path exactly equal + if ($normalizedMatchingPath === $normalizedFilePath) { return true; } - return $this->realpathMatcher->match($ignoredPath, $filePath); + // ensure add / suffix to ensure no same prefix directory + $suffixedMatchingPath = rtrim($normalizedMatchingPath, '/') . '/'; + return str_starts_with($normalizedFilePath, $suffixedMatchingPath); } } diff --git a/src/Skipper/RealpathMatcher.php b/src/Skipper/RealpathMatcher.php deleted file mode 100644 index 6a947fd26b1..00000000000 --- a/src/Skipper/RealpathMatcher.php +++ /dev/null @@ -1,35 +0,0 @@ -reflectionProvider->hasClass($element); - } - - public function matchSkip(string|object $element, string $filePath): ?SkipMatch - { - $skippedClasses = $this->skippedClassResolver->resolve(); - return $this->skipSkipper->match($element, $filePath, $skippedClasses); - } -} diff --git a/src/Skipper/Skipper/PathSkipper.php b/src/Skipper/Skipper/PathSkipper.php index 7d1d68d2b71..5616ad782e3 100644 --- a/src/Skipper/Skipper/PathSkipper.php +++ b/src/Skipper/Skipper/PathSkipper.php @@ -18,13 +18,12 @@ public function __construct( public function shouldSkip(string $filePath): bool { - foreach ($this->skippedPathsResolver->resolve() as $skippedPath) { - if ($this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, [$skippedPath])) { - $this->usedSkipCollector->markUsed($skippedPath); - return true; - } + $matchedPath = $this->fileInfoMatcher->matchPattern($filePath, $this->skippedPathsResolver->resolve()); + if ($matchedPath === null) { + return false; } - return false; + $this->usedSkipCollector->markUsed($matchedPath); + return true; } } diff --git a/src/Skipper/Skipper/Skipper.php b/src/Skipper/Skipper/Skipper.php index fee095cd785..9664e5595c7 100644 --- a/src/Skipper/Skipper/Skipper.php +++ b/src/Skipper/Skipper/Skipper.php @@ -5,9 +5,10 @@ namespace Rector\Skipper\Skipper; use PhpParser\Node; +use PHPStan\Reflection\ReflectionProvider; use Rector\Contract\Rector\RectorInterface; use Rector\ProcessAnalyzer\RectifiedAnalyzer; -use Rector\Skipper\SkipVoter\ClassSkipVoter; +use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; use Rector\Skipper\ValueObject\SkipMatch; /** @@ -19,7 +20,9 @@ public function __construct( private RectifiedAnalyzer $rectifiedAnalyzer, private PathSkipper $pathSkipper, - private ClassSkipVoter $classSkipVoter, + private SkipSkipper $skipSkipper, + private SkippedClassResolver $skippedClassResolver, + private ReflectionProvider $reflectionProvider, private UsedSkipCollector $usedSkipCollector, ) { } @@ -51,11 +54,11 @@ public function shouldSkipElementAndFilePath(string|object $element, string $fil */ public function matchSkip(string|object $element, string $filePath): ?SkipMatch { - if (! $this->classSkipVoter->match($element)) { + if (! is_object($element) && ! $this->reflectionProvider->hasClass($element)) { return null; } - return $this->classSkipVoter->matchSkip($element, $filePath); + return $this->skipSkipper->match($element, $filePath, $this->skippedClassResolver->resolve()); } public function markSkipUsed(SkipMatch $skipMatch): void diff --git a/tests/Skipper/FileSystem/FnMatchPathNormalizerTest.php b/tests/Skipper/FileSystem/FnMatchPathNormalizerTest.php deleted file mode 100644 index 1e176a58ae2..00000000000 --- a/tests/Skipper/FileSystem/FnMatchPathNormalizerTest.php +++ /dev/null @@ -1,43 +0,0 @@ -fnMatchPathNormalizer = $this->make(FnMatchPathNormalizer::class); - } - - #[DataProvider('providePaths')] - public function testPaths(string $path, string $expectedNormalizedPath): void - { - $normalizedPath = $this->fnMatchPathNormalizer->normalizeForFnmatch($path); - $this->assertSame($expectedNormalizedPath, $normalizedPath); - } - - /** - * @return Iterator> - */ - public static function providePaths(): Iterator - { - yield ['path/with/no/asterisk', 'path/with/no/asterisk']; - yield ['*path/with/asterisk/begin', '*path/with/asterisk/begin*']; - yield ['path/with/asterisk/end*', '*path/with/asterisk/end*']; - yield ['*path/with/asterisk/begin/and/end*', '*path/with/asterisk/begin/and/end*']; - yield [__DIR__ . '/Fixture/path/with/../in/it', PathNormalizer::normalize(__DIR__ . '/Fixture/path/in/it')]; - yield [__DIR__ . '/Fixture/path/with/../../in/it', PathNormalizer::normalize(__DIR__ . '/Fixture/in/it')]; - } -} diff --git a/tests/Skipper/Matcher/FileInfoMatcherTest.php b/tests/Skipper/Matcher/FileInfoMatcherTest.php index 862608b55c6..72f92ee4dc7 100644 --- a/tests/Skipper/Matcher/FileInfoMatcherTest.php +++ b/tests/Skipper/Matcher/FileInfoMatcherTest.php @@ -4,6 +4,8 @@ namespace Rector\Tests\Skipper\Matcher; +use Iterator; +use PHPUnit\Framework\Attributes\DataProvider; use Rector\Skipper\Matcher\FileInfoMatcher; use Rector\Testing\PHPUnit\AbstractLazyTestCase; @@ -33,9 +35,33 @@ public function testMatchPatternReturnsNullWhenNoPatternMatches(): void $this->assertNull($matchedPattern); } - public function testDoesFileInfoMatchPatternsStillReportsBoolean(): void + #[DataProvider('providePatterns')] + public function testPatternNormalization(string $filePath, string $filePattern, bool $shouldMatch): void { - $this->assertTrue($this->fileInfoMatcher->doesFileInfoMatchPatterns('/project/src/Foo.php', ['*/src/*'])); - $this->assertFalse($this->fileInfoMatcher->doesFileInfoMatchPatterns('/project/src/Foo.php', ['*/tests/*'])); + $matchedPattern = $this->fileInfoMatcher->matchPattern($filePath, [$filePattern]); + + $this->assertSame($shouldMatch ? $filePattern : null, $matchedPattern); + } + + /** + * @return Iterator + */ + public static function providePatterns(): Iterator + { + // a pattern without asterisk is used as is, and matches the path suffix + yield ['/project/path/with/no/asterisk', 'path/with/no/asterisk', true]; + yield ['/project/path/with/no/asterisk', 'path/with/another/asterisk', false]; + + // an asterisk on either end is padded to both ends + yield ['/project/path/with/asterisk/begin/Foo.php', '*path/with/asterisk/begin', true]; + yield ['/project/path/with/asterisk/end/Foo.php', 'path/with/asterisk/end*', true]; + + // ".." in a pattern is resolved against the real path + yield [__DIR__ . '/Fixture/path/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/path/with/../in/it', true]; + yield [__DIR__ . '/Fixture/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/path/with/../../in/it', true]; + yield [__DIR__ . '/Fixture/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/path/with/../in/it', false]; + + // a ".." pattern that resolves to nothing never matches + yield [__DIR__ . '/Fixture/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/missing/../nope', false]; } } diff --git a/tests/Skipper/FileSystem/Fixture/in/it/KeepThisFile.txt b/tests/Skipper/Matcher/Fixture/in/it/KeepThisFile.txt similarity index 100% rename from tests/Skipper/FileSystem/Fixture/in/it/KeepThisFile.txt rename to tests/Skipper/Matcher/Fixture/in/it/KeepThisFile.txt diff --git a/tests/Skipper/FileSystem/Fixture/path/in/it/KeepThisFile.txt b/tests/Skipper/Matcher/Fixture/path/in/it/KeepThisFile.txt similarity index 100% rename from tests/Skipper/FileSystem/Fixture/path/in/it/KeepThisFile.txt rename to tests/Skipper/Matcher/Fixture/path/in/it/KeepThisFile.txt diff --git a/tests/Skipper/FileSystem/Fixture/path/with/KeepThisFile.txt b/tests/Skipper/Matcher/Fixture/path/with/KeepThisFile.txt similarity index 100% rename from tests/Skipper/FileSystem/Fixture/path/with/KeepThisFile.txt rename to tests/Skipper/Matcher/Fixture/path/with/KeepThisFile.txt