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