Do not merge dead scope into break exit points when loop body always terminates

src/Analyser/NodeScopeResolver.php

@@ -1288,15 +1288,17 @@
 			$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
 			$storage = $originalStorage;
 			$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
-			$finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
-			$finalScope = $finalScopeResult->getScope();
+			$rawFinalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context);
+			$foreachBodyAlwaysTerminated = $rawFinalScopeResult->isAlwaysTerminating();
+			$finalScopeResult = $rawFinalScopeResult->filterOutLoopExitPoints();
+			$finalScope = $foreachBodyAlwaysTerminated ? null : $finalScopeResult->getScope();
 			$scopesWithIterableValueType = [];
 
 			$originalKeyVarExpr = null;
 			$continueExitPointHasUnoriginalKeyType = false;
 			if ($stmt->keyVar instanceof Variable && is_string($stmt->keyVar->name)) {
 				$originalKeyVarExpr = new OriginalForeachKeyExpr($stmt->keyVar->name);
-				if ($finalScope->hasExpressionType($originalKeyVarExpr)->yes()) {
+				if ($finalScope !== null && $finalScope->hasExpressionType($originalKeyVarExpr)->yes()) {
 					$scopesWithIterableValueType[] = $finalScope;
 				} else {
 					$continueExitPointHasUnoriginalKeyType = true;
@@ -1305,16 +1307,29 @@
 
 			foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
 				$continueScope = $continueExitPoint->getScope();
-				$finalScope = $continueScope->mergeWith($finalScope);
+				if ($finalScope === null) {
+					$finalScope = $continueScope;
+				} else {
+					$finalScope = $continueScope->mergeWith($finalScope);
+				}
 				if ($originalKeyVarExpr === null || !$continueScope->hasExpressionType($originalKeyVarExpr)->yes()) {
 					$continueExitPointHasUnoriginalKeyType = true;
 					continue;
 				}
 				$scopesWithIterableValueType[] = $continueScope;
 			}
 			$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
-			foreach ($breakExitPoints as $breakExitPoint) {
-				$finalScope = $breakExitPoint->getScope()->mergeWith($finalScope);
+			if ($finalScope !== null) {
+				foreach ($breakExitPoints as $breakExitPoint) {
+					$finalScope = $breakExitPoint->getScope()->mergeWith($finalScope);
+				}
+			} elseif (count($breakExitPoints) > 0) {
+				$finalScope = $breakExitPoints[0]->getScope();
+				for ($i = 1, $c = count($breakExitPoints); $i < $c; $i++) {
+					$finalScope = $breakExitPoints[$i]->getScope()->mergeWith($finalScope);
+				}
+			} else {
+				$finalScope = $finalScopeResult->getScope();
 			}
 
 			if ($unrolledEndScope !== null) {
@@ -1514,7 +1529,9 @@
 			$bodyScopeMaybeRan = $bodyScope;
 			$storage = $originalStorage;
 			$bodyScope = $this->processExprNode($stmt, $stmt->cond, $bodyScope, $storage, $nodeCallback, ExpressionContext::createDeep())->getTruthyScope();
-			$finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
+			$rawWhileBodyResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context);
+			$whileBodyAlwaysTerminated = $rawWhileBodyResult->isAlwaysTerminating();
+			$finalScopeResult = $rawWhileBodyResult->filterOutLoopExitPoints();
 			$finalScope = $finalScopeResult->getScope()->filterByFalseyValue($stmt->cond);
 
 			$alwaysIterates = false;
@@ -1532,7 +1549,7 @@
 
 			$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
 			if (count($breakExitPoints) > 0) {
-				$breakScope = $alwaysIterates ? null : $finalScope;
+				$breakScope = $alwaysIterates || $whileBodyAlwaysTerminated ? null : $finalScope;
 				foreach ($breakExitPoints as $breakExitPoint) {
 					$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
 				}
@@ -1610,7 +1627,9 @@
 			}
 
 			$storage = $originalStorage;
-			$bodyScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
+			$rawDoWhileBodyResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context);
+			$doWhileBodyAlwaysTerminated = $rawDoWhileBodyResult->isAlwaysTerminating();
+			$bodyScopeResult = $rawDoWhileBodyResult->filterOutLoopExitPoints();
 			$bodyScope = $bodyScopeResult->getScope();
 			foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
 				$bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope());
@@ -1645,7 +1664,7 @@
 
 			$breakExitPoints = $bodyScopeResult->getExitPointsByType(Break_::class);
 			if (count($breakExitPoints) > 0) {
-				$breakScope = $alwaysIterates ? null : $finalScope;
+				$breakScope = $alwaysIterates || $doWhileBodyAlwaysTerminated ? null : $finalScope;
 				foreach ($breakExitPoints as $breakExitPoint) {
 					$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
 				}
@@ -1744,7 +1763,9 @@
 				$bodyScope = $this->inferForLoopExpressions($stmt, $lastCondExpr, $bodyScope);
 			}
 
-			$finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
+			$rawForBodyResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context);
+			$forBodyAlwaysTerminated = $rawForBodyResult->isAlwaysTerminating();
+			$finalScopeResult = $rawForBodyResult->filterOutLoopExitPoints();
 			$finalScope = $finalScopeResult->getScope();
 			foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
 				$finalScope = $continueExitPoint->getScope()->mergeWith($finalScope);
