diff --git a/src/Config/RectorConfig.php b/src/Config/RectorConfig.php index ac3ed8bcd80..161d7fa726c 100644 --- a/src/Config/RectorConfig.php +++ b/src/Config/RectorConfig.php @@ -568,6 +568,38 @@ public function forgetByContract(string $contract): void } } + /** + * The container caches every instance it builds, so a rule that another service takes as a + * constructor dependency is cached like any other object. Returned unfiltered it would count + * as an active rule although no config ever registered it — and this is where the active set + * comes from on both paths: the node traverser is autowired from an "@param RectorInterface[]" + * parameter, and AbstractRectorTestCase reads the same list. + * + * The narrowing keys off the requested contract, so a wider one still sees every instance - + * findByContract(NodeVisitor::class) includes unregistered rules, since RectorInterface + * extends NodeVisitor. + * + * @template TType as object + * + * @param class-string $contractClass + * @return list + */ + public function findByContract(string $contractClass): array + { + $instances = parent::findByContract($contractClass); + + if (! is_a($contractClass, RectorInterface::class, true)) { + return $instances; + } + + return array_values( + array_filter( + $instances, + fn (object $instance): bool => isset($this->registeredRectorClasses[$instance::class]) + ) + ); + } + public function reportingRealPath(bool $absolute = true): void { SimpleParameterProvider::setParameter(Option::ABSOLUTE_FILE_PATH, $absolute); diff --git a/src/Testing/PHPUnit/AbstractRectorTestCase.php b/src/Testing/PHPUnit/AbstractRectorTestCase.php index f90461c745a..5c5eef5b464 100644 --- a/src/Testing/PHPUnit/AbstractRectorTestCase.php +++ b/src/Testing/PHPUnit/AbstractRectorTestCase.php @@ -7,6 +7,7 @@ use Iterator; use Nette\Utils\FileSystem; use Nette\Utils\Strings; +use PhpParser\NodeVisitor; use PHPUnit\Framework\ExpectationFailedException; use Rector\Application\ApplicationFileProcessor; use Rector\Autoloading\AdditionalAutoloader; @@ -100,8 +101,15 @@ protected function setUp(): void $this->forgetRectorsRules(); $rectorConfig->resetRuleConfigurations(); - // this has to be always empty, so we can add new rules with their configuration - $this->assertEmpty($rectorConfig->findByContract(RectorInterface::class)); + // this has to be always empty, so we can add new rules with their configuration. + // asked through the wider contract on purpose: the rule contract now reports only + // registered rules, and the registered list was just cleared, so it would answer + // "empty" without looking at what the forget above actually left behind + $leftOverRectors = array_filter( + $rectorConfig->findByContract(NodeVisitor::class), + static fn (NodeVisitor $nodeVisitor): bool => $nodeVisitor instanceof RectorInterface + ); + $this->assertSame([], $leftOverRectors); $this->bootFromConfigFiles([$configFile]); diff --git a/tests/Config/RectorConfigTest.php b/tests/Config/RectorConfigTest.php index 0f054548c7c..810aff16fa6 100644 --- a/tests/Config/RectorConfigTest.php +++ b/tests/Config/RectorConfigTest.php @@ -7,6 +7,7 @@ use Rector\Config\RectorConfig; use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; +use Rector\Contract\Rector\RectorInterface; use Rector\Renaming\Rector\MethodCall\RenameMethodRector; use Rector\Renaming\Rector\Name\RenameClassRector; use Rector\Renaming\Rector\PropertyFetch\RenamePropertyRector; @@ -14,6 +15,8 @@ use Rector\Renaming\ValueObject\RenameProperty; use Rector\Symfony\Set\SymfonySetList; use Rector\Testing\PHPUnit\AbstractLazyTestCase; +use Rector\Tests\Config\Source\DependencyOnlyRector; +use Rector\Tests\Config\Source\RegisteringRector; use Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector; final class RectorConfigTest extends AbstractLazyTestCase @@ -89,4 +92,21 @@ public function testRuleWithConfigurationComposerVersionBoundOnMissingPackage(): $registeredRectorRules = SimpleParameterProvider::provideArrayParameter(Option::REGISTERED_RECTOR_RULES); $this->assertNotContains(RenamePropertyRector::class, $registeredRectorRules); } + + public function testRuleTakenOnlyAsDependencyIsNotActive(): void + { + $rectorConfig = $this->getContainer(); + + $rectorConfig->rule(RegisteringRector::class); + + // building the registered rule also builds and caches its constructor dependency, + // which is a rule too - but one no config asked for + $activeRectorClasses = array_map( + static fn (object $rector): string => $rector::class, + $rectorConfig->findByContract(RectorInterface::class) + ); + + $this->assertContains(RegisteringRector::class, $activeRectorClasses); + $this->assertNotContains(DependencyOnlyRector::class, $activeRectorClasses); + } } diff --git a/tests/Config/Source/DependencyOnlyRector.php b/tests/Config/Source/DependencyOnlyRector.php new file mode 100644 index 00000000000..1ab97353c88 --- /dev/null +++ b/tests/Config/Source/DependencyOnlyRector.php @@ -0,0 +1,35 @@ +> + */ + public function getNodeTypes(): array + { + return [String_::class]; + } + + public function refactor(Node $node): ?Node + { + return null; + } +} diff --git a/tests/Config/Source/RegisteringRector.php b/tests/Config/Source/RegisteringRector.php new file mode 100644 index 00000000000..64e93579ec8 --- /dev/null +++ b/tests/Config/Source/RegisteringRector.php @@ -0,0 +1,36 @@ +> + */ + public function getNodeTypes(): array + { + return [String_::class]; + } + + public function refactor(Node $node): ?Node + { + return $this->dependencyOnlyRector->refactor($node); + } +}