Skip to content

Commit ea5d0a4

Browse files
TomasVotrubaTomasVotruba
authored
refactor: collapse single-use Skipper collaborators (#8226)
ClassSkipVoter was the only implementation of a voter pattern and the only consumer of its own indirection: both methods were pass-throughs to SkipSkipper and SkippedClassResolver. Inline it into Skipper. Fnmatcher, RealpathMatcher and FnMatchPathNormalizer were one-method services injected into FileInfoMatcher and used nowhere else. They are steps of one matching algorithm, so fold them in as private methods; FileInfoMatcher now has no constructor dependencies. PathSkipper looped over skipped paths and passed each one to the matcher as a single-element array, while matchPattern() already loops and returns the matching pattern. Call it once. That leaves doesFileInfoMatchPatterns() without callers, so drop it. FnMatchPathNormalizerTest cases move to FileInfoMatcherTest, asserted through the public matchPattern() instead of the folded-in method.
1 parent 840a9a4 commit ea5d0a4

12 files changed

Lines changed: 98 additions & 195 deletions

File tree

‎src/Skipper/FileSystem/FnMatchPathNormalizer.php‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎src/Skipper/Fnmatcher.php‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎src/Skipper/Matcher/FileInfoMatcher.php‎

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,13 @@
44

55
namespace Rector\Skipper\Matcher;
66

7-
use Rector\Skipper\FileSystem\FnMatchPathNormalizer;
87
use Rector\Skipper\FileSystem\PathNormalizer;
9-
use Rector\Skipper\Fnmatcher;
10-
use Rector\Skipper\RealpathMatcher;
118

129
/**
1310
* @see \Rector\Tests\Skipper\Matcher\FileInfoMatcherTest
1411
*/
15-
final readonly class FileInfoMatcher
12+
final class FileInfoMatcher
1613
{
17-
public function __construct(
18-
private FnMatchPathNormalizer $fnMatchPathNormalizer,
19-
private Fnmatcher $fnmatcher,
20-
private RealpathMatcher $realpathMatcher
21-
) {
22-
}
23-
24-
/**
25-
* @param string[] $filePatterns
26-
*/
27-
public function doesFileInfoMatchPatterns(string $filePath, array $filePatterns): bool
28-
{
29-
return $this->matchPattern($filePath, $filePatterns) !== null;
30-
}
31-
3214
/**
3315
* Returns the original (un-normalized) pattern that matched, so callers can report the exact
3416
* configured path. Returns null when no pattern matches.
@@ -58,7 +40,7 @@ private function doesFileMatchPattern(string $filePath, string $ignoredPath): bo
5840
return true;
5941
}
6042

61-
$ignoredPath = $this->fnMatchPathNormalizer->normalizeForFnmatch($ignoredPath);
43+
$ignoredPath = $this->normalizeForFnmatch($ignoredPath);
6244
if ($ignoredPath === '') {
6345
return false;
6446
}
@@ -71,10 +53,63 @@ private function doesFileMatchPattern(string $filePath, string $ignoredPath): bo
7153
return true;
7254
}
7355

74-
if ($this->fnmatcher->match($ignoredPath, $filePath)) {
56+
if ($this->matchFnmatch($ignoredPath, $filePath)) {
57+
return true;
58+
}
59+
60+
return $this->matchRealpath($ignoredPath, $filePath);
61+
}
62+
63+
private function normalizeForFnmatch(string $path): string
64+
{
65+
if (str_ends_with($path, '*') || str_starts_with($path, '*')) {
66+
return '*' . trim($path, '*') . '*';
67+
}
68+
69+
if (str_contains($path, '..')) {
70+
$realPath = realpath($path);
71+
if ($realPath === false) {
72+
return '';
73+
}
74+
75+
return PathNormalizer::normalize($realPath);
76+
}
77+
78+
return $path;
79+
}
80+
81+
private function matchFnmatch(string $matchingPath, string $filePath): bool
82+
{
83+
if (fnmatch($matchingPath, $filePath)) {
84+
return true;
85+
}
86+
87+
// in case of relative compare
88+
return fnmatch('*/' . $matchingPath, $filePath);
89+
}
90+
91+
private function matchRealpath(string $matchingPath, string $filePath): bool
92+
{
93+
$realPathMatchingPath = realpath($matchingPath);
94+
if ($realPathMatchingPath === false) {
95+
return false;
96+
}
97+
98+
$realpathFilePath = realpath($filePath);
99+
if ($realpathFilePath === false) {
100+
return false;
101+
}
102+
103+
$normalizedMatchingPath = PathNormalizer::normalize($realPathMatchingPath);
104+
$normalizedFilePath = PathNormalizer::normalize($realpathFilePath);
105+
106+
// skip define direct path exactly equal
107+
if ($normalizedMatchingPath === $normalizedFilePath) {
75108
return true;
76109
}
77110

78-
return $this->realpathMatcher->match($ignoredPath, $filePath);
111+
// ensure add / suffix to ensure no same prefix directory
112+
$suffixedMatchingPath = rtrim($normalizedMatchingPath, '/') . '/';
113+
return str_starts_with($normalizedFilePath, $suffixedMatchingPath);
79114
}
80115
}

