diff --git a/src/Type/ArrayType.php b/src/Type/ArrayType.php index a951f13c24b..ddd9446d5a5 100644 --- a/src/Type/ArrayType.php +++ b/src/Type/ArrayType.php @@ -147,8 +147,21 @@ public function accepts(Type $type, bool $strictTypes): AcceptsResult public function isSuperTypeOf(Type $type): IsSuperTypeOfResult { if ($type instanceof self || $type instanceof ConstantArrayType) { - return $this->getItemType()->isSuperTypeOf($type->getItemType()) + $result = $this->getItemType()->isSuperTypeOf($type->getItemType()) ->and($this->getIterableKeyType()->isSuperTypeOf($type->getIterableKeyType())); + + if ( + $result->no() + && $type->isConstantArray()->yes() + && !$type->isIterableAtLeastOnce()->yes() + ) { + // A constant array whose offending keys/values are all optional + // still admits the empty array, which is a subtype of every + // array type — so the relationship is `maybe`, not `no`. + return IsSuperTypeOfResult::createMaybe(); + } + + return $result; } if ($type instanceof CompoundType) { diff --git a/src/Type/Constant/ConstantArrayType.php b/src/Type/Constant/ConstantArrayType.php index 7eca2eaa587..02e1b94d0f0 100644 --- a/src/Type/Constant/ConstantArrayType.php +++ b/src/Type/Constant/ConstantArrayType.php @@ -1437,6 +1437,61 @@ public function unsetOffset(Type $offsetType, bool $preserveListCertainty = fals return $this->recreate($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $optionalKeys, $newIsList, $this->unsealed); } + /** + * Compute the list-ness trinary of a sealed array shape purely from its keys + * and their optionality: `yes` if every realization (choice of which optional + * keys are present) is a list, `no` if none is, `maybe` otherwise. An optional + * key that breaks list-ness only degrades the answer to `maybe`, because the + * realization where that key is absent may still be a list. + * + * Keys are normalized with `toArrayKey()` first, so an integer-like string key + * such as `'1'` counts as the integer key `1` it becomes at runtime. + * + * @param list $keyTypes + * @param int[] $optionalKeys + */ + private static function inferIsListFromShape(array $keyTypes, array $optionalKeys): TrinaryLogic + { + $optional = []; + foreach ($optionalKeys as $optionalKey) { + $optional[$optionalKey] = true; + } + + // Prefix lengths reachable by realizations that are still a valid list. + $validLengths = [0 => true]; + $existsInvalid = false; + + foreach ($keyTypes as $i => $keyType) { + $isOptional = array_key_exists($i, $optional); + $arrayKeyType = $keyType->toArrayKey(); + $value = $arrayKeyType instanceof ConstantIntegerType ? $arrayKeyType->getValue() : null; + + $newValidLengths = []; + foreach (array_keys($validLengths) as $length) { + if ($isOptional) { + // Skipping the key keeps the realization a valid list prefix. + $newValidLengths[$length] = true; + } + + if ($value === $length) { + // Including the key extends the list into the next slot. + $newValidLengths[$length + 1] = true; + } else { + // Including a non-sequential key yields a non-list realization. + $existsInvalid = true; + } + } + + $validLengths = $newValidLengths; + if ($validLengths === []) { + // No realization can be a list from here on. + return TrinaryLogic::createNo(); + } + } + + return $existsInvalid ? TrinaryLogic::createMaybe() : TrinaryLogic::createYes(); + } + /** * When we're unsetting something not on the array, it will be untouched, * So the nextAutoIndexes won't change, and the array might still be a list even with PHPStan definition. @@ -1452,6 +1507,9 @@ private static function isListAfterUnset(array $newKeyTypes, array $newOptionalK $isListOnlyIfKeysAreOptional = false; foreach ($newKeyTypes as $k2 => $newKeyType2) { + // An integer-like string key such as '1' is the integer key it becomes at + // runtime, so normalize before deciding whether it continues the list. + $newKeyType2 = $newKeyType2->toArrayKey(); if (!$newKeyType2 instanceof ConstantIntegerType || $newKeyType2->getValue() !== $k2) { // We found a non-optional key that implies that the array is never a list. if (!in_array($k2, $newOptionalKeys, true)) { @@ -3032,12 +3090,24 @@ public function mergeWith(self $otherArray): self /** @var list $keyTypes */ $keyTypes = $keyTypes; + // Merging widens keys present in only one side into optional keys, so the + // result can admit list realizations that neither input did. When the merged + // extras are the explicit-never sentinel (i.e. no real extras), the result is + // sealed and its list-ness follows purely from the merged shape. Two pure + // lists merge into a list (their optional keys are suffix-constrained), so + // keep `yes` in that case rather than degrading it from the shape. + $naiveIsList = $this->isList->and($otherArray->isList); + $mergedIsSealed = $mergedUnsealedKey instanceof NeverType && $mergedUnsealedKey->isExplicit(); + $isList = $mergedIsSealed && !$naiveIsList->yes() + ? self::inferIsListFromShape($keyTypes, $optionalKeys) + : $naiveIsList; + return $this->recreate( $keyTypes, $valueTypes, $nextAutoIndexes, $optionalKeys, - $this->isList->and($otherArray->isList), + $isList, $resultUnsealed, ); } @@ -3064,7 +3134,20 @@ private function legacyMergeWith(self $otherArray): self $nextAutoIndexes = array_values(array_unique(array_merge($this->nextAutoIndexes, $otherArray->nextAutoIndexes))); sort($nextAutoIndexes); - return $this->recreate($this->keyTypes, $valueTypes, $nextAutoIndexes, $optionalKeys, $this->isList->and($otherArray->isList), $this->unsealed); + // Merging widens keys present in only one side into optional keys, so the + // result can admit list realizations that neither input did (e.g. the empty + // array). When the result carries no real extras it is sealed and its + // list-ness follows purely from the merged shape, instead of the too-strict + // `$this->isList->and($otherArray->isList)`. Two pure lists merge into a list + // (their optional keys are suffix-constrained), so keep `yes` in that case. + $naiveIsList = $this->isList->and($otherArray->isList); + $mergedIsSealed = $this->unsealed === null + || ($this->unsealed[0] instanceof NeverType && $this->unsealed[0]->isExplicit()); + $isList = $mergedIsSealed && !$naiveIsList->yes() + ? self::inferIsListFromShape($this->keyTypes, $optionalKeys) + : $naiveIsList; + + return $this->recreate($this->keyTypes, $valueTypes, $nextAutoIndexes, $optionalKeys, $isList, $this->unsealed); } /** diff --git a/src/Type/Constant/ConstantArrayTypeBuilder.php b/src/Type/Constant/ConstantArrayTypeBuilder.php index e81a4d694eb..759d08ef3fe 100644 --- a/src/Type/Constant/ConstantArrayTypeBuilder.php +++ b/src/Type/Constant/ConstantArrayTypeBuilder.php @@ -224,11 +224,11 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt if ($offsetValue <= $max) { $this->isList = $this->isList->and(TrinaryLogic::createMaybe()); } else { - $this->isList = TrinaryLogic::createNo(); + $this->isList = $optional ? $this->isList->and(TrinaryLogic::createMaybe()) : TrinaryLogic::createNo(); } } } else { - $this->isList = TrinaryLogic::createNo(); + $this->isList = $optional ? $this->isList->and(TrinaryLogic::createMaybe()) : TrinaryLogic::createNo(); } if ($offsetValue >= $max) { @@ -245,10 +245,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt } } } else { - $this->isList = TrinaryLogic::createNo(); + $this->isList = $optional ? $this->isList->and(TrinaryLogic::createMaybe()) : TrinaryLogic::createNo(); } } else { - $this->isList = TrinaryLogic::createNo(); + $this->isList = $optional ? $this->isList->and(TrinaryLogic::createMaybe()) : TrinaryLogic::createNo(); } if ($optional) { diff --git a/tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php b/tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php index 156ad93ed2c..116974abe04 100644 --- a/tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php +++ b/tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php @@ -25,8 +25,8 @@ public function doFoo( assertType('list{0: string, 1: int, 2?: string, 3?: string}', $valid1); assertType('list{0: string, 1?: int, 2?: string, 3?: string}', $valid2); assertType('non-empty-array{0?: string, 1?: int, 2?: string, 3?: string}', $valid3); - assertType('*NEVER*', $invalid1); - assertType('*NEVER*', $invalid2); + assertType('list{0: string, 1: int, 2?: string, 4?: string}', $invalid1); + assertType('list{0: string, 1: int, 2?: string, foo?: string}', $invalid2); } } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14938.php b/tests/PHPStan/Analyser/nsrt/bug-14938.php new file mode 100644 index 00000000000..b8d92cc0c49 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14938.php @@ -0,0 +1,136 @@ + 'z']; + if (rand(0, 1)) { + $a['1'] = 'w'; + } + assertType("array{0: 'z', 1?: 'w'}", $a); + assertType('true', array_is_list($a)); + + $b = [0 => 'z']; + if (rand(0, 1)) { + $b['2'] = 'w'; + } + assertType("array{0: 'z', 2?: 'w'}", $b); + assertType('bool', array_is_list($b)); + + $c = [0 => 'z']; + if (rand(0, 1)) { + $c['01'] = 'w'; + } + assertType("array{0: 'z', '01'?: 'w'}", $c); + assertType('bool', array_is_list($c)); +} + +function builtViaConditionalAssignment(): void +{ + $b = [0 => 'z']; + if (rand(0, 1)) { + $b['y'] = 1; + } + assertType("array{0: 'z', y?: 1}", $b); + assertType('bool', array_is_list($b)); + + // Two pure lists still merge into a list. + $c = [0 => 'z']; + if (rand(0, 1)) { + $c[1] = 'w'; + } + assertType("array{0: 'z', 1?: 'w'}", $c); + assertType('true', array_is_list($c)); + + // Merging shapes disjoint save for the empty array still admits the empty list. + $d = []; + if (rand(0, 1)) { + $d['a'] = 1; + } + assertType('array{}|array{a: 1}', $d); + assertType('bool', array_is_list($d)); +} diff --git a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php index 5c85f860264..8cd5c954164 100644 --- a/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php +++ b/tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php @@ -1612,6 +1612,82 @@ public function testEqualsTreatsLegacyNullAndSealedMarkerAsEqual(): void } } + /** + * The `@api` constructor takes key types verbatim, so a caller can hand over an + * integer-like string key such as `'0'` that the builder would have normalized. + * Since such a key *is* the integer key at runtime, the merged shape's list-ness + * must be judged from the normalized key, not from `ConstantStringType`-ness. + */ + public function testMergeWithJudgesListnessFromNormalizedKeys(): void + { + foreach ([false, true] as $bleedingEdge) { + BleedingEdgeToggle::withBleedingEdge($bleedingEdge, function () use ($bleedingEdge): void { + // array{'0': string} merged with array{'0': int} stays array{'0': string|int}, + // which is a list because '0' is the integer key 0. + $stringKeyed = new ConstantArrayType([new ConstantStringType('0')], [new StringType()], [1]); + $otherStringKeyed = new ConstantArrayType([new ConstantStringType('0')], [new IntegerType()], [1]); + + $this->assertSame( + TrinaryLogic::createYes()->describe(), + $stringKeyed->mergeWith($otherStringKeyed)->isList()->describe(), + sprintf('bleedingEdge: %d', (int) $bleedingEdge), + ); + + // array{0: string, '1': int} merged with array{0: string} widens '1' into an + // optional key. Both realizations - with and without the key - are lists. + $withNumericStringKey = new ConstantArrayType( + [new ConstantIntegerType(0), new ConstantStringType('1')], + [new StringType(), new IntegerType()], + [2], + ); + $withoutNumericStringKey = new ConstantArrayType([new ConstantIntegerType(0)], [new StringType()], [1], [], TrinaryLogic::createYes()); + + $this->assertSame( + TrinaryLogic::createYes()->describe(), + $withNumericStringKey->mergeWith($withoutNumericStringKey)->isList()->describe(), + sprintf('bleedingEdge: %d', (int) $bleedingEdge), + ); + + // '01' is not a canonical integer key, so it stays a list-breaking string key. + // Optional, so the key-less realization keeps the answer at maybe. + $withNonCanonicalKey = new ConstantArrayType( + [new ConstantIntegerType(0), new ConstantStringType('01')], + [new StringType(), new IntegerType()], + [1], + ); + + $this->assertSame( + TrinaryLogic::createMaybe()->describe(), + $withNonCanonicalKey->mergeWith($withoutNumericStringKey)->isList()->describe(), + sprintf('bleedingEdge: %d', (int) $bleedingEdge), + ); + }); + } + } + + /** + * The same normalization is needed when re-deriving list-ness after an unset: + * unsetting the optional tail of list{'0': string, 1?: int} leaves a list, so + * the `preserveListCertainty` path must not conclude the shape became impossible. + */ + public function testUnsetOffsetJudgesListnessFromNormalizedKeys(): void + { + $type = new ConstantArrayType( + [new ConstantStringType('0'), new ConstantIntegerType(1)], + [new StringType(), new IntegerType()], + [2], + [1], + TrinaryLogic::createYes(), + ); + + $unset = $type->unsetOffset(new ConstantIntegerType(1), true); + $this->assertInstanceOf(ConstantArrayType::class, $unset); + $this->assertSame(TrinaryLogic::createYes()->describe(), $unset->isList()->describe()); + + $unsetWithoutCertainty = $type->unsetOffset(new ConstantIntegerType(1)); + $this->assertSame(TrinaryLogic::createMaybe()->describe(), $unsetWithoutCertainty->isList()->describe()); + } + public function testSealedness(): void { BleedingEdgeToggle::withBleedingEdge(false, function () {