Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/Application/FileProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public function __construct(
private RectorParser $rectorParser,
private NodeScopeAndMetadataDecorator $nodeScopeAndMetadataDecorator,
private UsedImportsResolver $usedImportsResolver,
private RectorRegistry $rectorRegistry,
) {
}

Expand All @@ -60,9 +59,6 @@ public function processFile(File $file, Configuration $configuration): FileProce
$fileHasChanged = false;
$filePath = $file->getFilePath();

$currentRectors = $this->rectorRegistry->getRectors();
$this->rectorNodeTraverser->refreshPhpRectors($currentRectors);

do {
$file->changeHasChanged(false);

Expand Down
63 changes: 59 additions & 4 deletions src/Application/RectorRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,92 @@

namespace Rector\Application;

use PhpParser\Node;
use Rector\Configuration\ConfigurationRuleFilter;
use Rector\Contract\Rector\RectorInterface;
use Rector\VersionBonding\ComposerPackageConstraintFilter;
use Rector\VersionBonding\PhpVersionedFilter;

/**
* @see \Rector\Tests\Application\RectorRegistryTest
*/
final class RectorRegistry
{
private bool $areRectorsPrepared = false;

/**
* @var RectorInterface[]
*/
private array $activeRectors = [];

/**
* @var array<class-string<Node>, RectorInterface[]>
*/
private array $cache = [];

/**
* @param RectorInterface[] $rectors
*/
public function __construct(
private readonly PhpVersionedFilter $phpVersionedFilter,
private readonly ComposerPackageConstraintFilter $composerPackageConstraintFilter,
private readonly ConfigurationRuleFilter $configurationRuleFilter,
private array $rectors,
) {
}

/**
* @param RectorInterface[] $rectors
* @api used in tests to update the active rules
*
* @internal Used only in Rector core, not supported outside. Might change any time.
*/
public function refreshRectors(array $rectors): void
{
$this->rectors = $rectors;
$this->areRectorsPrepared = false;
$this->cache = [];
}

/**
* @return array<RectorInterface>
* @return RectorInterface[]
*/
public function getRectors(): array
public function forNode(Node $node): array
{
return $this->rectors;
$this->prepareRectors();

$nodeClass = $node::class;

if (! isset($this->cache[$nodeClass])) {
$this->cache[$nodeClass] = [];

foreach ($this->activeRectors as $rector) {
foreach ($rector->getNodeTypes() as $nodeType) {
if (is_a($nodeClass, $nodeType, true)) {
$this->cache[$nodeClass][] = $rector;
continue 2;
}
}
}
}

return $this->cache[$nodeClass];
}

/**
* Filtering must happen lazily, after the configuration is set in ProcessCommand::execute(),
* otherwise options like --only are not yet available and get ignored.
*/
private function prepareRectors(): void
{
if ($this->areRectorsPrepared) {
return;
}

$activeRectors = $this->phpVersionedFilter->filter($this->rectors);
$activeRectors = $this->composerPackageConstraintFilter->filter($activeRectors);

$this->activeRectors = $this->configurationRuleFilter->filter($activeRectors);

$this->areRectorsPrepared = true;
}
}
2 changes: 2 additions & 0 deletions src/Configuration/ConfigurationRuleFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use Rector\VersionBonding\ValueObject\ComposerBoundRuleConfiguration;

/**
* @todo merge to RectorRegistry, as it is only used there
*
* Modify available rector rules based on the configuration options
* @see \Rector\Tests\Configuration\ConfigurationRuleFilterTest
*/
Expand Down
103 changes: 4 additions & 99 deletions src/PhpParser/NodeTraverser/RectorNodeTraverser.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,41 +9,16 @@
use PhpParser\Node\Expr;
use PhpParser\Node\Stmt;
use PhpParser\NodeVisitor;
use Rector\Configuration\ConfigurationRuleFilter;
use Rector\Contract\Rector\RectorInterface;
use Rector\Application\RectorRegistry;
use Rector\Rector\RectorRunner;
use Rector\VersionBonding\ComposerPackageConstraintFilter;
use Rector\VersionBonding\PhpVersionedFilter;
use Webmozart\Assert\Assert;

/**
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
*/
final class RectorNodeTraverser
{
/**
* @var RectorInterface[]
*/
private array $visitors = [];

private bool $stopTraversal;

private bool $areNodeVisitorsPrepared = false;

/**
* @var array<class-string<Node>, RectorInterface[]>
*/
private array $visitorsPerNodeClass = [];

/**
* @param RectorInterface[] $rectors
*/
public function __construct(
private array $rectors,
private readonly PhpVersionedFilter $phpVersionedFilter,
private readonly ComposerPackageConstraintFilter $composerPackageConstraintFilter,
private readonly ConfigurationRuleFilter $configurationRuleFilter,
private readonly RectorRunner $rectorRunner,
private readonly RectorRegistry $rectorRegistry,
) {
}

Expand All @@ -53,57 +28,11 @@ public function __construct(
*/
public function traverse(array $nodes): array
{
$this->prepareNodeVisitors();

$this->stopTraversal = false;

return $this->traverseArray($nodes);
}

/**
* @param RectorInterface[] $rectors
*
* @internal Used only in Rector core, not supported outside. Might change any time.
*/
public function refreshPhpRectors(array $rectors): void
{
Assert::allIsInstanceOf($rectors, RectorInterface::class);

$this->rectors = $rectors;
$this->visitors = [];
$this->visitorsPerNodeClass = [];

$this->areNodeVisitorsPrepared = false;

$this->prepareNodeVisitors();
}

/**
* @return RectorInterface[]
*
* @api used in tests
*/
public function getVisitorsForNode(Node $node): array
{
$nodeClass = $node::class;

if (! isset($this->visitorsPerNodeClass[$nodeClass])) {
$this->visitorsPerNodeClass[$nodeClass] = [];

/** @var RectorInterface $visitor */
foreach ($this->visitors as $visitor) {
foreach ($visitor->getNodeTypes() as $nodeType) {
if (is_a($nodeClass, $nodeType, true)) {
$this->visitorsPerNodeClass[$nodeClass][] = $visitor;
continue 2;
}
}
}
}

return $this->visitorsPerNodeClass[$nodeClass];
}

private function traverseNode(Node $node): void
{
foreach ($node->getSubNodeNames() as $name) {
Expand All @@ -121,7 +50,7 @@ private function traverseNode(Node $node): void
continue;
}

$currentNodeVisitors = $this->getVisitorsForNode($subNode);
$currentNodeVisitors = $this->rectorRegistry->forNode($subNode);

foreach ($currentNodeVisitors as $currentNodeVisitor) {
$return = $this->rectorRunner->run($currentNodeVisitor, $subNode);
Expand Down Expand Up @@ -168,8 +97,7 @@ private function traverseArray(array $nodes): array
continue;
}

$traverseChildren = true;
$currentNodeVisitors = $this->getVisitorsForNode($node);
$currentNodeVisitors = $this->rectorRegistry->forNode($node);

foreach ($currentNodeVisitors as $currentNodeVisitor) {
$return = $this->rectorRunner->run($currentNodeVisitor, $node);
Expand Down Expand Up @@ -234,27 +162,4 @@ private function ensureReplacementReasonable(Node $old, Node $new): void
);
}
}

/**
* This must happen after $this->configuration is set after ProcessCommand::execute() is run, otherwise we get default false positives.
*
* This should be removed after https://github.com/rectorphp/rector/issues/5584 is resolved
*/
private function prepareNodeVisitors(): void
{
if ($this->areNodeVisitorsPrepared) {
return;
}

// filter out by PHP version
$this->visitors = $this->phpVersionedFilter->filter($this->rectors);

// filter out by composer package constraint
$this->visitors = $this->composerPackageConstraintFilter->filter($this->visitors);

// filter by configuration
$this->visitors = $this->configurationRuleFilter->filter($this->visitors);

$this->areNodeVisitorsPrepared = true;
}
}
5 changes: 0 additions & 5 deletions src/Testing/PHPUnit/AbstractRectorTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
use Rector\Exception\ShouldNotHappenException;
use Rector\NodeTypeResolver\DependencyInjection\PHPStanServicesFactory;
use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider;
use Rector\PhpParser\NodeTraverser\RectorNodeTraverser;
use Rector\Testing\Contract\RectorTestInterface;
use Rector\Testing\Fixture\FixtureFileFinder;
use Rector\Testing\Fixture\FixtureFileUpdater;
Expand Down Expand Up @@ -109,10 +108,6 @@ protected function setUp(): void
// no rules at all, e.g. in case of only post rector run, yields an empty array
$rectors = $rectorConfig->findByContract(RectorInterface::class);

/** @var RectorNodeTraverser $rectorNodeTraverser */
$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);
Expand Down
62 changes: 48 additions & 14 deletions tests/Application/RectorRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,72 @@