‎src/Skipper/RealpathMatcher.php‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎src/Skipper/SkipVoter/ClassSkipVoter.php‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎src/Skipper/Skipper/PathSkipper.php‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,12 @@ public function __construct(
1818

1919
public function shouldSkip(string $filePath): bool
2020
{
21-
foreach ($this->skippedPathsResolver->resolve() as $skippedPath) {
22-
if ($this->fileInfoMatcher->doesFileInfoMatchPatterns($filePath, [$skippedPath])) {
23-
$this->usedSkipCollector->markUsed($skippedPath);
24-
return true;
25-
}
21+
$matchedPath = $this->fileInfoMatcher->matchPattern($filePath, $this->skippedPathsResolver->resolve());
22+
if ($matchedPath === null) {
23+
return false;
2624
}
2725

28-
return false;
26+
$this->usedSkipCollector->markUsed($matchedPath);
27+
return true;
2928
}
3029
}

‎src/Skipper/Skipper/Skipper.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
namespace Rector\Skipper\Skipper;
66

77
use PhpParser\Node;
8+
use PHPStan\Reflection\ReflectionProvider;
89
use Rector\Contract\Rector\RectorInterface;
910
use Rector\ProcessAnalyzer\RectifiedAnalyzer;
10-
use Rector\Skipper\SkipVoter\ClassSkipVoter;
11+
use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver;
1112
use Rector\Skipper\ValueObject\SkipMatch;
1213

1314
/**
@@ -19,7 +20,9 @@
1920
public function __construct(
2021
private RectifiedAnalyzer $rectifiedAnalyzer,
2122
private PathSkipper $pathSkipper,
22-
private ClassSkipVoter $classSkipVoter,
23+
private SkipSkipper $skipSkipper,
24+
private SkippedClassResolver $skippedClassResolver,
25+
private ReflectionProvider $reflectionProvider,
2326
private UsedSkipCollector $usedSkipCollector,
2427
) {
2528
}
@@ -51,11 +54,11 @@ public function shouldSkipElementAndFilePath(string|object $element, string $fil
5154
*/
5255
public function matchSkip(string|object $element, string $filePath): ?SkipMatch
5356
{
54-
if (! $this->classSkipVoter->match($element)) {
57+
if (! is_object($element) && ! $this->reflectionProvider->hasClass($element)) {
5558
return null;
5659
}
5760

58-
return $this->classSkipVoter->matchSkip($element, $filePath);
61+
return $this->skipSkipper->match($element, $filePath, $this->skippedClassResolver->resolve());
5962
}
6063

6164
public function markSkipUsed(SkipMatch $skipMatch): void

‎tests/Skipper/FileSystem/FnMatchPathNormalizerTest.php‎

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎tests/Skipper/Matcher/FileInfoMatcherTest.php‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace Rector\Tests\Skipper\Matcher;
66

7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
79
use Rector\Skipper\Matcher\FileInfoMatcher;
810
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
911

@@ -33,9 +35,33 @@ public function testMatchPatternReturnsNullWhenNoPatternMatches(): void
3335
$this->assertNull($matchedPattern);
3436
}
3537

36-
public function testDoesFileInfoMatchPatternsStillReportsBoolean(): void
38+
#[DataProvider('providePatterns')]
39+
public function testPatternNormalization(string $filePath, string $filePattern, bool $shouldMatch): void
3740
{
38-
$this->assertTrue($this->fileInfoMatcher->doesFileInfoMatchPatterns('/project/src/Foo.php', ['*/src/*']));
39-
$this->assertFalse($this->fileInfoMatcher->doesFileInfoMatchPatterns('/project/src/Foo.php', ['*/tests/*']));
41+
$matchedPattern = $this->fileInfoMatcher->matchPattern($filePath, [$filePattern]);
42+
43+
$this->assertSame($shouldMatch ? $filePattern : null, $matchedPattern);
44+
}
45+
46+
/**
47+
* @return Iterator<array{string, string, bool}>
48+
*/
49+
public static function providePatterns(): Iterator
50+
{
51+
// a pattern without asterisk is used as is, and matches the path suffix
52+
yield ['/project/path/with/no/asterisk', 'path/with/no/asterisk', true];
53+
yield ['/project/path/with/no/asterisk', 'path/with/another/asterisk', false];
54+
55+
// an asterisk on either end is padded to both ends
56+
yield ['/project/path/with/asterisk/begin/Foo.php', '*path/with/asterisk/begin', true];
57+
yield ['/project/path/with/asterisk/end/Foo.php', 'path/with/asterisk/end*', true];
58+
59+
// ".." in a pattern is resolved against the real path
60+
yield [__DIR__ . '/Fixture/path/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/path/with/../in/it', true];
61+
yield [__DIR__ . '/Fixture/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/path/with/../../in/it', true];
62+
yield [__DIR__ . '/Fixture/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/path/with/../in/it', false];
63+
64+
// a ".." pattern that resolves to nothing never matches
65+
yield [__DIR__ . '/Fixture/in/it/KeepThisFile.txt', __DIR__ . '/Fixture/missing/../nope', false];
4066
}
4167
}

tests/Skipper/FileSystem/Fixture/in/it/KeepThisFile.txt renamed to tests/Skipper/Matcher/Fixture/in/it/KeepThisFile.txt

File renamed without changes.

0 commit comments

Comments
 (0)