Skip to content

Commit e3441cd

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 d2326bf commit e3441cd

6 files changed

Lines changed: 160 additions & 3 deletions

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
}

‎tests/Issues/RepeatedThreeTimes/Source/NarrowParamUnionDocblockRector.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,21 @@
1111
use PHPStan\Type\ObjectType;
1212
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
1313
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
14+
use Rector\Contract\Rector\OrderedRectorInterface;
1415
use Rector\Rector\AbstractRector;
1516
use Rector\RuleDoc\RuleDefinition;
1617
use Rector\StaticTypeMapper\StaticTypeMapper;
1718

1819
/**
1920
* @param array|SomeCollection $items → @param SomeCollection $items
2021
*/
21-
final class NarrowParamUnionDocblockRector extends AbstractRector
22+
final class NarrowParamUnionDocblockRector extends AbstractRector implements OrderedRectorInterface
2223
{
24+
public function precededRule(): string
25+
{
26+
return NarrowArrayCollectionDocblockRector::class;
27+
}
28+
2329
public function __construct(
2430
private readonly PhpDocInfoFactory $phpDocInfoFactory,
2531
private readonly DocBlockUpdater $docBlockUpdater,

‎tests/Issues/RepeatedThreeTimes/Source/ParamTypeFromDocblockRector.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,20 @@
99
use PhpParser\Node\Stmt\ClassMethod;
1010
use PHPStan\Type\ObjectType;
1111
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
12+
use Rector\Contract\Rector\OrderedRectorInterface;
1213
use Rector\Rector\AbstractRector;
1314
use Rector\RuleDoc\RuleDefinition;
1415

1516
/**
1617
* @param SomeCollection $items → native SomeCollection $items param type
1718
*/
18-
final class ParamTypeFromDocblockRector extends AbstractRector
19+
final class ParamTypeFromDocblockRector extends AbstractRector implements OrderedRectorInterface
1920
{
21+
public function precededRule(): string
22+
{
23+
return NarrowParamUnionDocblockRector::class;
24+
}
25+
2026
public function __construct(
2127
private readonly PhpDocInfoFactory $phpDocInfoFactory
2228
) {
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)