Narrow the falsey branch of isset($a['x']['y']) only when the intermediate offset is known to exist

src/Analyser/ExprHandler/IssetHandler.php

@@ -39,6 +39,7 @@
 use PHPStan\Type\Constant\ConstantStringType;
 use PHPStan\Type\IntersectionType;
 use PHPStan\Type\MixedType;
+use PHPStan\Type\NeverType;
 use PHPStan\Type\NullType;
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\ObjectWithoutClassType;
@@ -212,25 +213,59 @@
 						if ($typesToRemove !== []) {
 							$typeToRemove = TypeCombinator::union(...$typesToRemove);
 
-							$result = $typeSpecifier->create(
-								$issetExpr->var,
-								$typeToRemove,
-								TypeSpecifierContext::createFalse(),
-								$scope,
-							)->setRootExpr($expr);
-
-							if ($scope->hasExpressionType($issetExpr->var)->maybe()) {
-								$result = $result->unionWith(
-									$typeSpecifier->create(
-										new IssetExpr($issetExpr->var),
-										new NullType(),
-										TypeSpecifierContext::createTruthy(),
-										$scope,
-									)->setRootExpr($expr),
-								);
+							// isset() can also be false because $issetExpr->var itself does not exist.
+							// Narrowing its type is only sound when it's known to exist, or when its
+							// certainty can be degraded to "maybe" alongside the narrowing.
+							if (
+								$issetExpr->var instanceof Expr\Variable
+								|| $this->isAlwaysSet($scope, $issetExpr->var)
+							) {
+								$result = $typeSpecifier->create(
+									$issetExpr->var,
+									$typeToRemove,
+									TypeSpecifierContext::createFalse(),
+									$scope,
+								)->setRootExpr($expr);
+
+								if ($scope->hasExpressionType($issetExpr->var)->maybe()) {
+									$result = $result->unionWith(
+										$typeSpecifier->create(
+											new IssetExpr($issetExpr->var),
+											new NullType(),
+											TypeSpecifierContext::createTruthy(),
+											$scope,
+										)->setRootExpr($expr),
+									);
+								}
+
+								return $result;
 							}
 
-							return $result;
+							// Every possible value of $issetExpr->var has the offset set,
+							// so the only way for isset() to be false is $issetExpr->var not existing.
+							if (
+								TypeCombinator::remove($varType, $typeToRemove) instanceof NeverType
+								&& $issetExpr->var instanceof ArrayDimFetch
+								&& $issetExpr->var->dim !== null
+								&& $this->isAlwaysSet($scope, $issetExpr->var->var)
+							) {
+								$varDimTypes = $scope->getType($issetExpr->var->dim)->toArrayKey()->getConstantScalarTypes();
+								if (count($varDimTypes) === 1) {
+									$varVarType = $scope->getType($issetExpr->var->var);
+									$withoutOffset = $varVarType->unsetOffset($varDimTypes[0]);
+
+									// A list type cannot express "offset 0 is missing", the intersection
+									// would collapse to never and wrongly kill the whole branch.
+									if (!TypeCombinator::intersect($withoutOffset, $varVarType) instanceof NeverType) {
+										return $typeSpecifier->create(
+											$issetExpr->var->var,
+											$withoutOffset,
+											TypeSpecifierContext::createTruthy(),
+											$scope,
+										)->setRootExpr($expr);
+									}
+								}
+							}
 						}
 					}
 				}
@@ -342,6 +377,31 @@
 		return $types;
 	}
 
