Fix missing detection of dead code in expressions

Author: staabm

src/Analyser/ExpressionResult.php

@@ -28,6 +28,7 @@ public function __construct(
 		private array $impurePoints,
 		?callable $truthyScopeCallback = null,
 		?callable $falseyScopeCallback = null,
+		private bool $isAlwaysTerminating = false,
 	)
 	{
 		$this->truthyScopeCallback = $truthyScopeCallback;
@@ -90,4 +91,9 @@ public function getFalseyScope(): MutatingScope
 		return $this->falseyScope;
 	}
 
+	public function isAlwaysTerminating(): bool
+	{
+		return $this->isAlwaysTerminating;
+	}
+
 }

src/Analyser/NodeScopeResolver.php

@@ -840,17 +840,20 @@ private function processStmtNode(
 		} elseif ($stmt instanceof Echo_) {
 			$hasYield = false;
 			$throwPoints = [];
+			$isAlwaysTerminating = false;
 			foreach ($stmt->exprs as $echoExpr) {
 				$result = $this->processExprNode($stmt, $echoExpr, $scope, $nodeCallback, ExpressionContext::createDeep());
 				$throwPoints = array_merge($throwPoints, $result->getThrowPoints());
 				$scope = $result->getScope();
 				$hasYield = $hasYield || $result->hasYield();
+				$isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating();
 			}
 
 			$throwPoints = $overridingThrowPoints ?? $throwPoints;
 			$impurePoints = [
 				new ImpurePoint($scope, $stmt, 'echo', 'echo', true),
 			];
+			return new StatementResult($scope, $hasYield, $isAlwaysTerminating, [], $throwPoints, $impurePoints);
 		} elseif ($stmt instanceof Return_) {
 			if ($stmt->expr !== null) {
 				$result = $this->processExprNode($stmt, $stmt->expr, $scope, $nodeCallback, ExpressionContext::createDeep());
@@ -2408,6 +2411,7 @@ public function processExprNode(Node\Stmt $stmt, Expr $expr, MutatingScope $scop
 			return $this->processExprNode($stmt, $newExpr, $scope, $nodeCallback, $context);
 		}
 
+		$isAlwaysTerminating = false;
 		$this->callNodeCallbackWithExpression($nodeCallback, $expr, $scope, $context);
 
 		if ($expr instanceof Variable) {
@@ -2591,6 +2595,7 @@ static function (): void {
 
 			if ($parametersAcceptor !== null) {
 				$expr = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $expr) ?? $expr;
+				$isAlwaysTerminating = $parametersAcceptor->getReturnType() instanceof NeverType;
 			}
 			$result = $this->processArgs($stmt, $functionReflection, null, $parametersAcceptor, $expr, $scope, $nodeCallback, $context);
 			$scope = $result->getScope();
@@ -3300,6 +3305,7 @@ static function (): void {
 			$hasYield = $result->hasYield();
 			$throwPoints = $result->getThrowPoints();
 			$impurePoints = $result->getImpurePoints();
+			$isAlwaysTerminating = $result->isAlwaysTerminating();
 			$result = $this->processExprNode($stmt, $expr->right, $scope, $nodeCallback, $context->enterDeep());
 			if (
 				($expr instanceof BinaryOp\Div || $expr instanceof BinaryOp\Mod) &&
@@ -3311,6 +3317,7 @@ static function (): void {
 			$hasYield = $hasYield || $result->hasYield();
 			$throwPoints = array_merge($throwPoints, $result->getThrowPoints());
 			$impurePoints = array_merge($impurePoints, $result->getImpurePoints());
+			$isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating();
 		} elseif ($expr instanceof Expr\Include_) {
 			$result = $this->processExprNode($stmt, $expr->expr, $scope, $nodeCallback, $context->enterDeep());
 			$throwPoints = $result->getThrowPoints();
@@ -3996,6 +4003,7 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
 			$impurePoints,
 			static fn (): MutatingScope => $scope->filterByTruthyValue($expr),
 			static fn (): MutatingScope => $scope->filterByFalseyValue($expr),
+			$isAlwaysTerminating,
 		);
 	}
 

tests/PHPStan/Rules/DeadCode/UnreachableStatementRuleTest.php

@@ -240,4 +240,26 @@ public function testMultipleUnreachable(): void
 		]);
 	}
 
+	public function testBug13232a(): void
+	{
+		$this->treatPhpDocTypesAsCertain = false;
+		$this->analyse([__DIR__ . '/data/bug-13232a.php'], [
+			[
+				'Unreachable statement - code above always terminates.',
+				11,
+			],
+		]);
+	}
+
+	public function testBug13232b(): void
+	{
+		$this->treatPhpDocTypesAsCertain = false;
+		$this->analyse([__DIR__ . '/data/bug-13232b.php'], [
+			[
+				'Unreachable statement - code above always terminates.',
+				19,
+			],
+		]);
+	}
+
 }

tests/PHPStan/Rules/DeadCode/data/bug-13232a.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace Bug13232a;
+
+final class HelloWorld
+{
+	public function sayHi(): void
+	{
+		echo 'Hello, ' . neverReturns()
+			. ' no way';
+		echo 'this will never happen';
+	}
+}
+function neverReturns(): never {}
+

tests/PHPStan/Rules/DeadCode/data/bug-13232b.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Bug13232b;
+
+final class HelloWorld
+{
+	public function sayHello(): void
+	{
+		if (rand(0, 1)) {
+			$x = new Foo();
+		} else {
+			$x = '1';
+		}
+		echo 'Hello, ' . mightReturnNever($x)
+			. ' no way';
+
+		echo 'Hello, ' . mightReturnNever(new Foo())
+			. ' no way';
+		echo 'this will never happen';
+	}
+}
+
+/** @phpstan-return ($x is Foo ? never : string) */
+function mightReturnNever(mixed $x)
+{
+	if ($x instanceof Foo) {
+		throw new LogicException();
+	}
+	return "hello";
+}
+
+class Foo
+{
+}