Skip to content

Commit

Permalink
[EarlyReturn] Improve RemoveAlwaysElseRector to handle multiple ElseI…
Browse files Browse the repository at this point in the history
…fs (#8178) (#5521)
  • Loading branch information
pkvach committed Jan 30, 2024
1 parent 58ef131 commit 3c51cd8
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 23 deletions.
@@ -0,0 +1,51 @@
<?php

namespace Rector\Tests\EarlyReturn\Rector\If_\RemoveAlwaysElseRector\Fixture;

class MultipleElseifsReturn
{
public function run($a)
{
if ($a == 1) {
return 1;
} elseif ($a == 2) {
return 2;
} elseif ($a == 3) {
return 3;
} elseif ($a == 4) {
return 4;
} elseif ($a == 5) {
return 5;
}
return 'more';
}
}
?>
-----
<?php

namespace Rector\Tests\EarlyReturn\Rector\If_\RemoveAlwaysElseRector\Fixture;

class MultipleElseifsReturn
{
public function run($a)
{
if ($a == 1) {
return 1;
}
if ($a == 2) {
return 2;
}
if ($a == 3) {
return 3;
}
if ($a == 4) {
return 4;
}
if ($a == 5) {
return 5;
}
return 'more';
}
}
?>
@@ -0,0 +1,51 @@
<?php

namespace Rector\Tests\EarlyReturn\Rector\If_\RemoveAlwaysElseRector\Fixture;

class MultipleElseifsWithNonTerminatingElseif
{
public function run($a)
{
if ($a == 1) {
return 1;
} elseif ($a == 2) {
return 2;
} elseif ($a == 3) {
echo 'Not returning';
} elseif ($a == 4) {
return 4;
} elseif ($a == 5) {
return 5;
}
return 'more';
}
}
?>
-----
<?php

namespace Rector\Tests\EarlyReturn\Rector\If_\RemoveAlwaysElseRector\Fixture;

class MultipleElseifsWithNonTerminatingElseif
{
public function run($a)
{
if ($a == 1) {
return 1;
}
if ($a == 2) {
return 2;
}
if ($a == 3) {
echo 'Not returning';
}
elseif ($a == 4) {
return 4;
}
elseif ($a == 5) {
return 5;
}
return 'more';
}
}
?>
81 changes: 62 additions & 19 deletions rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php
Expand Up @@ -79,25 +79,7 @@ public function refactor(Node $node): ?array
}

if ($node->elseifs !== []) {
$originalNode = clone $node;
$if = new If_($node->cond);
$if->stmts = $node->stmts;

$this->mirrorComments($if, $node);

/** @var ElseIf_ $firstElseIf */
$firstElseIf = array_shift($node->elseifs);
$node->cond = $firstElseIf->cond;
$node->stmts = $firstElseIf->stmts;
$this->mirrorComments($node, $firstElseIf);

$nodesToReturnAfterNode = $this->getStatementsElseIfs($node);
if ($originalNode->else instanceof Else_) {
$node->else = null;
$nodesToReturnAfterNode = [...$nodesToReturnAfterNode, $originalNode->else];
}

return [$if, $node, ...$nodesToReturnAfterNode];
return $this->handleElseIfs($node);
}

if ($node->else instanceof Else_) {
Expand All @@ -110,6 +92,50 @@ public function refactor(Node $node): ?array
return null;
}

/**
* @return Node[]
*/
private function handleElseIfs(If_ $if): array
{
$nodesToReturn = [];

$originalIf = clone $if;
$firstIf = $this->createIfFromNode($if);
$nodesToReturn[] = $firstIf;

while ($if->elseifs !== []) {
/** @var ElseIf_ $currentElseIf */
$currentElseIf = array_shift($if->elseifs);

// If the last statement in the `elseif` breaks flow, merge it into the original `if` and stop processing
if ($this->doesLastStatementBreakFlow($currentElseIf)) {
$this->updateIfWithElseIf($if, $currentElseIf);
$nodesToReturn = [...$nodesToReturn, $if, ...$this->getStatementsElseIfs($if)];

break;
}

$isLastElseIf = $if->elseifs === [];

// If it's the last `elseif`, merge it with the original `if` to keep the formatting
if ($isLastElseIf) {
$this->updateIfWithElseIf($if, $currentElseIf);
$nodesToReturn[] = $if;

break;
}

// Otherwise, create a separate `if` node for `elseif`
$nodesToReturn[] = $this->createIfFromNode($currentElseIf);
}

if ($originalIf->else instanceof Else_) {
$nodesToReturn[] = $originalIf->else;
}

return $nodesToReturn;
}

/**
* @return ElseIf_[]
*/
Expand Down Expand Up @@ -151,4 +177,21 @@ private function doesLastStatementBreakFlow(If_ | ElseIf_ | Else_ $node): bool
|| $lastStmt instanceof Continue_
|| ($lastStmt instanceof Expression && $lastStmt->expr instanceof Exit_));
}

private function createIfFromNode(If_ | ElseIf_ $node): If_
{
$if = new If_($node->cond);
$if->stmts = $node->stmts;
$this->mirrorComments($if, $node);

return $if;
}

private function updateIfWithElseIf(If_ $if, ElseIf_ $elseIf): void
{
$if->cond = $elseIf->cond;
$if->stmts = $elseIf->stmts;
$this->mirrorComments($if, $elseIf);
$if->else = null;
}
}
Expand Up @@ -38,11 +38,13 @@ class ComplexIfCondAnd
if ($d && $e) {
return false;
}
elseif ($f && $g) {
return 1;
if (!$f) {
return 0;
}

return 0;
if (!$g) {
return 0;
}
return 1;
}
}

Expand Down

0 comments on commit 3c51cd8

Please sign in to comment.