+	/**
+	 * Whether the expression is guaranteed to exist, so that narrowing it does not
+	 * leak the existence of an intermediate offset into the enclosing array type.
+	 */
+	private function isAlwaysSet(Scope $scope, Expr $expr): bool
+	{
+		if ($expr instanceof Expr\Variable) {
+			return is_string($expr->name) && $scope->hasVariableType($expr->name)->yes();
+		}
+
+		if ($expr instanceof ArrayDimFetch) {
+			if ($expr->dim === null) {
+				return false;
+			}
+
+			if (!$scope->getType($expr->var)->hasOffsetValueType($scope->getType($expr->dim))->yes()) {
+				return false;
+			}
+
+			return $this->isAlwaysSet($scope, $expr->var);
+		}
+
+		return true;
+	}
+
 	public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult
 	{
 		$hasYield = false;

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

@@ -0,0 +1,142 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15005Nsrt;
+
+use function PHPStan\Testing\assertType;
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function nestedIssetLeak(array $r): void
+{
+	assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+
+	$port = isset($r['K']['Port']) ? $r['K']['Port'] : null;
+
+	assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+
+	$secure = $r['K']['Secure'] ?? null;
+
+	echo $port, $secure;
+}
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function alsoAfterPlainIf(array $r): void
+{
+	if (isset($r['K']['Port'])) {
+		echo $r['K']['Port'];
+	}
+
+	assertType('string|null', $r['K']['Secure'] ?? null);
+}
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function branches(array $r): void
+{
+	if (isset($r['K']['Port'])) {
+		assertType('array{Port: int, Secure: string|null}', $r['K'] ?? null);
+	} else {
+		assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+	}
+	assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+}
+
+/** @param array{K?: array{Port: int, Secure: string|null}} $r */
+function optionalIntermediateKey(array $r): void
+{
+	if (isset($r['K']['Port'])) {
+		assertType('array{K: array{Port: int, Secure: string|null}}', $r);
+	} else {
+		assertType('array{}', $r);
+	}
+	assertType('array{}|array{K: array{Port: int, Secure: string|null}}', $r);
+}
+
+/** @param array<string, array<string, array{Port: int}>> $r */
+function threeLevels(array $r): void
+{
+	if (isset($r['A']['B']['Port'])) {
+		assertType('array{Port: int}', $r['A']['B'] ?? null);
+	} else {
+		assertType('array<string, array{Port: int}>|null', $r['A'] ?? null);
+		assertType('array<string, array<string, array{Port: int}>>', $r);
+	}
+}
+
+/** @param array{a: array{x: int}|array{y: int}} $r */
+function definitelySetIntermediate(array $r): void
+{
+	if (isset($r['a']['x'])) {
+		assertType('array{x: int}', $r['a']);
+	} else {
+		assertType('array{y: int}', $r['a']);
+	}
+}
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function nestedEmptyLeak(array $r): void
+{
+	if (empty($r['K']['Port'])) {
+		assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+	}
+	assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+}
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function nestedCoalesceLeak(array $r): void
+{
+	$port = $r['K']['Port'] ?? null;
+	assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+	echo $port;
+}
+
+/** @param array<int, array{Port: int}> $r */
+function intOffsets(array $r): void
+{
+	if (isset($r[0]['Port'])) {
+		assertType('array{Port: int}', $r[0] ?? null);
+	} else {
+		assertType('array<int<min, -1>|int<1, max>, array{Port: int}>', $r);
+		assertType('array{Port: int}|null', $r[1] ?? null);
+	}
+	assertType('array{Port: int}|null', $r[0] ?? null);
+}
+
+/** @param list<array{Port: int}> $r */
+function listOffsets(array $r): void
+{
+	if (isset($r[0]['Port'])) {
+		assertType('array{Port: int}', $r[0] ?? null);
+	} else {
+		assertType('list<array{Port: int}>', $r);
+		assertType('array{Port: int}|null', $r[0] ?? null);
+	}
+	assertType('array{Port: int}|null', $r[0] ?? null);
+}
+
+final class Holder
+{
+
+	/** @var array<string, array{Port: int, Secure: string|null}> */
+	public array $arr = [];
+
+	public function doFoo(): void
+	{
+		if (isset($this->arr['K']['Port'])) {
+			assertType('array{Port: int, Secure: string|null}', $this->arr['K'] ?? null);
+		} else {
+			assertType('array{Port: int, Secure: string|null}|null', $this->arr['K'] ?? null);
+		}
+		assertType('array{Port: int, Secure: string|null}|null', $this->arr['K'] ?? null);
+	}
+
+}
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function multipleIssetVars(array $r): void
+{
+	if (isset($r['K']['Port'], $r['K']['Secure'])) {
+		assertType('array{Port: int, Secure: string}', $r['K']);
+	} else {
+		assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+	}
+	assertType('array{Port: int, Secure: string|null}|null', $r['K'] ?? null);
+}

tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php

@@ -560,4 +560,9 @@ public function testBug14393(): void
 		]);
 	}
 
+	public function testBug15005(): void
+	{
+		$this->analyse([__DIR__ . '/data/bug-15005.php'], []);
+	}
+
 }

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

@@ -0,0 +1,25 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15005;
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function nestedIssetLeak(array $r): void
+{
+	$port = isset($r['K']['Port']) ? $r['K']['Port'] : null;
+
+	$secure = $r['K']['Secure'] ?? null;
+
+	echo $port, $secure;
+}
+
+/** @param array<string, array{Port: int, Secure: string|null}> $r */
+function alsoAfterPlainIf(array $r): void
+{
+	if (isset($r['K']['Port'])) {
+		echo $r['K']['Port'];
+	}
+
+	$secure = $r['K'] ?? null;
+
+	echo count((array) $secure);
+}