Skip to content

Commit fee72fa

Browse files
authored
[CodeQuality] Skip SingularSwitchToIfRector on nested break or continue in case body (#8500)
1 parent 23c8825 commit fee72fa

3 files changed

Lines changed: 101 additions & 0 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\Switch_\SingularSwitchToIfRector\Fixture;
4+
5+
final class ClosureWithBreak
6+
{
7+
public function run($value, array $items)
8+
{
9+
switch ($value) {
10+
case 100:
11+
$callback = function () use ($items) {
12+
foreach ($items as $item) {
13+
break;
14+
}
15+
};
16+
break;
17+
}
18+
}
19+
}
20+
21+
?>
22+
-----
23+
<?php
24+
25+
namespace Rector\Tests\CodeQuality\Rector\Switch_\SingularSwitchToIfRector\Fixture;
26+
27+
final class ClosureWithBreak
28+
{
29+
public function run($value, array $items)
30+
{
31+
if ($value === 100) {
32+
$callback = function () use ($items) {
33+
foreach ($items as $item) {
34+
break;
35+
}
36+
};
37+
}
38+
}
39+
}
40+
41+
?>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodeQuality\Rector\Switch_\SingularSwitchToIfRector\Fixture;
4+
5+
final class SkipNestedBreak
6+
{
7+
public function run($a, $b, $else, $value)
8+
{
9+
switch ($a) {
10+
case $b:
11+
$something = $else;
12+
if (! $something) {
13+
break;
14+
}
15+
$another = $value;
16+
break;
17+
}
18+
}
19+
}
20+
21+
?>

‎rules/CodeQuality/Rector/Switch_/SingularSwitchToIfRector.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@
77
use PhpParser\Node;
88
use PhpParser\Node\Expr;
99
use PhpParser\Node\Expr\BinaryOp\Identical;
10+
use PhpParser\Node\FunctionLike;
1011
use PhpParser\Node\Stmt;
1112
use PhpParser\Node\Stmt\Break_;
13+
use PhpParser\Node\Stmt\Class_;
14+
use PhpParser\Node\Stmt\Continue_;
1215
use PhpParser\Node\Stmt\If_;
1316
use PhpParser\Node\Stmt\Switch_;
17+
use PhpParser\NodeVisitor;
1418
use Rector\Rector\AbstractRector;
1519
use Rector\Renaming\NodeManipulator\SwitchManipulator;
1620
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -85,6 +89,11 @@ public function refactor(Node $node): array|If_|null
8589

8690
$onlyCase = $node->cases[0];
8791

92+
// nested break/continue would lose the switch to target and cause a fatal error
93+
if ($this->hasNestedBreakOrContinue($onlyCase->stmts)) {
94+
return null;
95+
}
96+
8897
// only default → basically unwrap
8998
if (! $onlyCase->cond instanceof Expr) {
9099
// remove default clause because it cause syntax error
@@ -96,4 +105,34 @@ public function refactor(Node $node): array|If_|null
96105

97106
return $if;
98107
}
108+
109+
/**
110+
* @param Stmt[] $stmts
111+
*/
112+
private function hasNestedBreakOrContinue(array $stmts): bool
113+
{
114+
$hasNested = false;
115+
116+
foreach ($stmts as $stmt) {
117+
// top level break is removed by SwitchManipulator
118+
if ($stmt instanceof Break_) {
119+
continue;
120+
}
121+
122+
$this->traverseNodesWithCallable($stmt, static function (Node $subNode) use (&$hasNested): ?int {
123+
if ($subNode instanceof Class_ || $subNode instanceof FunctionLike) {
124+
return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
125+
}
126+
127+
if ($subNode instanceof Break_ || $subNode instanceof Continue_) {
128+
$hasNested = true;
129+
return NodeVisitor::STOP_TRAVERSAL;
130+
}
131+
132+
return null;
133+
});
134+
}
135+
136+
return $hasNested;
137+
}
99138
}

0 commit comments

Comments
 (0)