From b9237d38529a4eec1067292712b5961d51b03f7e Mon Sep 17 00:00:00 2001
From: Tomas Votruba <tomas.vot@gmail.com>
Date: Sun, 2 Aug 2026 11:37:20 +0200
Subject: [PATCH] [NodeAnalyzer] Detect always terminated stmts with statements
 before the return

TerminatedNodeAnalyzer bailed out whenever the second-to-last statement of
a try/catch, if/else or switch branch was not itself a terminator. That guard
was added in #6837 to keep a break inside a switch case from being treated as
always terminated, but it also rejected every branch doing real work before
its return.

Move the check to the Switch_ branch, where a break/continue actually escapes
to the next statement, so the generic path only looks at the last statement.
---
 ...d_if_else_with_stmts_before_return.php.inc | 41 ++++++++++
 ...ed_switch_with_stmts_before_return.php.inc | 43 ++++++++++
 ...try_catch_with_stmts_before_return.php.inc | 45 ++++++++++
 ...skip_switch_case_nested_loop_break.php.inc | 22 +++++
 src/NodeAnalyzer/TerminatedNodeAnalyzer.php   | 82 +++++++++++++------
 5 files changed, 210 insertions(+), 23 deletions(-)
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_stmts_before_return.php.inc
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_switch_with_stmts_before_return.php.inc
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_stmts_before_return.php.inc
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/skip_switch_case_nested_loop_break.php.inc

diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_stmts_before_return.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_stmts_before_return.php.inc
new file mode 100644
index 00000000000..fd422e3acb8
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_stmts_before_return.php.inc
@@ -0,0 +1,41 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+class AlwaysTerminatedIfElseWithStmtsBeforeReturn
+{
+    public function run($a)
+    {
+        if ($a) {
+            echo 'if';
+            return 'A';
+        } else {
+            echo 'else';
+            return 'B';
+        }
+
+        echo 'never executed';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+class AlwaysTerminatedIfElseWithStmtsBeforeReturn
+{
+    public function run($a)
+    {
+        if ($a) {
+            echo 'if';
+            return 'A';
+        } else {
+            echo 'else';
+            return 'B';
+        }
+    }
+}
+
+?>
diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_switch_with_stmts_before_return.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_switch_with_stmts_before_return.php.inc
new file mode 100644
index 00000000000..205b62cdd4d
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_switch_with_stmts_before_return.php.inc
@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+class AlwaysTerminatedSwitchWithStmtsBeforeReturn
+{
+    public function run($a)
+    {
+        switch ($a) {
+            case 'a':
+                echo 'a';
+                return 'A';
+            default:
+                echo 'default';
+                return 'B';
+        }
+
+        echo 'never executed';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+class AlwaysTerminatedSwitchWithStmtsBeforeReturn
+{
+    public function run($a)
+    {
+        switch ($a) {
+            case 'a':
+                echo 'a';
+                return 'A';
+            default:
+                echo 'default';
+                return 'B';
+        }
+    }
+}
+
+?>
diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_stmts_before_return.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_stmts_before_return.php.inc
new file mode 100644
index 00000000000..1b8b580b4ce
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_stmts_before_return.php.inc
@@ -0,0 +1,45 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+use Exception;
+
+class AlwaysTerminatedTryCatchWithStmtsBeforeReturn
+{
+    public function run()
+    {
+        try {
+            echo 'try';
+            return something();
+        } catch (Exception $e) {
+            echo 'catch';
+            return null;
+        }
+
+        echo 'never executed';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+use Exception;
+
+class AlwaysTerminatedTryCatchWithStmtsBeforeReturn
+{
+    public function run()
+    {
+        try {
+            echo 'try';
+            return something();
+        } catch (Exception $e) {
+            echo 'catch';
+            return null;
+        }
+    }
+}
+
+?>
diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/skip_switch_case_nested_loop_break.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/skip_switch_case_nested_loop_break.php.inc
new file mode 100644
index 00000000000..0781d2a1289
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/skip_switch_case_nested_loop_break.php.inc
@@ -0,0 +1,22 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+final class SkipSwitchCaseNestedLoopBreak
+{
+    public function run($a, array $items)
+    {
+        switch ($a) {
+            case 'a':
+                foreach ($items as $item) {
+                    break;
+                }
+
+                return 'A';
+            default:
+                return 'B';
+        }
+
+        echo 'kept, as break targets are not resolved';
+    }
+}
diff --git a/src/NodeAnalyzer/TerminatedNodeAnalyzer.php b/src/NodeAnalyzer/TerminatedNodeAnalyzer.php
index 444df395921..c6f701d99dd 100644
--- a/src/NodeAnalyzer/TerminatedNodeAnalyzer.php
+++ b/src/NodeAnalyzer/TerminatedNodeAnalyzer.php
@@ -8,6 +8,7 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Exit_;
 use PhpParser\Node\Expr\Throw_;
+use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Break_;
 use PhpParser\Node\Stmt\ClassLike;
@@ -25,9 +26,11 @@
 use PhpParser\Node\Stmt\Return_;
 use PhpParser\Node\Stmt\Switch_;
 use PhpParser\Node\Stmt\TryCatch;
+use PhpParser\NodeVisitor;
+use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 use Rector\PhpParser\Node\FileNode;
 
-final class TerminatedNodeAnalyzer
+final readonly class TerminatedNodeAnalyzer
 {
     /**
      * @var array<class-string<Node>>
@@ -44,6 +47,11 @@ final class TerminatedNodeAnalyzer
      */
     private const array ALLOWED_CONTINUE_CURRENT_STMTS = [InlineHTML::class, Nop::class];
 
+    public function __construct(
+        private SimpleCallableNodeTraverser $simpleCallableNodeTraverser
+    ) {
+    }
+
     /**
      * @param StmtsAware $stmtsAware
      */
@@ -62,18 +70,18 @@ public function isAlwaysTerminated(Node $stmtsAware, Stmt $node, Stmt $currentSt
         }
 
         if ($node instanceof TryCatch) {
-            return $this->isTerminatedInLastStmtsTryCatch($node, $currentStmt);
+            return $this->isTerminatedInLastStmtsTryCatch($node);
         }
 
         if ($node instanceof If_) {
-            return $this->isTerminatedInLastStmtsIf($node, $currentStmt);
+            return $this->isTerminatedInLastStmtsIf($node);
         }
 
         /** @var Switch_ $node */
-        return $this->isTerminatedInLastStmtsSwitch($node, $currentStmt);
+        return $this->isTerminatedInLastStmtsSwitch($node);
     }
 
-    private function isTerminatedNode(Node $previousNode, Node $currentStmt): bool
+    private function isTerminatedNode(Stmt $previousNode, Stmt $currentStmt): bool
     {
         if (in_array($previousNode::class, self::TERMINABLE_NODES, true)) {
             return true;
@@ -90,12 +98,17 @@ private function isTerminatedNode(Node $previousNode, Node $currentStmt): bool
         return false;
     }
 
-    private function isTerminatedInLastStmtsSwitch(Switch_ $switch, Stmt $stmt): bool
+    private function isTerminatedInLastStmtsSwitch(Switch_ $switch): bool
     {
         if ($switch->cases === []) {
             return false;
         }
 
+        // a break/continue jumps out of the Switch_, so the next stmt is still executable
+        if ($this->hasEscapingJump($switch)) {
+            return false;
+        }
+
         $hasDefault = false;
         foreach ($switch->cases as $key => $case) {
             if (! $case->cond instanceof Expr) {
@@ -106,7 +119,7 @@ private function isTerminatedInLastStmtsSwitch(Switch_ $switch, Stmt $stmt): boo
                 continue;
             }
 
-            if (! $this->isTerminatedInLastStmts($case->stmts, $stmt)) {
+            if (! $this->isTerminatedInLastStmts($case->stmts)) {
                 return false;
             }
         }
@@ -114,25 +127,52 @@ private function isTerminatedInLastStmtsSwitch(Switch_ $switch, Stmt $stmt): boo
         return $hasDefault;
     }
 
-    private function isTerminatedInLastStmtsTryCatch(TryCatch $tryCatch, Stmt $stmt): bool
+    private function hasEscapingJump(Switch_ $switch): bool
     {
-        if ($tryCatch->finally instanceof Finally_ && $this->isTerminatedInLastStmts(
-            $tryCatch->finally->stmts,
-            $stmt
-        )) {
+        $hasEscapingJump = false;
+
+        foreach ($switch->cases as $case) {
+            $this->simpleCallableNodeTraverser->traverseNodesWithCallable(
+                $case->stmts,
+                static function (Node $node) use (&$hasEscapingJump): ?int {
+                    // nested scopes bring their own jump targets
+                    if ($node instanceof FunctionLike || $node instanceof ClassLike) {
+                        return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
+                    }
+
+                    if (! $node instanceof Break_ && ! $node instanceof Continue_ && ! $node instanceof Goto_) {
+                        return null;
+                    }
+
+                    $hasEscapingJump = true;
+                    return NodeVisitor::STOP_TRAVERSAL;
+                }
+            );
+
+            if ($hasEscapingJump) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private function isTerminatedInLastStmtsTryCatch(TryCatch $tryCatch): bool
+    {
+        if ($tryCatch->finally instanceof Finally_ && $this->isTerminatedInLastStmts($tryCatch->finally->stmts)) {
             return true;
         }
 
         foreach ($tryCatch->catches as $catch) {
-            if (! $this->isTerminatedInLastStmts($catch->stmts, $stmt)) {
+            if (! $this->isTerminatedInLastStmts($catch->stmts)) {
                 return false;
             }
         }
 
-        return $this->isTerminatedInLastStmts($tryCatch->stmts, $stmt);
+        return $this->isTerminatedInLastStmts($tryCatch->stmts);
     }
 
-    private function isTerminatedInLastStmtsIf(If_ $if, Stmt $stmt): bool
+    private function isTerminatedInLastStmtsIf(If_ $if): bool
     {
         // Without ElseIf_[] and Else_, after If_ is possibly executable
         if ($if->elseifs === [] && ! $if->else instanceof Else_) {
@@ -140,12 +180,12 @@ private function isTerminatedInLastStmtsIf(If_ $if, Stmt $stmt): bool
         }
 
         foreach ($if->elseifs as $elseif) {
-            if (! $this->isTerminatedInLastStmts($elseif->stmts, $stmt)) {
+            if (! $this->isTerminatedInLastStmts($elseif->stmts)) {
                 return false;
             }
         }
 
-        if (! $this->isTerminatedInLastStmts($if->stmts, $stmt)) {
+        if (! $this->isTerminatedInLastStmts($if->stmts)) {
             return false;
         }
 
@@ -153,13 +193,13 @@ private function isTerminatedInLastStmtsIf(If_ $if, Stmt $stmt): bool
             return false;
         }
 
-        return $this->isTerminatedInLastStmts($if->else->stmts, $stmt);
+        return $this->isTerminatedInLastStmts($if->else->stmts);
     }
 
     /**
      * @param Stmt[] $stmts
      */
-    private function isTerminatedInLastStmts(array $stmts, Node $node): bool
+    private function isTerminatedInLastStmts(array $stmts): bool
     {
         if ($stmts === []) {
             return false;
@@ -168,10 +208,6 @@ private function isTerminatedInLastStmts(array $stmts, Node $node): bool
         $lastKey = array_key_last($stmts);
         $lastNode = $stmts[$lastKey];
 
-        if (isset($stmts[$lastKey - 1]) && ! $this->isTerminatedNode($stmts[$lastKey - 1], $node)) {
-            return false;
-        }
-
         if ($lastNode instanceof Expression) {
             return $lastNode->expr instanceof Exit_ || $lastNode->expr instanceof Throw_;
         }