Record try-path definedness as conditional on the catch variable being undefined

At a try/catch join where the catch variable is untracked on the non-catch
path and certainly defined at the end of the catch body, its later
definedness tells the joined paths apart. Record conditional expression
holders with a certainty-No (and, for scopes where any variable can exist,
a maybe-defined-null) condition on the catch variable, restoring the
non-catch certainty and type of variables the join demoted to
maybe-defined.

To let such holders fire, the isset()-style falsey specification in
MutatingScope::applySpecifiedTypes() now publishes the certainty change
into the specified-expressions batch - rescuing the waiting holders around
unsetExpression()'s invalidation, which would otherwise drop them at the
very specification they wait for. A second catch clause merging into the
join drops the holders via the conditional-expressions intersection, so
multi-catch stays conservative.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA

Author: ondrejmirtes

src/Analyser/MutatingScope.php

@@ -4092,12 +4092,37 @@ public function applySpecifiedTypes(SpecifiedTypes $specifiedTypes): self
 				$expr = $issetExpr->getExpr();
 
 				if ($typeSpecification['sure']) {
+					$innerExprString = $scope->getNodeKey($expr);
 					$scope = $scope->setExpressionCertaintyKeepingType(
 						$expr,
 						TrinaryLogic::createMaybe(),
 					);
+					$specifiedExpressions[$innerExprString] = ExpressionTypeHolder::createMaybe(
+						$expr,
+						$scope->expressionTypes[$innerExprString]->getType(),
+					);
 				} else {
+					$innerExprString = $scope->getNodeKey($expr);
+					// Holders conditioned on this expression being undefined (a
+					// certainty-No condition) wait for exactly the specification
+					// applied here, but unsetExpression()'s invalidation would drop
+					// them before the conditional-expressions matcher below could
+					// fire them - carve them out and re-add them afterwards.
+					$rescuedHolders = [];
+					foreach ($scope->conditionalExpressions as $targetExprString => $targetHolders) {
+						foreach ($targetHolders as $holderKey => $conditionalHolder) {
+							$conditionHolder = $conditionalHolder->getConditionExpressionTypeHolders()[$innerExprString] ?? null;
+							if ($conditionHolder === null || !$conditionHolder->getCertainty()->no()) {
+								continue;
+							}
+							$rescuedHolders[$targetExprString][$holderKey] = $conditionalHolder;
+						}
+					}
 					$scope = $scope->unsetExpression($expr);
+					foreach ($rescuedHolders as $targetExprString => $targetHolders) {
+						$scope = $scope->addConditionalExpressions((string) $targetExprString, $targetHolders);
+					}
+					$specifiedExpressions[$innerExprString] = new ExpressionTypeHolder($expr, new ErrorType(), TrinaryLogic::createNo());
 				}
 				$scopeIsWorkingCopy = false;
 

src/Analyser/StmtHandler/TryCatchHandler.php

@@ -8,7 +8,10 @@
 use PhpParser\Node\Expr;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\TryCatch;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\ConditionalExpressionHolder;
 use PHPStan\Analyser\ExpressionResultStorage;
+use PHPStan\Analyser\ExpressionTypeHolder;
 use PHPStan\Analyser\InternalStatementExitPoint;
 use PHPStan\Analyser\InternalStatementResult;
 use PHPStan\Analyser\InternalThrowPoint;
@@ -26,7 +29,10 @@
 use PHPStan\Node\Variable\VariableWrite;
 use PHPStan\Node\VariableAssignNode;
 use PHPStan\ShouldNotHappenException;
+use PHPStan\TrinaryLogic;
+use PHPStan\Type\ErrorType;
 use PHPStan\Type\NeverType;
+use PHPStan\Type\NullType;
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\TypeCombinator;
 use Throwable;