namespace Rector\Tests\Application;

use PhpParser\Node\Stmt\Class_;
use Rector\Application\RectorRegistry;
use Rector\Configuration\Option;
use Rector\Configuration\Parameter\SimpleParameterProvider;
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement;
use Rector\Tests\PhpParser\NodeTraverser\Class_\RuleUsingClassRector;
use Rector\Tests\PhpParser\NodeTraverser\ClassLike\RuleUsingClassLikeRector;
use Rector\Tests\PhpParser\NodeTraverser\Function_\RuleUsingFunctionRector;

final class RectorRegistryTest extends AbstractLazyTestCase
{
private RectorRegistry $rectorRegistry;

private RuleUsingFunctionRector $ruleUsingFunctionRector;

private RuleUsingClassRector $ruleUsingClassRector;

private RuleUsingClassLikeRector $ruleUsingClassLikeRector;

protected function setUp(): void
{
parent::setUp();

SimpleParameterProvider::setParameter(Option::SKIP, [
FifthElement::class => ['*/skipped_directory/*'],
]);

$this->rectorRegistry = $this->make(RectorRegistry::class);
$this->rectorRegistry->refreshRectors([new FifthElement()]);
$this->rectorRegistry->refreshRectors([]);

$this->ruleUsingFunctionRector = new RuleUsingFunctionRector();
$this->ruleUsingClassRector = new RuleUsingClassRector();
$this->ruleUsingClassLikeRector = new RuleUsingClassLikeRector();
}

public function testForNodeWhenNoRectorsAvailable(): void
{
$rectors = $this->rectorRegistry->forNode(new Class_('test'));

$this->assertSame([], $rectors);
}

protected function tearDown(): void
public function testForNodeWhenNoRectorsMatch(): void
{
SimpleParameterProvider::setParameter(Option::SKIP, []);
$this->rectorRegistry->refreshRectors([$this->ruleUsingFunctionRector]);

$rectors = $this->rectorRegistry->forNode(new Class_('test'));

$this->assertSame([], $rectors);
}

public function testForNodeWhenSomeRectorsMatch(): void
{
$this->rectorRegistry->refreshRectors([
$this->ruleUsingFunctionRector,
$this->ruleUsingClassRector,
]);

$rectors = $this->rectorRegistry->forNode(new Class_('test'));

$this->assertEquals([$this->ruleUsingClassRector], $rectors);
}

public function testGetRectors(): void
public function testForNodeWhenAllRectorsMatch(): void
{
$rectors = $this->rectorRegistry->getRectors();
$this->rectorRegistry->refreshRectors([
$this->ruleUsingClassRector,
$this->ruleUsingClassLikeRector,
]);

$rectors = $this->rectorRegistry->forNode(new Class_('test'));

$this->assertCount(1, $rectors);
$this->assertInstanceOf(FifthElement::class, $rectors[0]);
$this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $rectors);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use Rector\RuleDoc\RuleDefinition;

/**
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
* @see \Rector\Tests\Application\RectorRegistryTest
*/
final class RuleUsingClassLikeRector extends AbstractRector
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use Rector\RuleDoc\RuleDefinition;

/**
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
* @see \Rector\Tests\Application\RectorRegistryTest
*/
final class RuleUsingClassRector extends AbstractRector
{
Expand Down
Loading
Loading