Do not let a trailing comment or an infinite loop hide a terminated block (#8476)

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.

Author: dchaudhari7177

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';
+        }
+    }
+}
+
+?>

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;
+        }
+    }
+}
+
+?>

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;
+        }
+    }
+}
+
+?>

src/NodeAnalyzer/TerminatedNodeAnalyzer.php

@@ -273,13 +273,25 @@ 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;
         }
 
         $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_;
         }