Fix #14411 without reverting PR phpstan#5056 preserveVacuousConditionals

Restore preserveVacuousConditionals from PR phpstan#5056 and add a targeted
guard overlap check in createConditionalExpressions() to prevent
creating ambiguous conditional expressions when the guard type overlaps
with the other branch's type for the guard variable.

This preserves the improved type narrowing from phpstan#5056 (bug phpstan#4173,
#12992, #14227) while also fixing the incorrect dependent type
narrowing reported in #14411.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: phpstan-bot

src/Analyser/ExprHandler/MethodCallHandler.php

@@ -153,24 +153,22 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		$scope = $argsResult->getScope();
 
 		if ($methodReflection !== null) {
-			if ($parametersAcceptor !== null) {
-				$methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
-				if ($methodThrowPoint !== null) {
-					$throwPoints[] = $methodThrowPoint;
-				}
+			$methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
+			if ($methodThrowPoint !== null) {
+				$throwPoints[] = $methodThrowPoint;
 			}
 
 			if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) {
 				$nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage);
 				$scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass());
-			} elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin() && $parametersAcceptor !== null) {
+			} elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin()) {
 				$scope = $scope->assignExpression(
 					new PossiblyImpureCallExpr($normalizedExpr, $normalizedExpr->var, sprintf('%s::%s()', $methodReflection->getDeclaringClass()->getDisplayName(), $methodReflection->getName())),
 					$parametersAcceptor->getReturnType(),
 					new MixedType(),
 				);
 			}
