Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Switch_\SingularSwitchToIfRector\Fixture;

final class ClosureWithBreak
{
public function run($value, array $items)
{
switch ($value) {
case 100:
$callback = function () use ($items) {
foreach ($items as $item) {
break;
}
};
break;
}
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\Switch_\SingularSwitchToIfRector\Fixture;

final class ClosureWithBreak
{
public function run($value, array $items)
{
if ($value === 100) {
$callback = function () use ($items) {
foreach ($items as $item) {
break;
}
};
}
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Switch_\SingularSwitchToIfRector\Fixture;

final class SkipNestedBreak
{
public function run($a, $b, $else, $value)
{
switch ($a) {
case $b:
$something = $else;
if (! $something) {
break;
}
$another = $value;
break;
}
}
}

?>
39 changes: 39 additions & 0 deletions rules/CodeQuality/Rector/Switch_/SingularSwitchToIfRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@
use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Break_;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Switch_;
use PhpParser\NodeVisitor;
use Rector\Rector\AbstractRector;
use Rector\Renaming\NodeManipulator\SwitchManipulator;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
Expand Down Expand Up @@ -85,6 +89,11 @@ public function refactor(Node $node): array|If_|null

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

// nested break/continue would lose the switch to target and cause a fatal error
if ($this->hasNestedBreakOrContinue($onlyCase->stmts)) {
return null;
}

// only default → basically unwrap
if (! $onlyCase->cond instanceof Expr) {
// remove default clause because it cause syntax error
Expand All @@ -96,4 +105,34 @@ public function refactor(Node $node): array|If_|null

return $if;
}

/**
* @param Stmt[] $stmts
*/
private function hasNestedBreakOrContinue(array $stmts): bool
{
$hasNested = false;

foreach ($stmts as $stmt) {
// top level break is removed by SwitchManipulator
if ($stmt instanceof Break_) {
continue;
}

$this->traverseNodesWithCallable($stmt, static function (Node $subNode) use (&$hasNested): ?int {
if ($subNode instanceof Class_ || $subNode instanceof FunctionLike) {
return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
}

if ($subNode instanceof Break_ || $subNode instanceof Continue_) {
$hasNested = true;
return NodeVisitor::STOP_TRAVERSAL;
}

return null;
});
}

return $hasNested;
}
}
Loading