Introduce getNextStatements in UnreachableStatementNode

Co-authored-by: Ondrej Mirtes <[email protected]>
phpstan · Jan 2, 2025 · e36bb83 · e36bb83
1 parent 068be33
commit e36bb83
Show file tree

Hide file tree

Showing 6 changed files with 203 additions and 16 deletions.
diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
@@ -316,13 +316,38 @@ public function processNodes(
 			}
 
 			$alreadyTerminated = true;
-			$nextStmt = $this->getFirstUnreachableNode(array_slice($nodes, $i + 1), true);
-			if (!$nextStmt instanceof Node\Stmt) {
+			$nextStmts = $this->getNextUnreachableStatements(array_slice($nodes, $i + 1), true);
+			$this->processUnreachableStatement($nextStmts, $scope, $nodeCallback);
+		}
+	}
+
+	/**
+	 * @param Node\Stmt[] $nextStmts
+	 * @param callable(Node $node, Scope $scope): void $nodeCallback
+	 */
+	private function processUnreachableStatement(array $nextStmts, MutatingScope $scope, callable $nodeCallback): void
+	{
+		if ($nextStmts === []) {
+			return;
+		}
+
+		$unreachableStatement = null;
+		$nextStatements = [];
+
+		foreach ($nextStmts as $key => $nextStmt) {
+			if ($key === 0) {
+				$unreachableStatement = $nextStmt;
 				continue;
 			}
 
-			$nodeCallback(new UnreachableStatementNode($nextStmt), $scope);
+			$nextStatements[] = $nextStmt;
+		}
+
+		if (!$unreachableStatement instanceof Node\Stmt) {
+			return;
 		}
+
+		$nodeCallback(new UnreachableStatementNode($unreachableStatement, $nextStatements), $scope);
 	}
 
 	/**
@@ -409,11 +434,8 @@ public function processStmtNodes(
 			}
 
 			$alreadyTerminated = true;
-			$nextStmt = $this->getFirstUnreachableNode(array_slice($stmts, $i + 1), $parentNode instanceof Node\Stmt\Namespace_);
-			if ($nextStmt === null) {
-				continue;
-			}
-			$nodeCallback(new UnreachableStatementNode($nextStmt), $scope);
+			$nextStmts = $this->getNextUnreachableStatements(array_slice($stmts, $i + 1), $parentNode instanceof Node\Stmt\Namespace_);
+			$this->processUnreachableStatement($nextStmts, $scope, $nodeCallback);
 		}
 
 		$statementResult = new StatementResult($scope, $hasYield, $alreadyTerminated, $exitPoints, $throwPoints, $impurePoints);
@@ -6514,22 +6536,31 @@ private function getPhpDocReturnType(ResolvedPhpDocBlock $resolvedPhpDoc, Type $
 	}
 
 	/**
-	 * @template T of Node
-	 * @param array<T> $nodes
-	 * @return T|null
+	 * @param array<Node> $nodes
+	 * @return list<Node\Stmt>
 	 */
-	private function getFirstUnreachableNode(array $nodes, bool $earlyBinding): ?Node
+	private function getNextUnreachableStatements(array $nodes, bool $earlyBinding): array
 	{
+		$stmts = [];
+		$isPassedUnreachableStatement = false;
 		foreach ($nodes as $node) {
+			if ($earlyBinding && ($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassLike || $node instanceof Node\Stmt\HaltCompiler)) {
+				continue;
+			}
+			if ($isPassedUnreachableStatement && $node instanceof Node\Stmt) {
+				$stmts[] = $node;
+				continue;
+			}
 			if ($node instanceof Node\Stmt\Nop) {
 				continue;
 			}
-			if ($earlyBinding && ($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassLike || $node instanceof Node\Stmt\HaltCompiler)) {
+			if (!$node instanceof Node\Stmt) {
 				continue;
 			}
-			return $node;
+			$stmts[] = $node;
+			$isPassedUnreachableStatement = true;
 		}
-		return null;
+		return $stmts;
 	}
 
 }
