[EarlyReturn] Handle If, elseIf, else all returned on RemoveAlwaysElseRector (#7659)

samsonasik · web-flow · commit 3cf1f519c544 · 2025-11-22T15:10:14.000Z
* [EarlyReturn] Handle If, elseIf, else all returned on RemoveAlwaysElseRector

* Fix method name as it verify not break flow
diff --git a/rules-tests/EarlyReturn/Rector/If_/RemoveAlwaysElseRector/Fixture/if_elseif_else.php.inc b/rules-tests/EarlyReturn/Rector/If_/RemoveAlwaysElseRector/Fixture/if_elseif_else.php.inc
@@ -0,0 +1,42 @@
+<?php
+
+namespace Rector\Tests\EarlyReturn\Rector\If_\RemoveAlwaysElseRector\Fixture;
+
+final class IfElseIfElse
+{
+    public function run($value, $formatForEmail, $time_context, $direction)
+    {
+        if ($value == 0) {
+            return $value;
+        } elseif ($formatForEmail) {
+            $value .= '|' . 1;
+
+            return $time_context . '&nbsp;&nbsp;//&nbsp;&nbsp;' . $direction . $value . ' minute(s)';
+        } else {
+            return $value;
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\EarlyReturn\Rector\If_\RemoveAlwaysElseRector\Fixture;
+
+final class IfElseIfElse
+{
+    public function run($value, $formatForEmail, $time_context, $direction)
+    {
+        if ($value == 0) {
+            return $value;
+        }
+        if ($formatForEmail) {
+            $value .= '|' . 1;
+            return $time_context . '&nbsp;&nbsp;//&nbsp;&nbsp;' . $direction . $value . ' minute(s)';
+        }
+        return $value;
+    }
+}
+
+?>
diff --git a/rules-tests/EarlyReturn/Rector/If_/RemoveAlwaysElseRector/Fixture/nested_if_with_terminating_elseif_and_else.php.inc b/rules-tests/EarlyReturn/Rector/If_/RemoveAlwaysElseRector/Fixture/nested_if_with_terminating_elseif_and_else.php.inc
@@ -37,9 +37,7 @@ class NestedIfWithTerminatingElseIfAndElse
             if ($cond3) {
                 return 'bar';
             }
-            else {
-                return 'baz';
-            }
+            return 'baz';
         }
         foo();
     }
diff --git a/rules-tests/EarlyReturn/Rector/If_/RemoveAlwaysElseRector/Fixture/process_empty_return_last.php.inc b/rules-tests/EarlyReturn/Rector/If_/RemoveAlwaysElseRector/Fixture/process_empty_return_last.php.inc
@@ -47,9 +47,7 @@ class ProcessEmptyReturnLast
             $value = 55;
             return 10;
         }
-        else {
-            return;
-        }
+        return;
     }
 
     public function secondRun($value)
