Skip to content

Commit 359eb18

Browse files
TomasVotrubaactions-user
TomasVotruba
and
actions-user
authored
[trav] Move rule selection to RectorRegistry (#8619)
Co-authored-by: GitHub Action <actions@github.com>
1 parent b664405 commit 359eb18

11 files changed

Lines changed: 119 additions & 213 deletions

File tree

‎src/Application/FileProcessor.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ public function __construct(
4444
private RectorParser $rectorParser,
4545
private NodeScopeAndMetadataDecorator $nodeScopeAndMetadataDecorator,
4646
private UsedImportsResolver $usedImportsResolver,
47-
private RectorRegistry $rectorRegistry,
4847
) {
4948
}
5049

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

63-
$currentRectors = $this->rectorRegistry->getRectors();
64-
$this->rectorNodeTraverser->refreshPhpRectors($currentRectors);
65-
6662
do {
6763
$file->changeHasChanged(false);
6864

‎src/Application/RectorRegistry.php‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,92 @@
44

55
namespace Rector\Application;
66

7+
use PhpParser\Node;
8+
use Rector\Configuration\ConfigurationRuleFilter;
79
use Rector\Contract\Rector\RectorInterface;
10+
use Rector\VersionBonding\ComposerPackageConstraintFilter;
11+
use Rector\VersionBonding\PhpVersionedFilter;
812

913
/**
1014
* @see \Rector\Tests\Application\RectorRegistryTest
1115
*/
1216
final class RectorRegistry
1317
{
18+
private bool $areRectorsPrepared = false;
19+
20+
/**
21+
* @var RectorInterface[]
22+
*/
23+
private array $activeRectors = [];
24+
25+
/**
26+
* @var array<class-string<Node>, RectorInterface[]>
27+
*/
28+
private array $cache = [];
29+
1430
/**
1531
* @param RectorInterface[] $rectors
1632
*/
1733
public function __construct(
34+
private readonly PhpVersionedFilter $phpVersionedFilter,
35+
private readonly ComposerPackageConstraintFilter $composerPackageConstraintFilter,
36+
private readonly ConfigurationRuleFilter $configurationRuleFilter,
1837
private array $rectors,
1938
) {
2039
}
2140

2241
/**
2342
* @param RectorInterface[] $rectors
24-
* @api used in tests to update the active rules
2543
*
2644
* @internal Used only in Rector core, not supported outside. Might change any time.
2745
*/
2846
public function refreshRectors(array $rectors): void
2947
{
3048
$this->rectors = $rectors;
49+
$this->areRectorsPrepared = false;
50+
$this->cache = [];
3151
}
3252

3353
/**
34-
* @return array<RectorInterface>
54+
* @return RectorInterface[]
3555
*/
36-
public function getRectors(): array
56+
public function forNode(Node $node): array
3757
{
38-
return $this->rectors;
58+
$this->prepareRectors();
59+
60+
$nodeClass = $node::class;
61+
62+
if (! isset($this->cache[$nodeClass])) {
63+
$this->cache[$nodeClass] = [];
64+
65+
foreach ($this->activeRectors as $rector) {
66+
foreach ($rector->getNodeTypes() as $nodeType) {
67+
if (is_a($nodeClass, $nodeType, true)) {
68+
$this->cache[$nodeClass][] = $rector;
69+
continue 2;
70+
}
71+
}
72+
}
73+
}
74+
75+
return $this->cache[$nodeClass];
76+
}
77+
78+
/**
79+
* Filtering must happen lazily, after the configuration is set in ProcessCommand::execute(),
80+
* otherwise options like --only are not yet available and get ignored.
81+
*/
82+
private function prepareRectors(): void
83+
{
84+
if ($this->areRectorsPrepared) {
85+
return;
86+
}
87+
88+
$activeRectors = $this->phpVersionedFilter->filter($this->rectors);
89+
$activeRectors = $this->composerPackageConstraintFilter->filter($activeRectors);
90+
91+
$this->activeRectors = $this->configurationRuleFilter->filter($activeRectors);
92+
93+
$this->areRectorsPrepared = true;
3994
}
4095
}

‎src/Configuration/ConfigurationRuleFilter.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use Rector\VersionBonding\ValueObject\ComposerBoundRuleConfiguration;
1414

1515
/**
16+
* @todo merge to RectorRegistry, as it is only used there
17+
*
1618
* Modify available rector rules based on the configuration options
1719
* @see \Rector\Tests\Configuration\ConfigurationRuleFilterTest
1820
*/

‎src/PhpParser/NodeTraverser/RectorNodeTraverser.php‎

Lines changed: 4 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -9,41 +9,16 @@
99
use PhpParser\Node\Expr;
1010
use PhpParser\Node\Stmt;
1111
use PhpParser\NodeVisitor;
12-
use Rector\Configuration\ConfigurationRuleFilter;
13-
use Rector\Contract\Rector\RectorInterface;
12+
use Rector\Application\RectorRegistry;
1413
use Rector\Rector\RectorRunner;
15-
use Rector\VersionBonding\ComposerPackageConstraintFilter;
16-
use Rector\VersionBonding\PhpVersionedFilter;
17-
use Webmozart\Assert\Assert;
1814

19-
/**
20-
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
21-
*/
2215
final class RectorNodeTraverser
2316
{
24-
/**
25-
* @var RectorInterface[]
26-
*/
27-
private array $visitors = [];
28-
2917
private bool $stopTraversal;
3018

31-
private bool $areNodeVisitorsPrepared = false;
32-
33-
/**
34-
* @var array<class-string<Node>, RectorInterface[]>
35-
*/
36-
private array $visitorsPerNodeClass = [];
37-
38-
/**
39-
* @param RectorInterface[] $rectors
40-
*/
4119
public function __construct(
42-
private array $rectors,
43-
private readonly PhpVersionedFilter $phpVersionedFilter,
44-
private readonly ComposerPackageConstraintFilter $composerPackageConstraintFilter,
45-
private readonly ConfigurationRuleFilter $configurationRuleFilter,
4620
private readonly RectorRunner $rectorRunner,
21+
private readonly RectorRegistry $rectorRegistry,
4722
) {
4823
}
4924

@@ -53,57 +28,11 @@ public function __construct(
5328
*/
5429
public function traverse(array $nodes): array
5530
{
56-
$this->prepareNodeVisitors();
57-
5831
$this->stopTraversal = false;
5932

6033
return $this->traverseArray($nodes);
6134
}
6235

63-
/**
64-
* @param RectorInterface[] $rectors
65-
*
66-
* @internal Used only in Rector core, not supported outside. Might change any time.
67-
*/
68-
public function refreshPhpRectors(array $rectors): void
69-
{
70-
Assert::allIsInstanceOf($rectors, RectorInterface::class);
71-
72-
$this->rectors = $rectors;
73-
$this->visitors = [];
74-
$this->visitorsPerNodeClass = [];
75-
76-
$this->areNodeVisitorsPrepared = false;
77-
78-
$this->prepareNodeVisitors();
79-
}
80-
81-
/**
82-
* @return RectorInterface[]
83-
*
84-
* @api used in tests
85-
*/
86-
public function getVisitorsForNode(Node $node): array
87-
{
88-
$nodeClass = $node::class;
89-
90-
if (! isset($this->visitorsPerNodeClass[$nodeClass])) {
91-
$this->visitorsPerNodeClass[$nodeClass] = [];
92-
93-
/** @var RectorInterface $visitor */
94-
foreach ($this->visitors as $visitor) {
95-
foreach ($visitor->getNodeTypes() as $nodeType) {
96-
if (is_a($nodeClass, $nodeType, true)) {
97-
$this->visitorsPerNodeClass[$nodeClass][] = $visitor;
98-
continue 2;
99-
}
100-
}
101-
}
102-
}
103-
104-
return $this->visitorsPerNodeClass[$nodeClass];
105-
}
106-
10736
private function traverseNode(Node $node): void
10837
{
10938
foreach ($node->getSubNodeNames() as $name) {
@@ -121,7 +50,7 @@ private function traverseNode(Node $node): void
12150
continue;
12251
}
12352

124-
$currentNodeVisitors = $this->getVisitorsForNode($subNode);
53+
$currentNodeVisitors = $this->rectorRegistry->forNode($subNode);
12554

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

171-
$traverseChildren = true;
172-
$currentNodeVisitors = $this->getVisitorsForNode($node);
100+
$currentNodeVisitors = $this->rectorRegistry->forNode($node);
173101

174102
foreach ($currentNodeVisitors as $currentNodeVisitor) {
175103
$return = $this->rectorRunner->run($currentNodeVisitor, $node);
@@ -234,27 +162,4 @@ private function ensureReplacementReasonable(Node $old, Node $new): void
234162
);
235163
}
236164
}
237-
238-
/**
239-
* This must happen after $this->configuration is set after ProcessCommand::execute() is run, otherwise we get default false positives.
240-
*
241-
* This should be removed after https://github.com/rectorphp/rector/issues/5584 is resolved
242-
*/
243-
private function prepareNodeVisitors(): void
244-
{
245-
if ($this->areNodeVisitorsPrepared) {
246-
return;
247-
}
248-
249-
// filter out by PHP version
250-
$this->visitors = $this->phpVersionedFilter->filter($this->rectors);
251-
252-
// filter out by composer package constraint
253-
$this->visitors = $this->composerPackageConstraintFilter->filter($this->visitors);
254-
255-
// filter by configuration
256-
$this->visitors = $this->configurationRuleFilter->filter($this->visitors);
257-
258-
$this->areNodeVisitorsPrepared = true;
259-
}
260165
}

‎src/Testing/PHPUnit/AbstractRectorTestCase.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
use Rector\Exception\ShouldNotHappenException;
2222
use Rector\NodeTypeResolver\DependencyInjection\PHPStanServicesFactory;
2323
use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider;
24-
use Rector\PhpParser\NodeTraverser\RectorNodeTraverser;
2524
use Rector\Testing\Contract\RectorTestInterface;
2625
use Rector\Testing\Fixture\FixtureFileFinder;
2726
use Rector\Testing\Fixture\FixtureFileUpdater;
@@ -109,10 +108,6 @@ protected function setUp(): void
109108
// no rules at all, e.g. in case of only post rector run, yields an empty array
110109
$rectors = $rectorConfig->findByContract(RectorInterface::class);
111110

112-
/** @var RectorNodeTraverser $rectorNodeTraverser */
113-
$rectorNodeTraverser = $rectorConfig->make(RectorNodeTraverser::class);
114-
$rectorNodeTraverser->refreshPhpRectors($rectors);
115-
116111
// keep the shared registry in sync with the current test's rules
117112
/** @var RectorRegistry $rectorRegistry */
118113
$rectorRegistry = $rectorConfig->make(RectorRegistry::class);

‎tests/Application/RectorRegistryTest.php‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,72 @@
44

55
namespace Rector\Tests\Application;
66

7+
use PhpParser\Node\Stmt\Class_;
78
use Rector\Application\RectorRegistry;
8-
use Rector\Configuration\Option;
9-
use Rector\Configuration\Parameter\SimpleParameterProvider;
109
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
11-
use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement;
10+
use Rector\Tests\PhpParser\NodeTraverser\Class_\RuleUsingClassRector;
11+
use Rector\Tests\PhpParser\NodeTraverser\ClassLike\RuleUsingClassLikeRector;
12+
use Rector\Tests\PhpParser\NodeTraverser\Function_\RuleUsingFunctionRector;
1213

1314
final class RectorRegistryTest extends AbstractLazyTestCase
1415
{
1516
private RectorRegistry $rectorRegistry;
1617

18+
private RuleUsingFunctionRector $ruleUsingFunctionRector;
19+
20+
private RuleUsingClassRector $ruleUsingClassRector;
21+
22+
private RuleUsingClassLikeRector $ruleUsingClassLikeRector;
23+
1724
protected function setUp(): void
1825
{
1926
parent::setUp();
2027

21-
SimpleParameterProvider::setParameter(Option::SKIP, [
22-
FifthElement::class => ['*/skipped_directory/*'],
23-
]);
24-
2528
$this->rectorRegistry = $this->make(RectorRegistry::class);
26-
$this->rectorRegistry->refreshRectors([new FifthElement()]);
29+
$this->rectorRegistry->refreshRectors([]);
30+
31+
$this->ruleUsingFunctionRector = new RuleUsingFunctionRector();
32+
$this->ruleUsingClassRector = new RuleUsingClassRector();
33+
$this->ruleUsingClassLikeRector = new RuleUsingClassLikeRector();
34+
}
35+
36+
public function testForNodeWhenNoRectorsAvailable(): void
37+
{
38+
$rectors = $this->rectorRegistry->forNode(new Class_('test'));
39+
40+
$this->assertSame([], $rectors);
2741
}
2842

29-
protected function tearDown(): void
43+
public function testForNodeWhenNoRectorsMatch(): void
3044
{
31-
SimpleParameterProvider::setParameter(Option::SKIP, []);
45+
$this->rectorRegistry->refreshRectors([$this->ruleUsingFunctionRector]);
46+
47+
$rectors = $this->rectorRegistry->forNode(new Class_('test'));
48+
49+
$this->assertSame([], $rectors);
50+
}
51+
52+
public function testForNodeWhenSomeRectorsMatch(): void
53+
{
54+
$this->rectorRegistry->refreshRectors([
55+
$this->ruleUsingFunctionRector,
56+
$this->ruleUsingClassRector,
57+
]);
58+
59+
$rectors = $this->rectorRegistry->forNode(new Class_('test'));
60+
61+
$this->assertEquals([$this->ruleUsingClassRector], $rectors);
3262
}
3363

34-
public function testGetRectors(): void
64+
public function testForNodeWhenAllRectorsMatch(): void
3565
{
36-
$rectors = $this->rectorRegistry->getRectors();
66+
$this->rectorRegistry->refreshRectors([
67+
$this->ruleUsingClassRector,
68+
$this->ruleUsingClassLikeRector,
69+
]);
70+
71+
$rectors = $this->rectorRegistry->forNode(new Class_('test'));
3772

38-
$this->assertCount(1, $rectors);
39-
$this->assertInstanceOf(FifthElement::class, $rectors[0]);
73+
$this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $rectors);
4074
}
4175
}

‎tests/PhpParser/NodeTraverser/ClassLike/RuleUsingClassLikeRector.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use Rector\RuleDoc\RuleDefinition;
1212

1313
/**
14-
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
14+
* @see \Rector\Tests\Application\RectorRegistryTest
1515
*/
1616
final class RuleUsingClassLikeRector extends AbstractRector
1717
{

‎tests/PhpParser/NodeTraverser/Class_/RuleUsingClassRector.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use Rector\RuleDoc\RuleDefinition;
1212

1313
/**
14-
* @see \Rector\Tests\PhpParser\NodeTraverser\RectorNodeTraverserTest
14+
* @see \Rector\Tests\Application\RectorRegistryTest
1515
*/
1616
final class RuleUsingClassRector extends AbstractRector
1717
{

0 commit comments

Comments
 (0)