Skip to content

Commit

Permalink
Fix wrong results in RemoveAlwaysElseRector (#4527)
Browse files Browse the repository at this point in the history
  • Loading branch information
jlherren committed Jul 16, 2023
1 parent 3b5db0a commit 818503e
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

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

class NestedIfWithElseAndNonTerminatingElseIf
{
public function run()
{
if ($cond1) {
if ($cond2) {
return 'foo';
} elseif ($cond3) {
echo 'Not returning';
} else {
return 'bar';
}
} else {
foo();
}
}
}

?>
-----
<?php

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

class NestedIfWithElseAndNonTerminatingElseIf
{
public function run()
{
if ($cond1) {
if ($cond2) {
return 'foo';
}
if ($cond3) {
echo 'Not returning';
}
else {
return 'bar';
}
} else {
foo();
}
}
}

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

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

class NestedIfWithNonTerminatingElse
{
public function run()
{
if ($cond1) {
if ($cond2) {
return 'foo';
} else {
echo 'Not returning';
}
} else {
foo();
}
}
}

?>
-----
<?php

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

class NestedIfWithNonTerminatingElse
{
public function run()
{
if ($cond1) {
if ($cond2) {
return 'foo';
}
echo 'Not returning';
} else {
foo();
}
}
}

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

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

class NestedIfWithTerminatingElseIfAndElse
{
public function run()
{
if ($cond1) {
if ($cond2) {
return 'foo';
} elseif ($cond3) {
return 'bar';
} else {
return 'baz';
}
} else {
foo();
}
}
}

?>
-----
<?php

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

class NestedIfWithTerminatingElseIfAndElse
{
public function run()
{
if ($cond1) {
if ($cond2) {
return 'foo';
}
if ($cond3) {
return 'bar';
}
else {
return 'baz';
}
}
foo();
}
}

?>
14 changes: 12 additions & 2 deletions rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,22 @@ private function getStatementsElseIfs(If_ $if): array
return $statements;
}

private function doesLastStatementBreakFlow(If_ | ElseIf_ $node): bool
private function doesLastStatementBreakFlow(If_ | ElseIf_ | Else_ $node): bool
{
$lastStmt = end($node->stmts);

if ($lastStmt instanceof If_ && $lastStmt->else instanceof Else_) {
return $this->doesLastStatementBreakFlow($lastStmt);
if ($this->doesLastStatementBreakFlow($lastStmt) || $this->doesLastStatementBreakFlow($lastStmt->else)) {
return true;
}

foreach ($lastStmt->elseifs as $elseIf) {
if ($this->doesLastStatementBreakFlow($elseIf)) {
return true;
}
}

return false;
}

return ! ($lastStmt instanceof Return_
Expand Down

0 comments on commit 818503e

Please sign in to comment.