Fix phpstan/phpstan#4173: Consequest scope with the same condition

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

@@ -3230,7 +3230,7 @@
 			} 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 = [];
@@ -3362,7 +3362,7 @@
 		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;
@@ -3372,6 +3372,18 @@
 
 		$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,
@@ -3477,6 +3489,48 @@
 		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 @@
 			}
 		}
 
-		if ($assertions === null || $assertions->getAll() === [] || $parametersAcceptor === null) {
+		if ($assertions === null || $assertions->getAll() === []) {
 			return null;
 		}
 
@@ -2236,7 +2236,7 @@
 			&& !$expr->name instanceof Name
 		) {
 			$nameType = $scope->getType($expr->name);
 			if ($nameType->isCallable()->yes()) {
 				$isPure = null;
 				foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
 					$variantIsPure = $variant->isPure();
@@ -2244,7 +2244,7 @@
 				}
 
 				if ($isPure !== null) {
 					if ($isPure->no()) {
 						return new SpecifiedTypes([], []);
 					}
 

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;
 	}