Record conditional definedness guards when merging if branches with a side-effect-free condition

src/Analyser/MutatingScope.php

@@ -3584,7 +3584,7 @@ public function isInFirstLevelStatement(): bool
 		return $this->inFirstLevelStatement;
 	}
 
-	public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = false): self
+	public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = false, bool $createConditionalDefinedness = false): self
 	{
 		if ($otherScope === null || $this === $otherScope) {
 			return $this;
@@ -3618,6 +3618,20 @@ public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals =
 			$ourExpressionTypes,
 			$mergedExpressionTypes,
 		);
+		if ($createConditionalDefinedness) {
+			$conditionalExpressions = $this->createConditionalDefinednessExpressions(
+				$conditionalExpressions,
+				$ourExpressionTypes,
+				$theirExpressionTypes,
+				$mergedExpressionTypes,
+			);
+			$conditionalExpressions = $this->createConditionalDefinednessExpressions(
+				$conditionalExpressions,
+				$theirExpressionTypes,
+				$ourExpressionTypes,
+				$mergedExpressionTypes,
+			);
+		}
 
 		$filter = static function (ExpressionTypeHolder $expressionTypeHolder) {
 			if ($expressionTypeHolder->getCertainty()->yes()) {
@@ -3900,6 +3914,96 @@ private function createConditionalExpressions(
 		return $conditionalExpressions;
 	}
 
+	/**
+	 * Records that a variable defined only in our branch is defined again
+	 * whenever the same branch condition holds — so a later `if` reusing that
+	 * condition does not report the variable as possibly undefined.
+	 *
+	 * The branch condition is reconstructed from the expressions that the
+	 * condition narrowed in our branch:
+	 * - "type guards": expressions defined in both branches whose type in our
+	 *   branch differs from the merged type (e.g. `$s` narrowed to `'banana'`
+	 *   by `$s === 'banana'`, or an array gaining a `hasOffset` accessory), and
+	 * - "existence guards": expressions made certain (Yes) in our branch but not
+	 *   in the other one (e.g. `$a['x']` made certain by `isset($a['x'])`).
+	 *
+	 * All guards are combined into a single condition, so re-establishing only
+	 * part of it later (`if ($a && $b)` then `if ($a)`) does not wrongly imply
+	 * the variable is defined. At least one type guard is required, which keeps
+	 * out spurious guards produced when the other branch's scope tracks nothing
+	 * for an expression (e.g. an untracked superglobal access).
+	 *
+	 * @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
+	 * @param array<string, ExpressionTypeHolder> $ourExpressionTypes
+	 * @param array<string, ExpressionTypeHolder> $theirExpressionTypes
+	 * @param array<string, ExpressionTypeHolder> $mergedExpressionTypes
+	 * @return array<string, ConditionalExpressionHolder[]>
+	 */
+	private function createConditionalDefinednessExpressions(
+		array $conditionalExpressions,
+		array $ourExpressionTypes,
+		array $theirExpressionTypes,
+		array $mergedExpressionTypes,
+	): array
+	{
+		$guards = [];
+		$hasTypeGuard = false;
+		$newlyDefined = [];
+		foreach ($ourExpressionTypes as $exprString => $holder) {
+			if ($holder->getExpr() instanceof VirtualNode) {
+				continue;
+			}
+			if (!$holder->getCertainty()->yes()) {
+				continue;
+			}
+			if (!array_key_exists($exprString, $mergedExpressionTypes)) {
+				continue;
+			}
+
+			$theirHolder = $theirExpressionTypes[$exprString] ?? null;
+			if ($theirHolder === null || !$theirHolder->getCertainty()->yes()) {
+				$guards[$exprString] = $holder;
+
+				$expr = $holder->getExpr();
+				if (
+					$theirHolder === null
+					&& $expr instanceof Variable
+					&& is_string($expr->name)
+					&& !$this->isGlobalVariable($expr->name)
+				) {
+					$newlyDefined[$exprString] = $holder;
+				}
+
+				continue;
+			}
+
+			if ($mergedExpressionTypes[$exprString]->equalTypes($holder)) {
+				continue;
+			}
+
+			$guards[$exprString] = $holder;
+			$hasTypeGuard = true;
+		}
+
+		if (!$hasTypeGuard || count($newlyDefined) === 0) {
+			return $conditionalExpressions;
+		}
+
+		foreach ($newlyDefined as $exprString => $holder) {
+			$variableGuards = $guards;
+			unset($variableGuards[$exprString]);
+
+			if (count($variableGuards) === 0) {
+				continue;
+			}
+
+			$conditionalExpression = new ConditionalExpressionHolder($variableGuards, $holder);
+			$conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression;
+		}
+
+		return $conditionalExpressions;
+	}
+
 	/**
 	 * @param array<string, ExpressionTypeHolder> $ourVariableTypeHolders
 	 * @param array<string, ExpressionTypeHolder> $theirVariableTypeHolders

src/Analyser/NodeScopeResolver.php

@@ -1311,6 +1311,13 @@ public function processStmtNode(
 			$alwaysTerminating = true;
 			$hasYield = $condResult->hasYield();
 
+			// Only a deterministic (side-effect-free) condition guarantees that a
+			// later `if` with the same condition picks the same branch, so that a
+			// variable defined here is defined there too. Elseif chains rebind the
+			// condition and are left to the regular per-branch tracking.
+			$createConditionalDefinedness = count($condResult->getImpurePoints()) === 0
+				&& count($stmt->elseifs) === 0;
+
 			$branchScopeStatementResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $condResult->getTruthyScope(), $storage, $nodeCallback, $context);
 
 			if (!$conditionType->isTrue()->no()) {
@@ -1375,7 +1382,7 @@ public function processStmtNode(
 
 			if ($stmt->else === null) {
 				if (!$ifAlwaysTrue && !$lastElseIfConditionIsTrue) {
-					$finalScope = $scope->mergeWith($finalScope, true);
+					$finalScope = $scope->mergeWith($finalScope, true, $createConditionalDefinedness);
 					$alwaysTerminating = false;
 				}
 			} else {
@@ -1387,7 +1394,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, true);
+					$finalScope = $branchScopeStatementResult->isAlwaysTerminating() ? $finalScope : $branchScope->mergeWith($finalScope, true, $createConditionalDefinedness);
 					$alwaysTerminating = $alwaysTerminating && $branchScopeStatementResult->isAlwaysTerminating();
 					if (count($branchScopeStatementResult->getEndStatements()) > 0) {
 						$endStatements = array_merge($endStatements, $branchScopeStatementResult->getEndStatements());

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

@@ -0,0 +1,130 @@
+<?php declare(strict_types = 1);
+
+namespace Bug7718;
+
+use PHPStan\TrinaryLogic;
+use function PHPStan\Testing\assertVariableCertainty;
+
+/**
+ * @param array<string, int> $elements
+ */
+function isset_twice(array $elements): void
+{
+	if (isset($elements['a'])) {
+		$a = 1;
+	}
+
+	if (isset($elements['a'])) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $a);
+	}
+}
+
+function strict_compare_twice(string $s): void
+{
+	if ($s === 'banana') {
+		$bananas = 1;
+	}
+
+	if ($s === 'banana') {
+		assertVariableCertainty(TrinaryLogic::createYes(), $bananas);
+	}
+}
+
+function int_compare_twice(int $x): void
+{
+	if ($x === 1) {
+		$a = 1;
+	}
+
+	if ($x === 1) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $a);
+	}
+}
+
+/**
+ * @param array<string, int> $elements
+ */
+function array_key_exists_twice(array $elements): void
+{
+	if (array_key_exists('a', $elements)) {
+		$a = 1;
+	}
+
+	if (array_key_exists('a', $elements)) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $a);
+	}
+}
+
+function instanceof_twice(mixed $x): void
+{
+	if ($x instanceof \Throwable) {
+		$a = 1;
+	}
+
+	if ($x instanceof \Throwable) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $a);
+	}
+}
+
+function not_null_twice(?int $x): void
+{
+	if ($x !== null) {
+		$a = 1;
+	}
+
+	if ($x !== null) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $a);
+	}
+}
+
+// A different condition must NOT prove the variable defined.
+function different_condition(string $s): void
+{
+	if ($s === 'a') {
+		$a = 1;
+	}
+
+	if ($s === 'b') {
+		assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
+	}
+}
+
+/**
+ * @param array<string, int> $elements
+ */
+function isset_then_else(array $elements): void
+{
+	if (isset($elements['a'])) {
+		$a = 1;
+	} else {
+		$a = 2;
+	}
+
+	if (isset($elements['a'])) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $a);
+	}
+}
+
+// Reusing only part of an `&&` condition must NOT prove the variable defined.
+function partial_condition_reuse(int $x, int $y): void
+{
+	if ($x === 1 && $y === 2) {
+		$a = 1;
+	}
+
+	if ($x === 1) {
+		assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
+	}
+}
+
+// A non-deterministic condition must NOT prove the variable defined when reused.
+function impure_condition(int $x): void
+{
+	if (rand(0, 1) === 1 && $x === 1) {
+		$v = 1;
+	}
+
+	if ($x === 1) {
+		assertVariableCertainty(TrinaryLogic::createMaybe(), $v);
+	}
+}