diff --git a/rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php b/rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php
@@ -74,7 +74,7 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?array
     {
-        if ($this->doesLastStatementBreakFlow($node)) {
+        if ($this->doesNotLastStatementBreakFlow($node)) {
             return null;
         }
 
@@ -108,7 +108,7 @@ private function handleElseIfs(If_ $if): array
             $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)) {
+            if ($this->doesNotLastStatementBreakFlow($currentElseIf)) {
                 $this->updateIfWithElseIf($if, $currentElseIf);
                 $nodesToReturn = [...$nodesToReturn, $if, ...$this->getStatementsElseIfs($if)];
 
@@ -130,7 +130,18 @@ private function handleElseIfs(If_ $if): array
         }
 
         if ($originalIf->else instanceof Else_) {
-            $nodesToReturn[] = $originalIf->else;
+            $mergeStmts = true;
+            foreach ($nodesToReturn as $nodeToReturn) {
+                if ($this->doesNotLastStatementBreakFlow($nodeToReturn)) {
+                    $nodesToReturn[] = $originalIf->else;
+                    $mergeStmts = false;
+                    break;
+                }
+            }
+
+            if ($mergeStmts) {
+                $nodesToReturn = array_merge($nodesToReturn, $originalIf->else->stmts);
+            }
         }
 
         return $nodesToReturn;
@@ -143,7 +154,7 @@ private function getStatementsElseIfs(If_ $if): array
     {
         $statements = [];
         foreach ($if->elseifs as $key => $elseif) {
-            if ($this->doesLastStatementBreakFlow($elseif) && $elseif->stmts !== []) {
+            if ($this->doesNotLastStatementBreakFlow($elseif) && $elseif->stmts !== []) {
                 continue;
             }
 
@@ -154,17 +165,17 @@ private function getStatementsElseIfs(If_ $if): array
         return $statements;
     }
 
-    private function doesLastStatementBreakFlow(If_ | ElseIf_ | Else_ $node): bool
+    private function doesNotLastStatementBreakFlow(If_ | ElseIf_ | Else_ $node): bool
     {
         $lastStmt = end($node->stmts);
 
         if ($lastStmt instanceof If_ && $lastStmt->else instanceof Else_) {
-            if ($this->doesLastStatementBreakFlow($lastStmt) || $this->doesLastStatementBreakFlow($lastStmt->else)) {
+            if ($this->doesNotLastStatementBreakFlow($lastStmt) || $this->doesNotLastStatementBreakFlow($lastStmt->else)) {
                 return true;
             }
 
             foreach ($lastStmt->elseifs as $elseIf) {
-                if ($this->doesLastStatementBreakFlow($elseIf)) {
+                if ($this->doesNotLastStatementBreakFlow($elseIf)) {
                     return true;
                 }
             }

Original file line number	Diff line number	Diff line change
`@@ -37,9 +37,7 @@ class NestedIfWithTerminatingElseIfAndElse`
`37`	`37`	`if ($cond3) {`
`38`	`38`	`return 'bar';`
`39`	`39`	`}`
`40`		`- else {`
`41`		`- return 'baz';`
`42`		`- }`
	`40`	`+ return 'baz';`
`43`	`41`	`}`
`44`	`42`	`foo();`
`45`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,9 +47,7 @@ class ProcessEmptyReturnLast`
`47`	`47`	`$value = 55;`
`48`	`48`	`return 10;`
`49`	`49`	`}`
`50`		`- else {`
`51`		`- return;`
`52`		`- }`
	`50`	`+ return;`
`53`	`51`	`}`
`54`	`52`
`55`	`53`	`public function secondRun($value)`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ public function getNodeTypes(): array`
`74`	`74`	`*/`
`75`	`75`	`public function refactor(Node $node): ?array`
`76`	`76`	`{`
`77`		`- if ($this->doesLastStatementBreakFlow($node)) {`
	`77`	`+ if ($this->doesNotLastStatementBreakFlow($node)) {`
`78`	`78`	`return null;`
`79`	`79`	`}`
`80`	`80`
`@@ -108,7 +108,7 @@ private function handleElseIfs(If_ $if): array`
`108`	`108`	`$currentElseIf = array_shift($if->elseifs);`
`109`	`109`
`110`	`110`	// If the last statement in the `elseif` breaks flow, merge it into the original `if` and stop processing
`111`		`- if ($this->doesLastStatementBreakFlow($currentElseIf)) {`
	`111`	`+ if ($this->doesNotLastStatementBreakFlow($currentElseIf)) {`
`112`	`112`	`$this->updateIfWithElseIf($if, $currentElseIf);`
`113`	`113`	`$nodesToReturn = [...$nodesToReturn, $if, ...$this->getStatementsElseIfs($if)];`
`114`	`114`
`@@ -130,7 +130,18 @@ private function handleElseIfs(If_ $if): array`
`130`	`130`	`}`
`131`	`131`
`132`	`132`	`if ($originalIf->else instanceof Else_) {`
`133`		`- $nodesToReturn[] = $originalIf->else;`
	`133`	`+ $mergeStmts = true;`
	`134`	`+ foreach ($nodesToReturn as $nodeToReturn) {`
	`135`	`+ if ($this->doesNotLastStatementBreakFlow($nodeToReturn)) {`
	`136`	`+ $nodesToReturn[] = $originalIf->else;`
	`137`	`+ $mergeStmts = false;`
	`138`	`+ break;`
	`139`	`+ }`
	`140`	`+ }`
	`141`	`+`
	`142`	`+ if ($mergeStmts) {`
	`143`	`+ $nodesToReturn = array_merge($nodesToReturn, $originalIf->else->stmts);`
	`144`	`+ }`
`134`	`145`	`}`
`135`	`146`
`136`	`147`	`return $nodesToReturn;`
`@@ -143,7 +154,7 @@ private function getStatementsElseIfs(If_ $if): array`
`143`	`154`	`{`
`144`	`155`	`$statements = [];`
`145`	`156`	`foreach ($if->elseifs as $key => $elseif) {`
`146`		`- if ($this->doesLastStatementBreakFlow($elseif) && $elseif->stmts !== []) {`
	`157`	`+ if ($this->doesNotLastStatementBreakFlow($elseif) && $elseif->stmts !== []) {`
`147`	`158`	`continue;`
`148`	`159`	`}`
`149`	`160`
`@@ -154,17 +165,17 @@ private function getStatementsElseIfs(If_ $if): array`
`154`	`165`	`return $statements;`
`155`	`166`	`}`
`156`	`167`
`157`		`- private function doesLastStatementBreakFlow(If_ \| ElseIf_ \| Else_ $node): bool`
	`168`	`+ private function doesNotLastStatementBreakFlow(If_ \| ElseIf_ \| Else_ $node): bool`
`158`	`169`	`{`
`159`	`170`	`$lastStmt = end($node->stmts);`
`160`	`171`
`161`	`172`	`if ($lastStmt instanceof If_ && $lastStmt->else instanceof Else_) {`
`162`		`- if ($this->doesLastStatementBreakFlow($lastStmt) \|\| $this->doesLastStatementBreakFlow($lastStmt->else)) {`
	`173`	`+ if ($this->doesNotLastStatementBreakFlow($lastStmt) \|\| $this->doesNotLastStatementBreakFlow($lastStmt->else)) {`
`163`	`174`	`return true;`
`164`	`175`	`}`
`165`	`176`
`166`	`177`	`foreach ($lastStmt->elseifs as $elseIf) {`
`167`		`- if ($this->doesLastStatementBreakFlow($elseIf)) {`
	`178`	`+ if ($this->doesNotLastStatementBreakFlow($elseIf)) {`
`168`	`179`	`return true;`
`169`	`180`	`}`
`170`	`181`	`}`