@@ -1762,7 +1783,7 @@
 
 			$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
 			if (count($breakExitPoints) > 0) {
-				$breakScope = $alwaysIterates->yes() ? null : $finalScope;
+				$breakScope = $alwaysIterates->yes() || $forBodyAlwaysTerminated ? null : $finalScope;
 				foreach ($breakExitPoints as $breakExitPoint) {
 					$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
 				}
@@ -3921,6 +3942,7 @@
 		$chainScope = $originalScope;
 		$entryScopes = [];
 		$breakScopes = [];
+		$hasNormalCompletion = false;
 		foreach ($keyTypes as $i => $keyType) {
 			$valueType = $valueTypes[$i];
 			$isOptional = isset($optionalKeys[$i]);
@@ -3965,28 +3987,39 @@
 			$entryScopes[] = $iterScope;
 
 			$iterStorage = $originalStorage->duplicate();
-			$bodyResult = $this->processStmtNodesInternal(
+			$rawBodyResult = $this->processStmtNodesInternal(
 				$stmt,
 				$stmt->stmts,
 				$iterScope,
 				$iterStorage,
 				new NoopNodeCallback(),
 				$context->enterDeep(),
-			)->filterOutLoopExitPoints();
+			);
+			$bodyAlwaysTerminated = $rawBodyResult->isAlwaysTerminating();
+			$bodyResult = $rawBodyResult->filterOutLoopExitPoints();
 
-			$iterEndScope = $bodyResult->getScope();
+			$iterEndScope = $bodyAlwaysTerminated ? null : $bodyResult->getScope();
 			foreach ($bodyResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
-				$iterEndScope = $iterEndScope->mergeWith($continueExitPoint->getScope());
+				if ($iterEndScope === null) {
+					$iterEndScope = $continueExitPoint->getScope();
+				} else {
+					$iterEndScope = $iterEndScope->mergeWith($continueExitPoint->getScope());
+				}
 			}
 			foreach ($bodyResult->getExitPointsByType(Break_::class) as $breakExitPoint) {
 				$breakScopes[] = $breakExitPoint->getScope();
 			}
 
+			if ($iterEndScope === null) {
+				continue;
+			}
+
 			if ($isOptional) {
 				$chainScope = $iterEndScope->mergeWith($chainScope);
 			} else {
 				$chainScope = $iterEndScope;
 			}
+			$hasNormalCompletion = true;
 		}
 
 		$bodyScope = $entryScopes[0];
@@ -4002,8 +4035,15 @@
 			$bodyScope = $bodyScope->mergeWith($chainScope);
 		}
 
