From d645376e065ed7c6b5bf4d14e401570b0b819f4e Mon Sep 17 00:00:00 2001 From: staabm <120441+staabm@users.noreply.github.com> Date: Sun, 5 Jul 2026 08:43:36 +0000 Subject: [PATCH] Narrow the value of an optional array offset on the left of `??` when compared with `===`/`!==` without asserting it exists - In `TypeSpecifier::createForExpr()`, when a coalesce is specified in the false context and the compared type does not remove the default value (so the left side may still be absent), narrow the left side's value via the new `narrowCoalesceLeftPreservingExistence()` helper instead of leaving it untouched. - The helper rebuilds the container's constant-array shape with the compared type removed from the offset's value while keeping the key optional, so `($a['k'] ?? null) !== false` yields `array{k?: mixed~false}` rather than `array{k?: mixed}`, and does so for any compared constant (e.g. `!== 2` on `1|2|3` yields `1|3`). - Only variable and property/static-property containers are overwritten; nested offsets (`$a[$x]['k']`) are left alone so the outer offset is not wrongly memorized as existing (preserves the fix from #6000). - Added `ConstantArrayType::replaceOffsetValueTypePreservingOptionality()` which replaces an existing offset's value without promoting an optional key to a required one (unlike `setExistingOffsetValueType()`). --- phpstan-baseline.neon | 2 +- src/Analyser/TypeSpecifier.php | 95 +++++++++++++++++++- src/Type/Constant/ConstantArrayType.php | 27 ++++++ tests/PHPStan/Analyser/nsrt/bug-12057.php | 102 ++++++++++++++++++++++ 4 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-12057.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b5777ca536..25b5259561 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -972,7 +972,7 @@ parameters: - rawMessage: 'Doing instanceof PHPStan\Type\Constant\ConstantStringType is error-prone and deprecated. Use Type::getConstantStrings() instead.' identifier: phpstanApi.instanceofType - count: 5 + count: 6 path: src/Type/Constant/ConstantArrayType.php - diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 73c56dbf23..ca977433ee 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -46,6 +46,7 @@ use function count; use function get_class; use function in_array; +use function is_string; use function strtolower; use function substr; use const COUNT_NORMAL; @@ -612,11 +613,23 @@ private function createForExpr( !$context->null() && $expr instanceof Expr\BinaryOp\Coalesce ) { - if ( - ($context->true() && $type->isSuperTypeOf($scope->getType($expr->right))->no()) - || ($context->false() && $type->isSuperTypeOf($scope->getType($expr->right))->yes()) - ) { + $rightIsSuperType = $type->isSuperTypeOf($scope->getType($expr->right)); + if ($context->true() && $rightIsSuperType->no()) { $expr = $expr->left; + } elseif ($context->false()) { + if ($rightIsSuperType->yes()) { + // The default value would be removed too, so the left side is + // guaranteed to be set and can be narrowed directly. + $expr = $expr->left; + } else { + // The default value survives, so the left side may still be + // absent (e.g. an optional array offset). Narrow its value + // without asserting that it exists. + $narrowed = $this->narrowCoalesceLeftPreservingExistence($expr->left, $type, $scope); + if ($narrowed !== null) { + return $narrowed; + } + } } } @@ -738,6 +751,80 @@ private function createForExpr( return $types; } + /** + * Narrows the value of a null-coalescing left side (e.g. `$a['k']` in + * `($a['k'] ?? $default) !== $type`) by removing $type from it, while + * preserving whether the offset actually exists. Returns null when this + * cannot be done without asserting the existence of a preceding offset, + * which would defeat the purpose of the `??` access. + */ + private function narrowCoalesceLeftPreservingExistence(Expr $leftExpr, Type $type, Scope $scope): ?SpecifiedTypes + { + if (!$leftExpr instanceof Expr\ArrayDimFetch || $leftExpr->dim === null) { + return null; + } + + // Overwriting the container is only safe when doing so does not imply + // that a preceding offset exists. A variable or a property access is the + // safe case; a nested offset (`$a[$x]['k']`) would memorize `$a[$x]` as + // existing, which is exactly what the `??` access guards against. + $containerExpr = $leftExpr->var; + if (!$this->isSafeCoalesceContainerToOverwrite($containerExpr)) { + return null; + } + + $containerType = $scope->getType($containerExpr); + if (!$containerType->isConstantArray()->yes()) { + return null; + } + + $dimType = $scope->getType($leftExpr->dim); + + $newParts = []; + foreach ($containerType->getConstantArrays() as $constantArray) { + $narrowedValueType = TypeCombinator::remove($constantArray->getOffsetValueType($dimType), $type); + if ($narrowedValueType instanceof NeverType) { + return null; + } + + $newParts[] = $constantArray->replaceOffsetValueTypePreservingOptionality($dimType, $narrowedValueType); + } + + if ($newParts === []) { + return null; + } + + $exprString = $this->exprPrinter->printExpr($containerExpr); + + return (new SpecifiedTypes( + [$exprString => [$containerExpr, TypeCombinator::union(...$newParts)]], + [], + ))->setAlwaysOverwriteTypes(); + } + + /** + * Whether the container of a null-coalescing offset access can have its type + * overwritten without implying that a preceding array offset exists. Plain + * variables and property accesses reaching down to one are safe; array + * offsets and dynamic accesses in the chain are not. + */ + private function isSafeCoalesceContainerToOverwrite(Expr $expr): bool + { + if ($expr instanceof Expr\Variable && is_string($expr->name)) { + return true; + } + + if ($expr instanceof PropertyFetch && $expr->name instanceof Node\Identifier) { + return $this->isSafeCoalesceContainerToOverwrite($expr->var); + } + + if ($expr instanceof Expr\StaticPropertyFetch && $expr->name instanceof Node\VarLikeIdentifier) { + return $expr->class instanceof Name || $this->isSafeCoalesceContainerToOverwrite($expr->class); + } + + return false; + } + private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes { if ($expr instanceof Expr\NullsafePropertyFetch) { diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 7eca2eaa58..792d84d79c 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -1311,6 +1311,33 @@ public function setExistingOffsetValueType(Type $offsetType, Type $valueType): T return $builder->getArray(); } + /** + * Replaces the value type of an already-present offset, keeping the key's + * existing optional status untouched. Unlike setExistingOffsetValueType(), + * this never promotes an optional key to a required one. No-op when the + * offset is not part of the shape. + */ + public function replaceOffsetValueTypePreservingOptionality(Type $offsetType, Type $valueType): self + { + $offsetType = $offsetType->toArrayKey(); + if (!$offsetType instanceof ConstantIntegerType && !$offsetType instanceof ConstantStringType) { + return $this; + } + + $valueTypes = $this->valueTypes; + foreach ($this->keyTypes as $i => $keyType) { + if ($keyType->getValue() !== $offsetType->getValue()) { + continue; + } + + $valueTypes[$i] = $valueType; + + return new self($this->keyTypes, $valueTypes, $this->nextAutoIndexes, $this->optionalKeys, $this->isList, $this->unsealed); + } + + return $this; + } + /** * Removes or marks as optional the key(s) matching the given offset type from this constant array. * diff --git a/tests/PHPStan/Analyser/nsrt/bug-12057.php b/tests/PHPStan/Analyser/nsrt/bug-12057.php new file mode 100644 index 0000000000..111ccc304f --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-12057.php @@ -0,0 +1,102 @@ +options['foo'] ?? null) === false) { + return; + } + + assertType('array{foo?: mixed~false}', $c->options); +} + +/** + * A nested offset must NOT be memorized as existing: when the outer key is + * missing, the whole coalesce expression is the default and the condition + * can still be satisfied. + * + * @param array $rows + */ +function nestedOffsetKeepsExistenceUncertain(array $rows, string $key): void +{ + if (($rows[$key]['foo'] ?? null) === false) { + return; + } + + assertType('array', $rows); +} + +/** + * When the default value would itself be removed, the left side is guaranteed + * to be set and is narrowed to a required, non-null offset. + * + * @param array{foo?: int|null} $options + */ +function defaultRemovedForcesExistence(array $options): void +{ + if (($options['foo'] ?? null) !== null) { + assertType('array{foo: int}', $options); + } +}