Skip to content

Commit 8bcd555

Browse files
committed
avoid traversing by rules no longer valid in case of change type
1 parent cf9f536 commit 8bcd555

File tree

5 files changed

+30
-32
lines changed

5 files changed

+30
-32
lines changed

rules/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public function refactor(Node $node): ?Node
142142
return null;
143143
}
144144

145-
/** @var Expression<Assign|AssignOp> $previousStmt */
145+
/** @var Assign|AssignOp $assign */
146146
$assign = $previousStmt->expr;
147147

148148
return $this->processSimplifyUselessVariable($node, $stmt, $assign, $key);

rules/CodeQuality/Rector/If_/SimplifyIfElseToTernaryRector.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public function getNodeTypes(): array
8181
/**
8282
* @param If_ $node
8383
*/
84-
public function refactor(Node $node): ?Node
84+
public function refactor(Node $node): ?Expression
8585
{
8686
if (! $node->else instanceof Else_) {
8787
return null;

src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,6 @@
1313

1414
abstract class AbstractImmutableNodeTraverser implements NodeTraverserInterface
1515
{
16-
/**
17-
* @deprecated Use NodeVisitor::DONT_TRAVERSE_CHILDREN instead.
18-
*/
19-
public const DONT_TRAVERSE_CHILDREN = NodeVisitor::DONT_TRAVERSE_CHILDREN;
20-
21-
/**
22-
* @deprecated Use NodeVisitor::STOP_TRAVERSAL instead.
23-
*/
24-
public const STOP_TRAVERSAL = NodeVisitor::STOP_TRAVERSAL;
25-
26-
/**
27-
* @deprecated Use NodeVisitor::REMOVE_NODE instead.
28-
*/
29-
public const REMOVE_NODE = NodeVisitor::REMOVE_NODE;
30-
31-
/**
32-
* @deprecated Use NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN instead.
33-
*/
34-
public const DONT_TRAVERSE_CURRENT_AND_CHILDREN = NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
35-
3616
/**
3717
* @var list<NodeVisitor> Visitors
3818
*/
@@ -131,13 +111,22 @@ protected function traverseNode(Node $node): void
131111
$traverseChildren = true;
132112
$visitorIndex = -1;
133113
$currentNodeVisitors = $this->getVisitorsForNode($subNode);
114+
134115
foreach ($currentNodeVisitors as $visitorIndex => $visitor) {
135116
$return = $visitor->enterNode($subNode);
136117
if ($return !== null) {
137118
if ($return instanceof Node) {
119+
120+
$originalNodeClass = $subNode::class;
121+
138122
$this->ensureReplacementReasonable($subNode, $return);
139123
$subNode = $return;
140124
$node->{$name} = $return;
125+
126+
if ($originalNodeClass !== $return::class) {
127+
$traverseChildren = false;
128+
break;
129+
}
141130
} elseif ($return === NodeVisitor::DONT_TRAVERSE_CHILDREN) {
142131
$traverseChildren = false;
143132
} elseif ($return === NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN) {
@@ -210,12 +199,21 @@ protected function traverseArray(array $nodes): array
210199
$traverseChildren = true;
211200
$visitorIndex = -1;
212201
$currentNodeVisitors = $this->getVisitorsForNode($node);
202+
213203
foreach ($currentNodeVisitors as $visitorIndex => $visitor) {
214204
$return = $visitor->enterNode($node);
215205
if ($return !== null) {
216206
if ($return instanceof Node) {
217207
$this->ensureReplacementReasonable($node, $return);
208+
209+
// hos node type changed?
210+
$originalNodeCLass = $node::class;
218211
$nodes[$i] = $node = $return;
212+
213+
if ($originalNodeCLass !== $return::class) {
214+
$traverseChildren = false;
215+
break;
216+
}
219217
} elseif (\is_array($return)) {
220218
$doNodes[] = [$i, $return];
221219
continue 2;

src/Rector/AbstractRector.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,11 @@ public function beforeTraverse(array $nodes): ?array
128128
*/
129129
final public function enterNode(Node $node): int|Node|null
130130
{
131-
if (! $this->isMatchingNodeType($node)) {
132-
return null;
133-
}
131+
// if previous Rector rule changed the node type, we have to skip it here
132+
// as here we expect still the original type
133+
// if (! $this->isMatchingNodeType($node)) {
134+
// return null;
135+
//}
134136

135137
if (is_a($this, HTMLAverseRectorInterface::class, true) && $this->file->containsHTML()) {
136138
return null;

tests/Issues/SimplifyVariableIfElseTernary/config/configured_rule.php

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88
use Rector\Config\RectorConfig;
99

1010
return RectorConfig::configure()
11-
->withRules(
12-
[
13-
SimplifyIfElseToTernaryRector::class,
14-
SimplifyUselessVariableRector::class,
15-
CompleteDynamicPropertiesRector::class,
16-
]
17-
);
11+
->withRules([
12+
SimplifyIfElseToTernaryRector::class,
13+
SimplifyUselessVariableRector::class,
14+
CompleteDynamicPropertiesRector::class,
15+
]);

0 commit comments

Comments
 (0)