Skip to content

Commit acbd2db

Browse files
TomasVotrubaTomasVotruba
committed
wip
1 parent 02fdcde commit acbd2db

2 files changed

Lines changed: 134 additions & 51 deletions

File tree

‎rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
use PHPStan\Reflection\ClassReflection;
2222
use PHPStan\Type\MixedType;
2323
use PHPStan\Type\ObjectType;
24+
use Rector\CodeQuality\Rector\If_\ExplicitNullableObjectInstanceRector;
25+
use Rector\Contract\Rector\OrderedRectorInterface;
2426
use Rector\NodeManipulator\IfManipulator;
2527
use Rector\NodeTypeResolver\Node\AttributeKey;
2628
use Rector\Rector\AbstractRector;
@@ -31,7 +33,7 @@
3133
/**
3234
* @see \Rector\Tests\DeadCode\Rector\If_\RemoveDeadInstanceOfRector\RemoveDeadInstanceOfRectorTest
3335
*/
34-
final class RemoveDeadInstanceOfRector extends AbstractRector
36+
final class RemoveDeadInstanceOfRector extends AbstractRector implements OrderedRectorInterface
3537
{
3638
public function __construct(
3739
private readonly IfManipulator $ifManipulator,
@@ -72,6 +74,11 @@ public function getNodeTypes(): array
7274
return [If_::class];
7375
}
7476

77+
public function precededRule(): string
78+
{
79+
return ExplicitNullableObjectInstanceRector::class;
80+
}
81+
7582
/**
7683
* @param If_ $node
7784
* @return Stmt[]|null|NodeVisitor::REMOVE_NODE|If_

‎src/PhpParser/NodeTraverser/RectorNodeTraverser.php‎

Lines changed: 126 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,24 @@
1010
use PhpParser\Node\Stmt;
1111
use PhpParser\NodeVisitor;
1212
use Rector\Application\RectorRegistry;
13+
use Rector\Exception\ShouldNotHappenException;
14+
use Rector\PhpParser\Printer\BetterStandardPrinter;
1315
use Rector\Rector\RectorRunner;
1416
use Rector\ValueObject\Application\File;
1517

1618
final class RectorNodeTraverser
1719
{
20+
/**
21+
* Safety cap on local re-visits of a single node, so a rule that never settles cannot loop forever.
22+
*/
23+
private const int MAX_REVISITS = 50;
24+
1825
private bool $stopTraversal;
1926

2027
public function __construct(
2128
private readonly RectorRunner $rectorRunner,
2229
private readonly RectorRegistry $rectorRegistry,
30+
private readonly BetterStandardPrinter $betterStandardPrinter,
2331
) {
2432
}
2533

@@ -51,31 +59,8 @@ private function traverseNode(Node $node, File $file): void
5159
continue;
5260
}
5361

54-
$currentNodeVisitors = $this->rectorRegistry->forNodeAndFile($subNode, $file);
55-
56-
foreach ($currentNodeVisitors as $currentNodeVisitor) {
57-
$return = $this->rectorRunner->run($currentNodeVisitor, $subNode);
58-
if ($return === null) {
59-
continue;
60-
}
61-
62-
if ($return instanceof Node) {
63-
$originalSubNodeClass = $subNode::class;
64-
65-
$this->ensureReplacementReasonable($subNode, $return);
66-
$subNode = $return;
67-
$node->{$name} = $return;
68-
69-
if ($originalSubNodeClass !== $subNode::class) {
70-
// stop traversing as node type changed and visitors won't work
71-
continue 2;
72-
}
73-
} else {
74-
throw new LogicException('RectorRunner::run() returned invalid value of type ' . gettype($return));
75-
}
76-
}
77-
78-
$this->traverseNode($subNode, $file);
62+
// a single sub-node slot cannot hold a list or be removed
63+
$node->{$name} = $this->refactorNode($subNode, $file, false);
7964

8065
if ($this->stopTraversal) {
8166
break;
@@ -99,33 +84,16 @@ private function traverseArray(array $nodes, File $file): array
9984
continue;
10085
}
10186

102-
$currentNodeVisitors = $this->rectorRegistry->forNodeAndFile($node, $file);
103-
104-
foreach ($currentNodeVisitors as $currentNodeVisitor) {
105-
$return = $this->rectorRunner->run($currentNodeVisitor, $node);
106-
if ($return !== null) {
107-
if ($return instanceof Node) {
108-
$originalNodeNodeClass = $node::class;
109-
$this->ensureReplacementReasonable($node, $return);
110-
$nodes[$i] = $node = $return;
111-
112-
if ($originalNodeNodeClass !== $return::class) {
113-
// stop traversing as node type changed and visitors won't work
114-
continue 2;
115-
}
116-
} elseif (\is_array($return)) {
117-
$doNodes[] = [$i, $return];
118-
continue 2;
119-
} elseif ($return === NodeVisitor::REMOVE_NODE) {
120-
$doNodes[] = [$i, []];
121-
continue 2;
122-
} else {
123-
throw new LogicException('RectorRunner::run() returned invalid value of type ' . gettype($return));
124-
}
125-
}
87+
$result = $this->refactorNode($node, $file, true);
88+
89+
if (\is_array($result)) {
90+
$doNodes[] = [$i, $result];
91+
} elseif ($result === NodeVisitor::REMOVE_NODE) {
92+
$doNodes[] = [$i, []];
93+
} else {
94+
$nodes[$i] = $result;
12695
}
12796

128-
$this->traverseNode($node, $file);
12997
if ($this->stopTraversal) {
13098
break;
13199
}
@@ -140,6 +108,114 @@ private function traverseArray(array $nodes, File $file): array
140108
return $nodes;
141109
}
142110

