Ignore Nop nodes in NodeScopeResolver when creating UnreachableStatem…

…entNode
phpstan · Mar 8, 2023 · fa580cd · fa580cd
1 parent 04edd04
commit fa580cd
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
 	 */