-		foreach ($breakScopes as $breakScope) {
-			$chainScope = $chainScope->mergeWith($breakScope);
+		if ($hasNormalCompletion) {
+			foreach ($breakScopes as $breakScope) {
+				$chainScope = $chainScope->mergeWith($breakScope);
+			}
+		} elseif (count($breakScopes) > 0) {
+			$chainScope = $breakScopes[0];
+			for ($i = 1, $c = count($breakScopes); $i < $c; $i++) {
+				$chainScope = $chainScope->mergeWith($breakScopes[$i]);
+			}
 		}
 
 		return ['bodyScope' => $bodyScope, 'endScope' => $chainScope];

tests/PHPStan/Analyser/nsrt/bug-1946.php

@@ -0,0 +1,186 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Bug1946;
+
+use function PHPStan\Testing\assertType;
+
+function foreachAllBranchesBreakWithAssignment(): void
+{
+	$tag = null;
+	foreach (["a", "b", "c"] as $tag) {
+		if ($tag === "a") {
+			$tag = null;
+			break;
+		} else {
+			$tag = null;
+			break;
+		}
+	}
+
+	assertType('null', $tag);
+}
+
+function foreachIfElseBreakDifferentTypes(): void
+{
+	$tag = null;
+	foreach (["a", "b", "c"] as $tag) {
+		if ($tag === "a") {
+			$tag = 1;
+			break;
+		} else {
+			$tag = 2;
+			break;
+		}
+	}
+
+	assertType('1|2', $tag);
+}
+
+function foreachAllBranchesReturn(): void
+{
+	$tag = null;
+	foreach (["a", "b", "c"] as $tag) {
+		if ($tag === "a") {
+			$tag = null;
+			return;
+		} else {
+			$tag = null;
+			return;
+		}
+	}
+
+	assertType('null', $tag);
+}
+
+function foreachOnlyIfBreaksNoElse(): void
+{
+	$tag = null;
+	foreach (["a", "b", "c"] as $tag) {
+		if ($tag === "a") {
+			$tag = null;
+			break;
+		}
+	}
+
+	assertType("'c'|null", $tag);
+}
+
+/**
+ * @param string[] $arr
+ */
+function foreachNonConstantArrayAllBreak(array $arr): void
+{
+	$tag = null;
+	foreach ($arr as $tag) {
+		if ($tag === "a") {
+			$tag = null;
+			break;
+		} else {
+			$tag = null;
+			break;
+		}
+	}
+
+	assertType('null', $tag);
+}
+
+function foreachElseIfAllBreak(): void
+{
+	$tag = null;
+	foreach (["a", "b", "c"] as $tag) {
+		if ($tag === "a") {
+			$tag = 1;
+			break;
+		} elseif ($tag === "b") {
+			$tag = 2;
+			break;
+		} else {
+			$tag = 3;
+			break;
+		}
+	}
+
+	assertType('1|2|3', $tag);
+}
+
+function foreachBreakWithContinue(): void
+{
+	$tag = null;
+	foreach (["a", "b", "c"] as $tag) {
+		if ($tag === "a") {
+			$tag = null;
+			continue;
+		} else {
+			$tag = null;
+			break;
+		}
+	}
+
+	assertType('null', $tag);
+}
+
+function whileAllBreakMayNotIterate(): void
+{
+	$x = 'hello';
+	while (rand(0, 1)) {
+		if (rand(0, 1)) {
+			$x = 1;
+			break;
+		} else {
+			$x = 2;
+			break;
+		}
+	}
+
+	assertType("1|2|'hello'", $x);
+}
+
+function whileTrueAllBreak(): void
+{
+	$x = 'hello';
+	while (true) {
+		if (rand(0, 1)) {
+			$x = 1;
+			break;
+		} else {
+			$x = 2;
+			break;
+		}
+	}
+
+	assertType('1|2', $x);
+}
+
+function doWhileAllBreak(): void
+{
+	$x = 'hello';
+	do {
+		if (rand(0, 1)) {
+			$x = 1;
+			break;
+		} else {
+			$x = 2;
+			break;
+		}
+	} while (rand(0, 1));
+
+	assertType('1|2', $x);
+}
+
+function forAllBreakAlwaysIterates(): void
+{
+	$x = 'hello';
+	for ($i = 0; $i < 10; $i++) {
+		if (rand(0, 1)) {
+			$x = 1;
+			break;
+		} else {
+			$x = 2;
+			break;
+		}
+	}
+
+	assertType('1|2', $x);
+}

tests/PHPStan/Analyser/nsrt/for-loop-i-type.php

@@ -59,7 +59,7 @@ public function doLOrem() {
 			break;
 		}
 
-		assertType('int<1, max>', $i);
+		assertType('int<1, 49>', $i);
 	}
 
 }