Skip to content

Commit 0d3fa42

Browse files
committed
Add OrderedRectorInterface to order dependent rules in a single pass
A rule implementing OrderedRectorInterface declares precededRule(), the rule that must run before it. RectorRegistry topologically sorts active rules so each dependent rule runs after its predecessor within one file pass, instead of relying on repeated FileProcessor passes to converge.
1 parent 70e25dc commit 0d3fa42

4 files changed

Lines changed: 146 additions & 1 deletion

File tree

‎src/Application/RectorRegistry.php‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66

77
use PhpParser\Node;
88
use Rector\Configuration\ConfigurationRuleFilter;
9+
use Rector\Contract\Rector\OrderedRectorInterface;
910
use Rector\Contract\Rector\RectorInterface;
11+
use Rector\Exception\ShouldNotHappenException;
1012
use Rector\Skipper\Skipper\Skipper;
1113
use Rector\ValueObject\Application\File;
1214
use Rector\VersionBonding\ComposerPackageConstraintFilter;
@@ -112,9 +114,77 @@ private function prepareRectors(): void
112114

113115
$activeRectors = $this->phpVersionedFilter->filter($this->rectors);
114116
$activeRectors = $this->composerPackageConstraintFilter->filter($activeRectors);
117+
$activeRectors = $this->configurationRuleFilter->filter($activeRectors);
115118

116-
$this->activeRectors = $this->configurationRuleFilter->filter($activeRectors);
119+
$this->activeRectors = $this->sortByPrecededRule($activeRectors);
117120

118121
$this->areRectorsPrepared = true;
119122
}
123+
124+
/**
125+
* Rules implementing OrderedRectorInterface are moved after the rule they depend on,
126+
* so a chain of dependent rules converges in a single file pass.
127+
*
128+
* @param RectorInterface[] $rectors
129+
* @return RectorInterface[]
130+
*/
131+
private function sortByPrecededRule(array $rectors): array
132+
{
133+
$rectorByClass = [];
134+
foreach ($rectors as $rector) {
135+
$rectorByClass[$rector::class] = $rector;
136+
}
137+
138+
$precededClassByClass = [];
139+
foreach ($rectors as $rector) {
140+
if (! $rector instanceof OrderedRectorInterface) {
141+
continue;
142+
}
143+
144+
$precededClass = $rector->precededRule();
145+
146+
// ignore a predecessor filtered out of the active set
147+
if (isset($rectorByClass[$precededClass])) {
148+
$precededClassByClass[$rector::class] = $precededClass;
149+
}
150+
}
151+
152+
if ($precededClassByClass === []) {
153+
return $rectors;
154+
}
155+
156+
$sortedRectors = [];
157+
$emittedClasses = [];
158+
$remainingRectors = $rectors;
159+
160+
while ($remainingRectors !== []) {
161+
$hasProgress = false;
162+
163+
foreach ($remainingRectors as $key => $rector) {
164+
$precededClass = $precededClassByClass[$rector::class] ?? null;
165+
if ($precededClass !== null && ! isset($emittedClasses[$precededClass])) {
166+
continue;
167+
}
168+
169+
$sortedRectors[] = $rector;
170+
$emittedClasses[$rector::class] = true;
171+
unset($remainingRectors[$key]);
172+
$hasProgress = true;
173+
}
174+
175+
if (! $hasProgress) {
176+
$remainingClasses = [];
177+
foreach ($remainingRectors as $rector) {
178+
$remainingClasses[] = $rector::class;
179+
}
180+
181+
throw new ShouldNotHappenException(sprintf(
182+
'Circular rule ordering detected among: %s',
183+
implode(', ', $remainingClasses)
184+
));
185+
}
186+
}
187+
188+
return $sortedRectors;
189+
}
120190
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Contract\Rector;
6+
7+
interface OrderedRectorInterface
8+
{
9+
/**
10+
* Rule that must run before this one, so dependent changes converge in a single file pass.
11+
*
12+
* @return class-string<RectorInterface>
13+
*/
14+
public function precededRule(): string;
15+
}

‎tests/Application/RectorRegistryTest.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PhpParser\Node\Stmt\Class_;
88
use Rector\Application\RectorRegistry;
99
use Rector\Testing\PHPUnit\AbstractLazyTestCase;
10+
use Rector\Tests\PhpParser\NodeTraverser\Class_\OrderedClassRector;
1011
use Rector\Tests\PhpParser\NodeTraverser\Class_\RuleUsingClassRector;
1112
use Rector\Tests\PhpParser\NodeTraverser\ClassLike\RuleUsingClassLikeRector;
1213
use Rector\Tests\PhpParser\NodeTraverser\Function_\RuleUsingFunctionRector;
@@ -72,4 +73,19 @@ public function testForNodeWhenAllRectorsMatch(): void
7273

7374
$this->assertEquals([$this->ruleUsingClassRector, $this->ruleUsingClassLikeRector], $rectors);
7475
}
76+
77+
public function testForNodeSortsByPrecededRule(): void
78+
{
79+
$orderedClassRector = new OrderedClassRector();
80+
81+
// registered before the rule it depends on
82+
$this->rectorRegistry->refreshRectors([
83+
$orderedClassRector,
84+
$this->ruleUsingClassRector,
85+
]);
86+
87+
$rectors = $this->rectorRegistry->forNode(new Class_('test'));
88+
89+
$this->assertEquals([$this->ruleUsingClassRector, $orderedClassRector], $rectors);
90+
}
7591
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\PhpParser\NodeTraverser\Class_;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Stmt\Class_;
9+
use Rector\Contract\Rector\OrderedRectorInterface;
10+
use Rector\Rector\AbstractRector;
11+
use Rector\RuleDoc\CodeSample\CodeSample;
12+
use Rector\RuleDoc\RuleDefinition;
13+
14+
/**
15+
* @see \Rector\Tests\Application\RectorRegistryTest
16+
*/
17+
final class OrderedClassRector extends AbstractRector implements OrderedRectorInterface
18+
{
19+
public function precededRule(): string
20+
{
21+
return RuleUsingClassRector::class;
22+
}
23+
24+
public function getRuleDefinition(): RuleDefinition
25+
{
26+
return new RuleDefinition('This rule runs after RuleUsingClassRector', [new CodeSample('', '')]);
27+
}
28+
29+
/**
30+
* @return array<class-string<Node>>
31+
*/
32+
public function getNodeTypes(): array
33+
{
34+
return [Class_::class];
35+
}
36+
37+
/**
38+
* @param Class_ $node
39+
*/
40+
public function refactor(Node $node): Node
41+
{
42+
return $node;
43+
}
44+
}

0 commit comments

Comments
 (0)