111+
/**
112+
* Applies the node's rules and traverses into its children, then re-applies its rules while a
113+
* descendant keeps changing - so a rule that depends on a change made deeper in the tree (ordered
114+
* via OrderedRectorInterface, or a plain multi-pass chain) converges within a single file pass.
115+
*
116+
* Termination is structural: the loop stops once a round reports no change, or reports a change
117+
* that leaves the node's printed form identical (a rule that keeps returning the same result).
118+
*
119+
* @param bool $allowListReturn whether an array / REMOVE_NODE return is valid here (only for a
120+
* node that lives inside a list, not a single sub-node slot)
121+
* @return NodeVisitor::REMOVE_NODE|Node|Node[]
122+
*/
123+
private function refactorNode(Node $node, File $file, bool $allowListReturn): int|Node|array
124+
{
125+
$lastPrint = null;
126+
$revisits = 0;
127+
128+
while (true) {
129+
$changeCountBefore = $this->changeCount($file);
130+
$return = $this->applyRectors($node, $file);
131+
132+
if (\is_array($return) || $return === NodeVisitor::REMOVE_NODE) {
133+
if (! $allowListReturn) {
134+
throw new LogicException('RectorRunner::run() returned invalid value of type ' . gettype($return));
135+
}
136+
137+
// a list / removal replaces the node in its parent list; its children are not traversed
138+
return $return;
139+
}
140+
141+
$typeChanged = $return::class !== $node::class;
142+
$node = $return;
143+
144+
if ($typeChanged) {
145+
// stop traversing as node type changed and visitors won't work
146+
return $node;
147+
}
148+
149+
$this->traverseNode($node, $file);
150+
if ($this->stopTraversal) {
151+
return $node;
152+
}
153+
154+
// no rule (here or deeper) reported a change this round: the subtree is stable
155+
if ($this->changeCount($file) === $changeCountBefore) {
156+
return $node;
157+
}
158+
159+
// a change was reported; keep going only while it actually reshapes the node
160+
$printNow = $this->betterStandardPrinter->print($node);
161+
if ($printNow === $lastPrint) {
162+
return $node;
163+
}
164+
165+
$lastPrint = $printNow;
166+
167+
if (++$revisits >= self::MAX_REVISITS) {
168+
throw new ShouldNotHappenException(
169+
sprintf('Node "%s" did not converge after %d re-visits', $node->getType(), $revisits)
170+
);
171+
}
172+
}
173+
}
174+
175+
/**
176+
* Runs every rule subscribed to this node, in order. A rule returning a same-type node keeps the
177+
* remaining rules running on the new node; a type change, an array or REMOVE_NODE stops the chain
178+
* and is handed back to the caller.
179+
*
180+
* @return NodeVisitor::REMOVE_NODE|Node|Node[]
181+
*/
182+
private function applyRectors(Node $node, File $file): int|Node|array
183+
{
184+
$currentNodeVisitors = $this->rectorRegistry->forNodeAndFile($node, $file);
185+
186+
foreach ($currentNodeVisitors as $currentNodeVisitor) {
187+
$return = $this->rectorRunner->run($currentNodeVisitor, $node);
188+
if ($return === null) {
189+
continue;
190+
}
191+
192+
if ($return instanceof Node) {
193+
$originalNodeClass = $node::class;
194+
$this->ensureReplacementReasonable($node, $return);
195+
$node = $return;
196+
197+
if ($originalNodeClass !== $node::class) {
198+
return $node;
199+
}
200+
201+
continue;
202+
}
203+
204+
if (\is_array($return) || $return === NodeVisitor::REMOVE_NODE) {
205+
return $return;
206+
}
207+
208+
throw new LogicException('RectorRunner::run() returned invalid value of type ' . gettype($return));
209+
}
210+
211+
return $node;
212+
}
213+
214+
private function changeCount(File $file): int
215+
{
216+
return count($file->getRectorWithLineChanges());
217+
}
218+
143219
private function ensureReplacementReasonable(Node $old, Node $new): void
144220
{
145221
if ($old instanceof Stmt) {

0 commit comments

Comments
 (0)