@@ -239,7 +245,13 @@ public function processStmt(
 			$catchScopeForFinally = $catchScopeResult->getScope();
 			$catchFlows[] = [$originalCatchType, VariableFlow::sequence($catchNode->var !== null ? VariableFlowBuilder::targetWrite($catchNode->var, VariableWrite::KIND_CATCH, $catchScopeForFinally, $storage) : null, $catchScopeResult->getVariableFlow())];
 
-			$finalScope = $catchScopeResult->isAlwaysTerminating() ? $finalScope : $catchScopeResult->getScope()->mergeWith($finalScope);
+			if (!$catchScopeResult->isAlwaysTerminating()) {
+				$mergedScope = $catchScopeResult->getScope()->mergeWith($finalScope);
+				if ($variableName !== null && $finalScope !== null) {
+					$mergedScope = $this->addCatchVariableDefinednessConditionals($mergedScope, $finalScope, $catchScopeResult->getScope(), $variableName);
+				}
+				$finalScope = $mergedScope;
+			}
 			$alwaysTerminating = $alwaysTerminating && $catchScopeResult->isAlwaysTerminating();
 			$hasYield = $hasYield || $catchScopeResult->hasYield();
 			$catchThrowPoints = $catchScopeResult->getThrowPoints();
@@ -323,4 +335,70 @@ public function processStmt(
 		return new InternalStatementResult($finalScope, hasYield: $hasYield, isAlwaysTerminating: $alwaysTerminating, exitPoints: $exitPoints, throwPoints: array_merge($throwPoints, $throwPointsForLater), impurePoints: $impurePoints, variableFlow: VariableFlow::tryCatch($branchScopeResult->getVariableFlow(), $catchFlows, $finallyFlow));
 	}
 
+	/**
+	 * The catch variable's definedness after the try/catch tells the two joined
+	 * paths apart: when it is untracked on the non-catch path and certainly
+	 * defined at the end of the catch body, "the catch variable is undefined"
+	 * later implies the non-catch path ran. Record that as conditional
+	 * expression holders with a certainty-No condition on the catch variable,
+	 * restoring the non-catch certainty (and type) of variables the join
+	 * demoted to maybe-defined - e.g. `isset($e) || $var instanceof \DateTime`
+	 * evaluates `$var` only where `$e` is narrowed away.
+	 */
+	private function addCatchVariableDefinednessConditionals(
+		MutatingScope $mergedScope,
+		MutatingScope $nonCatchScope,
+		MutatingScope $catchEndScope,
+		string $variableName,
+	): MutatingScope
+	{
+		$variableExprString = '$' . $variableName;
+		if (isset($nonCatchScope->expressionTypes[$variableExprString])) {
+			return $mergedScope;
+		}
+
+		$catchVariableHolder = $catchEndScope->expressionTypes[$variableExprString] ?? null;
+		if ($catchVariableHolder === null || !$catchVariableHolder->getCertainty()->yes()) {
+			return $mergedScope;
+		}
+
+		$conditions = [
+			[
+				$variableExprString => new ExpressionTypeHolder(new Variable($variableName), new ErrorType(), TrinaryLogic::createNo()),
+			],
+		];
+		if ($catchVariableHolder->getType()->isNull()->no()) {
+			// In a scope where any variable can exist (e.g. the top level), the
+			// isset() machinery models "!isset($e)" on a maybe-defined variable as
+			// "maybe defined, null when defined" instead of unsetting it. That
+			// state excludes the catch path just the same - it guarantees a
+			// defined, non-null catch variable.
+			$conditions[] = [
+				$variableExprString => ExpressionTypeHolder::createMaybe(new Variable($variableName), new NullType()),
+			];
+		}
+		foreach ($nonCatchScope->expressionTypes as $exprString => $holder) {
+			if (!$holder->getCertainty()->yes()) {
+				continue;
+			}
+			$expr = $holder->getExpr();
+			if (!$expr instanceof Variable || !is_string($expr->name)) {
+				continue;
+			}
+			$mergedHolder = $mergedScope->expressionTypes[$exprString] ?? null;
+			if ($mergedHolder === null || !$mergedHolder->getCertainty()->maybe()) {
+				continue;
+			}
+
+			$conditionalHolders = [];
+			foreach ($conditions as $condition) {
+				$conditionalHolder = new ConditionalExpressionHolder($condition, $holder);
+				$conditionalHolders[$conditionalHolder->getKey()] = $conditionalHolder;
+			}
+			$mergedScope = $mergedScope->addConditionalExpressions((string) $exprString, $conditionalHolders);
+		}
+
+		return $mergedScope;
+	}
+
 }

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

@@ -0,0 +1,23 @@
+<?php declare(strict_types = 1);
+
+namespace Bug6608;
+
+use PHPStan\TrinaryLogic;
+use function PHPStan\Testing\assertType;
+use function PHPStan\Testing\assertVariableCertainty;
+
+function (string $s): void {
+	try {
+		$var = new \DateTime($s);
+	} catch (\Throwable $e) {}
+
+	if (isset($e) || $var instanceof \DateTime) {
+	}
+
+	if (!isset($e)) {
+		assertVariableCertainty(TrinaryLogic::createYes(), $var);
+		assertType('DateTime', $var);
+	} else {
+		assertVariableCertainty(TrinaryLogic::createMaybe(), $var);
+	}
+};

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

@@ -1777,4 +1777,13 @@ public function testBug11109(): void
 		$this->analyse([__DIR__ . '/data/bug-11109.php'], []);
 	}
 
+	public function testBug6608(): void
+	{
+		$this->cliArgumentsVariablesRegistered = true;
+		$this->polluteScopeWithLoopInitialAssignments = false;
+		$this->checkMaybeUndefinedVariables = true;
+		$this->polluteScopeWithAlwaysIterableForeach = true;
+		$this->analyse([__DIR__ . '/data/bug-6608.php'], []);
+	}
+
 }

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

@@ -0,0 +1,8 @@
+<?php declare(strict_types = 1);
+
+try {
+	$var = new \DateTime('nope');
+} catch (\Throwable $e) {}
+
+if (isset($e) || $var instanceof \DateTime) {
+}