From fa580cde1f8080252930d3cf84332d83087b34d7 Mon Sep 17 00:00:00 2001
From: Martin Herndl <martin@herndl.org>
Date: Wed, 8 Mar 2023 10:55:21 +0100
Subject: [PATCH] Ignore Nop nodes in NodeScopeResolver when creating
 UnreachableStatementNode

---
 src/Analyser/NodeScopeResolver.php            | 33 +++++++++++++------
 .../DeadCode/UnreachableStatementRule.php     |  4 ---
 .../DeadCode/UnreachableStatementRuleTest.php | 23 +++++++++++--
 .../Rules/DeadCode/data/unreachable.php       | 22 +++++++++++++
 4 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
index fbbd17c4098..fe50e19f246 100644
--- a/src/Analyser/NodeScopeResolver.php
+++ b/src/Analyser/NodeScopeResolver.php
@@ -240,7 +240,6 @@ public function processNodes(
 		callable $nodeCallback,
 	): void
 	{
-		$nodesCount = count($nodes);
 		foreach ($nodes as $i => $node) {
 			if (!$node instanceof Node\Stmt) {
 				continue;
@@ -252,14 +251,12 @@ public function processNodes(
 				continue;
 			}
 
-			if ($i < $nodesCount - 1) {
-				$nextStmt = $nodes[$i + 1];
-				if (!$nextStmt instanceof Node\Stmt) {
-					continue;
-				}
-
-				$nodeCallback(new UnreachableStatementNode($nextStmt), $scope);
+			$nextStmt = $this->getFirstNonNopNode(array_slice($nodes, $i + 1));
+			if (!$nextStmt instanceof Node\Stmt) {
+				continue;
 			}
+
+			$nodeCallback(new UnreachableStatementNode($nextStmt), $scope);
 			break;
 		}
 	}
@@ -323,8 +320,8 @@ public function processStmtNodes(
 			}
 
 			$alreadyTerminated = true;
-			if ($i < $stmtCount - 1) {
-				$nextStmt = $stmts[$i + 1];
+			$nextStmt = $this->getFirstNonNopNode(array_slice($stmts, $i + 1));
+			if ($nextStmt !== null) {
 				$nodeCallback(new UnreachableStatementNode($nextStmt), $scope);
 			}
 			break;
@@ -4365,4 +4362,20 @@ private function getPhpDocReturnType(ResolvedPhpDocBlock $resolvedPhpDoc, Type $
 		return null;
 	}
 
+	/**
+	 * @template T of Node
+	 * @param array<T> $nodes
+	 * @return T
+	 */
+	private function getFirstNonNopNode(array $nodes): ?Node
+	{
+		foreach ($nodes as $node) {
+			if (!$node instanceof Node\Stmt\Nop) {
+				return $node;
+			}
+		}
+
+		return null;
+	}
+
 }
diff --git a/src/Rules/DeadCode/UnreachableStatementRule.php b/src/Rules/DeadCode/UnreachableStatementRule.php
index 8f4793267a7..ffe4876c6de 100644
--- a/src/Rules/DeadCode/UnreachableStatementRule.php
+++ b/src/Rules/DeadCode/UnreachableStatementRule.php
@@ -21,10 +21,6 @@ public function getNodeType(): string
 
 	public function processNode(Node $node, Scope $scope): array
 	{
-		if ($node->getOriginalStatement() instanceof Node\Stmt\Nop) {
-			return [];
-		}
-
 		return [
 			RuleErrorBuilder::message('Unreachable statement - code above always terminates.')
 				->identifier('deadCode.unreachableStatement')
diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php
index e0b4d4ce6c9..52324c96df3 100644
--- a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php
+++ b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php
@@ -41,11 +41,19 @@ public function testRule(): void
 			],
 			[
 				'Unreachable statement - code above always terminates.',
-				71,
+				44,
 			],
 			[
 				'Unreachable statement - code above always terminates.',
-				135,
+				58,
+			],
+			[
+				'Unreachable statement - code above always terminates.',
+				93,
+			],
+			[
+				'Unreachable statement - code above always terminates.',
+				157,
 			],
 		]);
 	}
@@ -141,4 +149,15 @@ public function testBug8620(): void
 		$this->analyse([__DIR__ . '/data/bug-8620.php'], []);
 	}
 
+	public function testBug8966(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/bug-8966.php'], [
+			[
+				'Unreachable statement - code above always terminates.',
+				8,
+			],
+		]);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/DeadCode/data/unreachable.php b/tests/PHPStan/Rules/DeadCode/data/unreachable.php
index b43f2507fb8..48f39153f12 100644
--- a/tests/PHPStan/Rules/DeadCode/data/unreachable.php
+++ b/tests/PHPStan/Rules/DeadCode/data/unreachable.php
@@ -36,6 +36,28 @@ public function doLorem()
 		// this is why...
 	}
 
+	public function doLorem2(string $foo)
+	{
+		return;
+		// this is why...
+
+		echo $foo;
+	}
+
+	public function doLorem3()
+	{
+		return;
+		;
+	}
+
+	public function doLorem4(string $foo)
+	{
+		return;
+		;
+
+		echo $foo;
+	}
+
 	/**
 	 * @param \stdClass[] $all
 	 */