diff --git a/src/Node/UnreachableStatementNode.php b/src/Node/UnreachableStatementNode.php
@@ -10,9 +10,12 @@
 final class UnreachableStatementNode extends Stmt implements VirtualNode
 {
 
-	public function __construct(private Stmt $originalStatement)
+	/** @param Stmt[] $nextStatements */
+	public function __construct(private Stmt $originalStatement, private array $nextStatements = [])
 	{
 		parent::__construct($originalStatement->getAttributes());
+
+		$this->nextStatements = $nextStatements;
 	}
 
 	public function getOriginalStatement(): Stmt
@@ -33,4 +36,12 @@ public function getSubNodeNames(): array
 		return [];
 	}
 
+	/**
+	 * @return Stmt[]
+	 */
+	public function getNextStatements(): array
+	{
+		return $this->nextStatements;
+	}
+
 }
diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementNextStatementsRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementNextStatementsRuleTest.php
@@ -0,0 +1,94 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\DeadCode;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\UnreachableStatementNode;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<Rule>
+ */
+class UnreachableStatementNextStatementsRuleTest extends RuleTestCase
+{
+
+	/**
+	 * @return Rule<Node>
+	 */
+	protected function getRule(): Rule
+	{
+		return new class implements Rule {
+
+			public function getNodeType(): string
+			{
+				return UnreachableStatementNode::class;
+			}
+
+			/**
+			 * @param UnreachableStatementNode $node
+			 */
+			public function processNode(Node $node, Scope $scope): array
+			{
+				$errors = [
+					RuleErrorBuilder::message('First unreachable')
+						->identifier('tests.nextUnreachableStatements')
+						->build(),
+				];
+
+				foreach ($node->getNextStatements() as $nextStatement) {
+					$errors[] = RuleErrorBuilder::message('Another unreachable')
+						->line($nextStatement->getStartLine())
+						->identifier('tests.nextUnreachableStatements')
+						->build();
+				}
+
+				return $errors;
+			}
+
+		};
+	}
+
+	public function testRule(): void
+	{
+		$this->analyse([__DIR__ . '/data/multiple_unreachable.php'], [
+			[
+				'First unreachable',
+				14,
+			],
+			[
+				'Another unreachable',
+				15,
+			],
+			[
+				'Another unreachable',
+				17,
+			],
+			[
+				'Another unreachable',
+				22,
+			],
+		]);
+	}
+
+	public function testRuleTopLevel(): void
+	{
+		$this->analyse([__DIR__ . '/data/multiple_unreachable_top_level.php'], [
+			[
+				'First unreachable',
+				9,
+			],
+			[
+				'Another unreachable',
+				10,
+			],
+			[
+				'Another unreachable',
+				17,
+			],
+		]);
+	}
+
+}
diff --git a/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php
@@ -230,4 +230,15 @@ public function testBug11992(): void
 		$this->analyse([__DIR__ . '/data/bug-11992.php'], []);
 	}
 
+	public function testMultipleUnreachable(): void
+	{
+		$this->treatPhpDocTypesAsCertain = true;
+		$this->analyse([__DIR__ . '/data/multiple_unreachable.php'], [
+			[
+				'Unreachable statement - code above always terminates.',
+				14,
+			],
+		]);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable.php b/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace MultipleUnreachable;
+
+/**
+ * @param 'foo' $foo
+ */
+function foo($foo)
+{
+    if ($foo === 'foo') {
+        return 1;
+    }
+
+    echo 'statement 1';
+    echo 'statement 2';
+
+    function innerFunction()
+    {
+        echo 'statement 3';
+    }
+
+    echo innerFunction();
+}
diff --git a/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable_top_level.php b/tests/PHPStan/Rules/DeadCode/data/multiple_unreachable_top_level.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace MultipleUnreachableTopLevel;
+
+if (true) {
+	return 1;
+}
+
+echo 'statement 1';
+echo 'statement 2';
+
+function func()
+{
+	echo 'statement 3';
+}
+
+echo func();