From ca28dbc92198e4b8fab337e34012a944eceea898 Mon Sep 17 00:00:00 2001
From: dchaudhari7177 <111210939+dchaudhari7177@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:36:04 +0530
Subject: [PATCH] Do not let a trailing comment or an infinite loop hide a
 terminated block

isTerminatedInLastStmts() judges a block by its last stmt and accepts only a
Return_ or an exit/throw Expression. Two shapes slip past it:

  try {
      return something();
      // nothing to do here     <- parsed as Nop, and it is the last stmt
  } catch (Exception $e) {
      return null;
  }

  try {
      while (true) {            <- never falls through, but is not a Return_
          if ($a) { return 'A'; }
          $a = doSomething();
      }
  } catch (Exception $e) {
      return null;
  }

Both blocks always terminate, so isAlwaysTerminated() should say so, and callers
that ask "does this method fall through?" get false. Reported in
rectorphp/rector#9897, where it makes ConsoleExecuteReturnIntRector append an
unreachable `return Command::SUCCESS;` that PHPStan then flags as
deadCode.unreachable.

Trailing Nops are now popped before the last stmt is read, and a trailing
infinite loop goes through isTerminatedInfiniteLoop(), the same helper
isAlwaysTerminated() already uses one level up -- so a loop carrying a
break/goto still counts as falling through.

Three fixtures for RemoveUnreachableStatementRector: a try/catch with a
trailing comment, a try/catch ending in while (true), and an if/else with a
trailing comment.
---
 ...ated_if_else_with_trailing_comment.php.inc | 39 ++++++++++++++
 ...ated_try_catch_with_infinite_while.php.inc | 53 +++++++++++++++++++
 ...ed_try_catch_with_trailing_comment.php.inc | 43 +++++++++++++++
 src/NodeAnalyzer/TerminatedNodeAnalyzer.php   | 12 +++++
 4 files changed, 147 insertions(+)
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_trailing_comment.php.inc
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_infinite_while.php.inc
 create mode 100644 rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_trailing_comment.php.inc

diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_trailing_comment.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_trailing_comment.php.inc
new file mode 100644
index 00000000000..f5c8bf0de45
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_if_else_with_trailing_comment.php.inc
@@ -0,0 +1,39 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+class AlwaysTerminatedIfElseWithTrailingComment
+{
+    public function run($a)
+    {
+        if ($a) {
+            return 'A';
+            // and that is all
+        } else {
+            return 'B';
+        }
+
+        echo 'never executed';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+class AlwaysTerminatedIfElseWithTrailingComment
+{
+    public function run($a)
+    {
+        if ($a) {
+            return 'A';
+            // and that is all
+        } else {
+            return 'B';
+        }
+    }
+}
+
+?>
diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_infinite_while.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_infinite_while.php.inc
new file mode 100644
index 00000000000..95030eb7cec
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_infinite_while.php.inc
@@ -0,0 +1,53 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+use Exception;
+
+class AlwaysTerminatedTryCatchWithInfiniteWhile
+{
+    public function run($a)
+    {
+        try {
+            while (true) {
+                if ($a) {
+                    return 'A';
+                }
+
+                $a = doSomething();
+            }
+        } catch (Exception $e) {
+            return null;
+        }
+
+        echo 'never executed';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+use Exception;
+
+class AlwaysTerminatedTryCatchWithInfiniteWhile
+{
+    public function run($a)
+    {
+        try {
+            while (true) {
+                if ($a) {
+                    return 'A';
+                }
+
+                $a = doSomething();
+            }
+        } catch (Exception $e) {
+            return null;
+        }
+    }
+}
+
+?>
diff --git a/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_trailing_comment.php.inc b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_trailing_comment.php.inc
new file mode 100644
index 00000000000..1b262cce5b2
--- /dev/null
+++ b/rules-tests/DeadCode/Rector/Stmt/RemoveUnreachableStatementRector/Fixture/always_terminated_try_catch_with_trailing_comment.php.inc
@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+use Exception;
+
+class AlwaysTerminatedTryCatchWithTrailingComment
+{
+    public function run()
+    {
+        try {
+            return something();
+            // nothing to do here
+        } catch (Exception $e) {
+            return null;
+        }
+
+        echo 'never executed';
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Stmt\RemoveUnreachableStatementRector\Fixture;
+
+use Exception;
+
+class AlwaysTerminatedTryCatchWithTrailingComment
+{
+    public function run()
+    {
+        try {
+            return something();
+            // nothing to do here
+        } catch (Exception $e) {
+            return null;
+        }
+    }
+}
+
+?>
diff --git a/src/NodeAnalyzer/TerminatedNodeAnalyzer.php b/src/NodeAnalyzer/TerminatedNodeAnalyzer.php
index bb9752dfbac..0a1423c5f38 100644
--- a/src/NodeAnalyzer/TerminatedNodeAnalyzer.php
+++ b/src/NodeAnalyzer/TerminatedNodeAnalyzer.php
@@ -273,6 +273,12 @@ private function isTerminatedInLastStmtsIf(If_ $if): bool
      */
     private function isTerminatedInLastStmts(array $stmts): bool
     {
+        // a trailing comment is parsed as a Nop, which executes nothing and so
+        // must not hide the terminating stmt in front of it
+        while ($stmts !== [] && end($stmts) instanceof Nop) {
+            array_pop($stmts);
+        }
+
         if ($stmts === []) {
             return false;
         }
@@ -280,6 +286,12 @@ private function isTerminatedInLastStmts(array $stmts): bool
         $lastKey = array_key_last($stmts);
         $lastNode = $stmts[$lastKey];
 
+        // an infinite loop with no break terminates its block as surely as a
+        // return does, the same way isAlwaysTerminated() reads it one level up
+        if ($lastNode instanceof While_ || $lastNode instanceof Do_ || $lastNode instanceof For_) {
+            return $this->isTerminatedInfiniteLoop($lastNode);
+        }
+
         if ($lastNode instanceof Expression) {
             return $lastNode->expr instanceof Exit_ || $lastNode->expr instanceof Throw_;
         }