-			if ($parametersAcceptor !== null && !$methodReflection->isStatic()) {
+			if (!$methodReflection->isStatic()) {
 				$selfOutType = $methodReflection->getSelfOutType();
 				if ($selfOutType !== null) {
 					$scope = $scope->assignExpression(

src/Analyser/ExprHandler/StaticCallHandler.php

@@ -198,7 +198,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		$scope = $argsResult->getScope();
 		$scopeFunction = $scope->getFunction();
 
-		if ($methodReflection !== null && $parametersAcceptor !== null) {
+		if ($methodReflection !== null) {
 			$methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
 			if ($methodThrowPoint !== null) {
 				$throwPoints[] = $methodThrowPoint;
@@ -221,7 +221,6 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
 		} elseif (
 			$methodReflection !== null
 			&& $this->rememberPossiblyImpureFunctionValues
-			&& $parametersAcceptor !== null
 			&& $scope->isInClass()
 			&& $scope->getClassReflection()->is($methodReflection->getDeclaringClass()->getName())
 			&& $methodReflection->hasSideEffects()->maybe()

src/Analyser/MutatingScope.php

@@ -3205,7 +3205,7 @@ public function filterBySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
 			} else {
 				$scope = $scope->removeTypeFromExpression($expr, $type);
 			}
-			$specifiedExpressions[$typeSpecification['exprString']] = ExpressionTypeHolder::createYes($expr, $scope->getType($expr));
+			$specifiedExpressions[$typeSpecification['exprString']] = ExpressionTypeHolder::createYes($expr, $scope->getScopeType($expr));
 		}
 
 		$conditions = [];
@@ -3337,7 +3337,7 @@ public function isInFirstLevelStatement(): bool
 		return $this->inFirstLevelStatement;
 	}
 
-	public function mergeWith(?self $otherScope): self
+	public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = false): self
 	{
 		if ($otherScope === null || $this === $otherScope) {
 			return $this;
@@ -3347,6 +3347,18 @@ public function mergeWith(?self $otherScope): self
 
 		$mergedExpressionTypes = $this->mergeVariableHolders($ourExpressionTypes, $theirExpressionTypes);
 		$conditionalExpressions = $this->intersectConditionalExpressions($otherScope->conditionalExpressions);
+		if ($preserveVacuousConditionals) {
+			$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
+				$conditionalExpressions,
+				$this->conditionalExpressions,
+				$theirExpressionTypes,
+			);
+			$conditionalExpressions = $this->preserveVacuousConditionalExpressions(
+				$conditionalExpressions,
+				$otherScope->conditionalExpressions,
+				$ourExpressionTypes,
+			);
+		}
 		$conditionalExpressions = $this->createConditionalExpressions(
 			$conditionalExpressions,
 			$ourExpressionTypes,
@@ -3452,6 +3464,48 @@ private function intersectConditionalExpressions(array $otherConditionalExpressi
 		return $newConditionalExpressions;
 	}
 
+	/**
+	 * @param array<string, ConditionalExpressionHolder[]> $currentConditionalExpressions
+	 * @param array<string, ConditionalExpressionHolder[]> $sourceConditionalExpressions
+	 * @param array<string, ExpressionTypeHolder> $otherExpressionTypes
+	 * @return array<string, ConditionalExpressionHolder[]>
+	 */
+	private function preserveVacuousConditionalExpressions(
+		array $currentConditionalExpressions,
+		array $sourceConditionalExpressions,
+		array $otherExpressionTypes,
+	): array
+	{
+		foreach ($sourceConditionalExpressions as $exprString => $holders) {
+			foreach ($holders as $key => $holder) {
+				if (isset($currentConditionalExpressions[$exprString][$key])) {
+					continue;
+				}
+
+				$typeHolder = $holder->getTypeHolder();
+				if ($typeHolder->getCertainty()->no() && !$typeHolder->getExpr() instanceof Variable) {
+					continue;
+				}
+
+				foreach ($holder->getConditionExpressionTypeHolders() as $guardExprString => $guardTypeHolder) {
+					if (!array_key_exists($guardExprString, $otherExpressionTypes)) {
+						continue;
+					}
+
+					$otherType = $otherExpressionTypes[$guardExprString]->getType();
+					$guardType = $guardTypeHolder->getType();
+
+					if ($otherType->isSuperTypeOf($guardType)->no()) {
+						$currentConditionalExpressions[$exprString][$key] = $holder;
+						break;
+					}
+				}
+			}
+		}
+
+		return $currentConditionalExpressions;
+	}
+
 	/**
 	 * @param array<string, ConditionalExpressionHolder[]> $newConditionalExpressions
 	 * @param array<string, ConditionalExpressionHolder[]> $existingConditionalExpressions

src/Analyser/NodeScopeResolver.php

@@ -1146,7 +1146,7 @@ public function processStmtNode(
 					$throwPoints = array_merge($throwPoints, $branchScopeStatementResult->getThrowPoints());
 					$impurePoints = array_merge($impurePoints, $branchScopeStatementResult->getImpurePoints());
 					$branchScope = $branchScopeStatementResult->getScope();
-					$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope);
+					$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope, true);
 					$alwaysTerminating = $alwaysTerminating && $branchScopeStatementResult->isAlwaysTerminating();
 					if (count($branchScopeStatementResult->getEndStatements()) > 0) {
 						$endStatements = array_merge($endStatements, $branchScopeStatementResult->getEndStatements());
@@ -1170,7 +1170,7 @@ public function processStmtNode(
 
 			if ($stmt->else === null) {
 				if (!$ifAlwaysTrue && !$lastElseIfConditionIsTrue) {
-					$finalScope = $scope->mergeWith($finalScope);
+					$finalScope = $scope->mergeWith($finalScope, true);
 					$alwaysTerminating = false;
 				}
 			} else {
@@ -1182,7 +1182,7 @@ public function processStmtNode(
 					$throwPoints = array_merge($throwPoints, $branchScopeStatementResult->getThrowPoints());
 					$impurePoints = array_merge($impurePoints, $branchScopeStatementResult->getImpurePoints());
 					$branchScope = $branchScopeStatementResult->getScope();
-					$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope);
+					$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope, true);
 					$alwaysTerminating = $alwaysTerminating && $branchScopeStatementResult->isAlwaysTerminating();
 					if (count($branchScopeStatementResult->getEndStatements()) > 0) {
 						$endStatements = array_merge($endStatements, $branchScopeStatementResult->getEndStatements());

src/Analyser/TypeSpecifier.php

@@ -1896,7 +1896,7 @@ private function specifyTypesFromCallableCall(TypeSpecifierContext $context, Fun
 			}
 		}
 
-		if ($assertions === null || $assertions->getAll() === [] || $parametersAcceptor === null) {
+		if ($assertions === null || $assertions->getAll() === []) {
 			return null;
 		}
 

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

@@ -59,7 +59,7 @@ public function testWithBooleans($data): void
 			assertType('1|2|3|10', $data);
 			assertType('bool', $update);
 		} else {
-			assertType('1|2|3|10', $data);
+			assertType('1|2', $data);
 			assertType('bool', $update);
 		}
 
@@ -81,15 +81,15 @@ public function testWithBooleans($data): void
 
 		if ($data === 3) {
 			assertType('bool', $update);
-			assertType('bool', $foo);
+			assertType('true', $foo);
 		} else {
 			assertType('bool', $update);
 			assertType('bool', $foo);
 		}
 
 		if ($data === 1 || $data === 2) {
 			assertType('bool', $update);
-			assertType('bool', $foo);
+			assertType('false', $foo);
 		} else {
 			assertType('bool', $update);
 			assertType('bool', $foo);

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

@@ -911,12 +911,7 @@ public function testBug4173(): void
 		$this->polluteScopeWithLoopInitialAssignments = false;
 		$this->checkMaybeUndefinedVariables = true;
 		$this->polluteScopeWithAlwaysIterableForeach = true;
-		$this->analyse([__DIR__ . '/data/bug-4173.php'], [
-			[
-				'Variable $value might not be defined.', // could be fixed
-				30,
-			],
-		]);
+		$this->analyse([__DIR__ . '/data/bug-4173.php'], []);
 	}
 
 	public function testBug5805(): void
@@ -1119,29 +1114,13 @@ public function testDynamicAccess(): void
 				18,
 			],
 			[
-				'Variable $foo might not be defined.',
-				36,
-			],
-			[
-				'Variable $foo might not be defined.',
-				37,
-			],
-			[
-				'Variable $bar might not be defined.',
+				'Undefined variable: $bar',
 				38,
 			],
 			[
-				'Variable $bar might not be defined.',
-				40,
-			],
-			[
-				'Variable $foo might not be defined.',
+				'Undefined variable: $foo',
 				41,
 			],
-			[
-				'Variable $bar might not be defined.',
-				42,
-			],
 			[
 				'Undefined variable: $buz',
 				44,
@@ -1158,14 +1137,6 @@ public function testDynamicAccess(): void
 				'Undefined variable: $buz',
 				49,
 			],
-			[
-				'Variable $bar might not be defined.',
-				49,
-			],
-			[
-				'Variable $foo might not be defined.',
-				49,
-			],
 			[
 				'Variable $foo might not be defined.',
 				50,
@@ -1487,6 +1458,24 @@ public function testBug13920(): void
 		$this->analyse([__DIR__ . '/data/bug-13920.php'], []);
 	}
 
+	public function testBug12992(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = false;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+		$this->analyse([__DIR__ . '/data/bug-12992.php'], []);
+	}
+
+	public function testBug14227(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = false;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+		$this->analyse([__DIR__ . '/data/bug-14227.php'], []);
+	}
+
 	public function testBug14117(): void
 	{
 		$this->cliArgumentsVariablesRegistered = true;
@@ -1495,10 +1484,6 @@ public function testBug14117(): void
 		$this->polluteScopeWithAlwaysIterableForeach = true;
 
 		$this->analyse([__DIR__ . '/data/bug-14117.php'], [
-			[
-				'Variable $value might not be defined.',
-				33,
-			],
 			[
 				'Variable $value might not be defined.',
 				49,

tests/PHPStan/Rules/Variables/data/bug-12992.php

@@ -0,0 +1,40 @@
+<?php declare(strict_types = 1);
+
+namespace Bug12992;
+
+class HelloWorld
+{
+	public function test(\Closure $fx): void
+	{
+		$capture = tmpfile();
+		if ($capture !== false) {
+			$capturePath = stream_get_meta_data($capture)['uri'] ?? '';
+			if (@is_writable($capturePath)) {
+				$errorLogPrevious = ini_set('error_log', $capturePath);
+			} else {
+				$capture = false;
+			}
+		}
+
+		if ($capture !== false) {
+			fclose($capture);
+
+			ini_set('error_log', $errorLogPrevious);
+		}
+	}
+}
+
+function test(bool $v): void
+{
+	if ($v) {
+	    if (rand() === 3) {
+	        $newvar = 1;
+	    } else {
+	        $v = false;
+	    }
+	}
+
+    if ($v === true) {
+        echo $newvar;
+    }
+}

tests/PHPStan/Rules/Variables/data/bug-14227.php

@@ -0,0 +1,19 @@
+<?php declare(strict_types = 1);
+
+namespace Bug14227;
+
+function moo(): void {
+	$key = rand(0, 2);
+
+	if ($key === 1) {
+		$value = 'test';
+	}
+
+	if ($key === 2) {
+		unset($value);
+	}
+
+	if ($key === 1) {
+		echo $value;
+	}
+}

tests/PHPStan/Rules/Variables/data/dynamic-access.php

@@ -35,18 +35,18 @@ public function testScope(): void
 		if ($name === 'foo') {
 			echo $$name; // ok
 			echo $foo; // ok
-			echo $bar;
+			echo $bar; // not ok
 		} elseif ($name === 'bar') {
 			echo $$name; // ok
-			echo $foo;
+			echo $foo; // not ok
 			echo $bar; // ok
 		} else {
-			echo $$name; // ok
-			echo $foo;
-			echo $bar;
+			echo $$name; // not ok
+			echo $foo; // not ok
+			echo $bar; // not ok
 		}
 
-		echo $$name; // ok
+		echo $$name; // ok for foo and bar but not buz
 		echo $foo;
 		echo $bar;
 	}