diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/SkipUsedTrackingTest.php b/rules-tests/Renaming/Rector/Name/RenameClassRector/SkipUsedTrackingTest.php index 4a12fa76cbf..8c1421abc99 100644 --- a/rules-tests/Renaming/Rector/Name/RenameClassRector/SkipUsedTrackingTest.php +++ b/rules-tests/Renaming/Rector/Name/RenameClassRector/SkipUsedTrackingTest.php @@ -10,7 +10,8 @@ use Rector\Testing\PHPUnit\AbstractRectorTestCase; /** - * A class/path skip is only "used" when the rule would actually have changed the skipped file. + * A rule-scoped path skip is marked used for every file whose path it matches, as the rule is + * skipped once per file. * * @see RenameClassRector */ @@ -32,7 +33,7 @@ public function testMarksSkipUsedOnlyWhenRuleWouldChangeFile(): void $usedPaths = $usedSkips[RenameClassRector::class] ?? []; - // the skip that actually prevented a rename is marked used + // both skip masks match a processed file, so both are marked used $this->assertContains('*skip_used_renames_old_class*', $usedPaths); // the skip on a file the rule would not have touched is never marked used diff --git a/src/Application/RectorRegistry.php b/src/Application/RectorRegistry.php index 4e2679f3b57..691f8c03b8f 100644 --- a/src/Application/RectorRegistry.php +++ b/src/Application/RectorRegistry.php @@ -33,9 +33,7 @@ public function refreshRectors(array $rectors): void } /** - * @return RectorInterface[] - * - * @api used in tests + * @return array */ public function forPath(string $filePath): array { diff --git a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php index 417f012c4ae..a076c16c92c 100644 --- a/src/PhpParser/NodeTraverser/RectorNodeTraverser.php +++ b/src/PhpParser/NodeTraverser/RectorNodeTraverser.php @@ -17,15 +17,7 @@ use Webmozart\Assert\Assert; /** - * Based on native NodeTraverser class, but heavily customized for Rector needs. - * - * The main differences are: - * - no leaveNode(), the RectorRunner calls each rule's refactor() method on enter - * - cached visitors per node class for performance, e.g. when we find rules for Class_ node, they're cached for next time - * - immutability features, register Rector rules once, then use; no changes on the fly - * * @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest - * @internal No BC promise on this class, it might change any time. */ final class RectorNodeTraverser { diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index 65917eb0415..828d0528714 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -9,6 +9,7 @@ use Nette\Utils\Strings; use PHPUnit\Framework\ExpectationFailedException; use Rector\Application\ApplicationFileProcessor; +use Rector\Application\RectorRegistry; use Rector\Autoloading\AdditionalAutoloader; use Rector\Autoloading\BootstrapFilesIncluder; use Rector\Composer\InstalledPackageResolver; @@ -112,6 +113,11 @@ protected function setUp(): void $rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class); $rectorNodeTraverser->refreshPhpRectors($rectors); + // keep the shared registry in sync with the current test's rules + /** @var RectorRegistry $rectorRegistry */ + $rectorRegistry = $rectorConfig->make(RectorRegistry::class); + $rectorRegistry->refreshRectors($rectors); + // store cache self::$cacheByRuleAndConfig[$cacheKey] = true; } diff --git a/tests/Application/RectorRegistryTest.php b/tests/Application/RectorRegistryTest.php index f20fbd0acff..bff14754b54 100644 --- a/tests/Application/RectorRegistryTest.php +++ b/tests/Application/RectorRegistryTest.php @@ -8,7 +8,7 @@ use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Testing\PHPUnit\AbstractLazyTestCase; -use Rector\Tests\Application\Source\SkippableRector; +use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement; final class RectorRegistryTest extends AbstractLazyTestCase { @@ -19,11 +19,11 @@ protected function setUp(): void parent::setUp(); SimpleParameterProvider::setParameter(Option::SKIP, [ - SkippableRector::class => ['*/skipped_directory/*'], + FifthElement::class => ['*/skipped_directory/*'], ]); $this->rectorRegistry = $this->make(RectorRegistry::class); - $this->rectorRegistry->refreshRectors([new SkippableRector()]); + $this->rectorRegistry->refreshRectors([new FifthElement()]); } protected function tearDown(): void @@ -43,6 +43,6 @@ public function testForPathKeepsRectorForOtherPath(): void $rectorsForPath = $this->rectorRegistry->forPath(__DIR__ . '/some_file.php'); $this->assertCount(1, $rectorsForPath); - $this->assertInstanceOf(SkippableRector::class, $rectorsForPath[0]); + $this->assertInstanceOf(FifthElement::class, $rectorsForPath[0]); } } diff --git a/tests/Application/Source/SkippableRector.php b/tests/Application/Source/SkippableRector.php deleted file mode 100644 index d7a63644f10..00000000000 --- a/tests/Application/Source/SkippableRector.php +++ /dev/null @@ -1,27 +0,0 @@ - ['some/path'], + NotSkippedClass::class => ['some/path'], // Rector rule, must not be reported OrdSingleByteRector::class => ['some/path'], // post rector rule, must not be reported @@ -80,7 +81,8 @@ public function testReportsSkippedNonRectorClass(): void $output = $this->bufferedOutput->fetch(); - $this->assertStringContainsString('AnotherClassToSkip', $output); + $this->assertStringContainsString('NotSkippedClass', $output); + $this->assertStringNotContainsString('OrdSingleByteRector', $output); $this->assertStringNotContainsString('NameImportingPostRector', $output); } diff --git a/tests/Skipper/Skipper/Fixture/Element/FifthElement.php b/tests/Skipper/Skipper/Fixture/Element/FifthElement.php index 2b49be13b67..0835815f472 100644 --- a/tests/Skipper/Skipper/Fixture/Element/FifthElement.php +++ b/tests/Skipper/Skipper/Fixture/Element/FifthElement.php @@ -4,6 +4,23 @@ namespace Rector\Tests\Skipper\Skipper\Fixture\Element; -final class FifthElement +use PhpParser\Node; +use Rector\RuleDoc\RuleDefinition; + +final class FifthElement implements \Rector\Contract\Rector\RectorInterface { + public function getRuleDefinition(): RuleDefinition + { + // TODO: Implement getRuleDefinition() method. + } + + public function getNodeTypes(): array + { + // TODO: Implement getNodeTypes() method. + } + + public function refactor(Node $node) + { + // TODO: Implement refactor() method. + } } diff --git a/tests/Skipper/Skipper/SkipperTest.php b/tests/Skipper/Skipper/SkipperTest.php index d64748f52bc..7c43afa3888 100644 --- a/tests/Skipper/Skipper/SkipperTest.php +++ b/tests/Skipper/Skipper/SkipperTest.php @@ -68,23 +68,23 @@ public static function provideDataShouldSkipFilePath(): Iterator yield [__DIR__ . '/Fixture/PathSkippedWithMask/another_file.txt', true]; } - #[DataProvider('provideCheckerAndFile')] - public function testSkipRectorAndFile(object $rector, string $filePath, bool $expectedSkip): void + #[DataProvider('provideRectorAndFile')] + public function testSkipElementAndFilePath(object $rector, string $filePath, bool $expectedSkip): void { $resolvedSkip = $this->skipper->shouldSkipRectorAndFile($rector, $filePath); $this->assertSame($expectedSkip, $resolvedSkip); } - /** - * @return Iterator, mixed>> - */ - public static function provideCheckerAndFile(): Iterator + public static function provideRectorAndFile(): Iterator { yield [new FifthElement(), __DIR__ . '/Fixture', true]; yield [new AnotherClassToSkip(), __DIR__ . '/Fixture/someFile', true]; yield [new AnotherClassToSkip(), __DIR__ . '/Fixture/someDirectory/anotherFile.php', true]; + yield [new FifthElement(), __DIR__ . '/Fixture/someFile', true]; + yield [new FifthElement(), __DIR__ . '/Fixture/someDirectory/anotherFile.php', true]; + yield [new NotSkippedClass(), __DIR__ . '/Fixture/someFile', false]; yield [new NotSkippedClass(), __DIR__ . '/Fixture/someOtherFile', false]; } diff --git a/tests/Skipper/Skipper/Source/AnotherClassToSkip.php b/tests/Skipper/Skipper/Source/AnotherClassToSkip.php index 150276ff3f3..6f24714ab14 100644 --- a/tests/Skipper/Skipper/Source/AnotherClassToSkip.php +++ b/tests/Skipper/Skipper/Source/AnotherClassToSkip.php @@ -4,6 +4,24 @@ namespace Rector\Tests\Skipper\Skipper\Source; -final class AnotherClassToSkip +use PhpParser\Node; +use Rector\Contract\Rector\RectorInterface; +use Rector\RuleDoc\RuleDefinition; + +final class AnotherClassToSkip implements RectorInterface { + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition('Fixture rule used to assert path scoped skipping', []); + } + + public function getNodeTypes(): array + { + return []; + } + + public function refactor(Node $node) + { + return null; + } } diff --git a/tests/Skipper/Skipper/UsedSkipCollectorTest.php b/tests/Skipper/Skipper/UsedSkipCollectorTest.php index 3b6c862db55..a64611a9ce3 100644 --- a/tests/Skipper/Skipper/UsedSkipCollectorTest.php +++ b/tests/Skipper/Skipper/UsedSkipCollectorTest.php @@ -50,7 +50,7 @@ protected function tearDown(): void public function testCollectsOnlyMatchedSkips(): void { $this->skipper->shouldSkipFilePath('tests/Skipper/Skipper/Fixture/SomeSkippedPath/any.txt'); - $this->skipper->shouldSkipRectorAndFile(new FifthElement(), __FILE__); + $this->skipper->shouldSkipRectorAndFile(new FifthElement(), __DIR__ . '/Fixture/someFile'); $usedSkips = $this->usedSkipCollector->provide();