Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Type/ArrayType.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,18 @@ 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 possibly-empty constant array admits `[]`, a subtype of every
// array type, so the relationship is at worst `maybe`, never `no`.
return IsSuperTypeOfResult::createMaybe();
}
return $result;
}

if ($type instanceof CompoundType) {
Expand Down
107 changes: 105 additions & 2 deletions src/Type/Constant/ConstantArrayType.php
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,56 @@ public function unsetOffset(Type $offsetType, bool $preserveListCertainty = fals
return $this->recreate($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $optionalKeys, $newIsList, $this->unsealed);
}

/**
* List-ness of a sealed shape from its keys and optionality: `yes` if every
* realization (choice of present optional keys) is a list, `no` if none is,
* `maybe` otherwise.
*
* @param list<ConstantIntegerType|ConstantStringType> $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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line makes me feel that we might need to handle shapes with a key which php auto converts to int (e.g. "1").

please add a test for such example. usually a fix would need toArrayKey()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed. inferIsListFromShape() now normalizes each key with toArrayKey() before deciding whether it continues the list, so a numeric-string key like "1" (which PHP stores as the integer key 1) is treated as an integer.

In practice keys reaching this method are already normalized (the builder calls toArrayKey() in setOffsetValueType(), and PHPDoc array{"1": …} resolves to array{1: …}), so it's a defensive fix — but a directly-constructed ConstantArrayType can carry an un-normalized "1" key, which is what the new test testMergeWithTreatsNumericStringKeyAsIntWhenRecomputingListness exercises: merging array{0: string, '1'?: string} yields isList = yes (it fails without the toArrayKey() call, giving maybe).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed up: the same instanceof ConstantIntegerType pattern in the adjacent isListAfterUnset() (the preserveListCertainty path of unsetOffset()) had the same numeric-string-key gap, so I normalized there too — matching the two fixes #6026 landed for this. Added testUnsetOffsetTreatsNumericStringKeyAsIntWhenRecomputingListness (unsetting the optional tail of list{'0': string, 1?: int} keeps isList = yes; it regresses to a broken result without the toArrayKey() call).

// A numeric-string key like "1" is an integer key at runtime, so
// normalize before deciding whether it continues the list.
$arrayKey = $keyType->toArrayKey();
$value = $arrayKey instanceof ConstantIntegerType ? $arrayKey->getValue() : null;

$newValidLengths = [];
foreach (array_keys($validLengths) as $length) {
if ($isOptional) {
$newValidLengths[$length] = true;
}

// A key equal to the current length extends the prefix; anything
// else is a non-list realization.
if ($value === $length) {
$newValidLengths[$length + 1] = true;
} else {
$existsInvalid = true;
}
}

$validLengths = $newValidLengths;
if ($validLengths === []) {
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.
Expand All @@ -1454,6 +1504,9 @@ private static function isListAfterUnset(array $newKeyTypes, array $newOptionalK

$isListOnlyIfKeysAreOptional = false;
foreach ($newKeyTypes as $k2 => $newKeyType2) {
// A numeric-string key like "1" is an integer key 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)) {
Expand Down Expand Up @@ -3034,12 +3087,21 @@ public function mergeWith(self $otherArray): self
/** @var list<ConstantIntegerType|ConstantStringType> $keyTypes */
$keyTypes = $keyTypes;

// Merging widens single-side keys to optional, so a sealed result may gain
// list realizations (e.g. `[]`) the naive `and` misses. `or`-ing in the
// shape's own list-ness lifts a `no`/`maybe` while keeping a genuine `yes`.
$naiveIsList = $this->isList->and($otherArray->isList);
$mergedIsSealed = $mergedUnsealedKey instanceof NeverType && $mergedUnsealedKey->isExplicit();
$isList = $mergedIsSealed
? $naiveIsList->or(self::inferIsListFromShape($keyTypes, $optionalKeys))
: $naiveIsList;

return $this->recreate(
$keyTypes,
$valueTypes,
$nextAutoIndexes,
$optionalKeys,
$this->isList->and($otherArray->isList),
$isList,
$resultUnsealed,
);
}
Expand All @@ -3066,7 +3128,16 @@ 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);
// Same recompute as mergeWith(), over `$this`'s keys only — this legacy
// path drops the other side's extra keys.
$naiveIsList = $this->isList->and($otherArray->isList);
$mergedIsSealed = $this->unsealed === null
|| ($this->unsealed[0] instanceof NeverType && $this->unsealed[0]->isExplicit());
$isList = $mergedIsSealed
? $naiveIsList->or(self::inferIsListFromShape($this->keyTypes, $optionalKeys))
: $naiveIsList;

return $this->recreate($this->keyTypes, $valueTypes, $nextAutoIndexes, $optionalKeys, $isList, $this->unsealed);
}

/**
Expand Down Expand Up @@ -3170,6 +3241,38 @@ public function makeList(): Type
return new NeverType();
}

// isList is Maybe. In a sealed shape a key past a gap in the 0..n sequence
// (or any non-integer key) can never appear in a list, so keep only the
// contiguous 0..m prefix. Unsealed extras may fill the gaps, so keep every
// key there.
if ($this->isUnsealed()->no()) {
$positionByIndex = [];
foreach ($this->keyTypes as $position => $keyType) {
if (!$keyType instanceof ConstantIntegerType) {
continue;
}
$positionByIndex[$keyType->getValue()] = $position;
}

$keptPositions = [];
for ($index = 0; array_key_exists($index, $positionByIndex); $index++) {
$keptPositions[] = $positionByIndex[$index];
}

if (count($keptPositions) < count($this->keyTypes)) {
$builder = ConstantArrayTypeBuilder::createEmpty();
foreach ($keptPositions as $position) {
$builder->setOffsetValueType(
$this->keyTypes[$position],
$this->valueTypes[$position],
$this->isOptionalKey($position),
);
}

return $builder->getArray();
}
}

return $this->recreate($this->keyTypes, $this->valueTypes, $this->nextAutoIndexes, $this->optionalKeys, TrinaryLogic::createYes(), $this->unsealed);
}

Expand Down
20 changes: 16 additions & 4 deletions src/Type/Constant/ConstantArrayTypeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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->markNonListKey($optional);
}
}
} else {
$this->isList = TrinaryLogic::createNo();
$this->markNonListKey($optional);
}

if ($offsetValue >= $max) {
Expand All @@ -245,10 +245,10 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
}
}
} else {
$this->isList = TrinaryLogic::createNo();
$this->markNonListKey($optional);
}
} else {
$this->isList = TrinaryLogic::createNo();
$this->markNonListKey($optional);
}

if ($optional) {
Expand Down Expand Up @@ -410,6 +410,18 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $opt
$this->degradeToGeneralArray = true;
}

/**
* Record adding a key incompatible with list ordering. A required key breaks
* list-ness; an optional one only degrades Yes to Maybe (No stays No), since
* the array is still a list when the key is absent.
*/
private function markNonListKey(bool $optional): void
{
$this->isList = $optional
? $this->isList->and(TrinaryLogic::createMaybe())
: TrinaryLogic::createNo();
}

public function degradeToGeneralArray(bool $oversized = false): void
{
if ($this->disableArrayDegradation) {
Expand Down
7 changes: 5 additions & 2 deletions tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ 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);
// The trailing keys can never appear in a list (4 sits past the gap at
// 3; foo is not an integer), so they are dropped, leaving the valid
// list projection rather than an empty *NEVER*.
assertType('array{0: string, 1: int, 2?: string}', $invalid1);
assertType('array{0: string, 1: int, 2?: string}', $invalid2);
}

}
86 changes: 86 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-14938.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php

namespace Bug14938;

use function PHPStan\Testing\assertType;

class Foo
{

/**
* @param array{a?: string} $optStr
* @param array{0: int, a?: string} $listPlusOptStr
* @param array{-1?: string} $optNeg
* @param array{0: int, 5?: string} $listPlusGap
* @param array{a: string} $reqStr
* @param array{1: string} $gapReq
* @param array{0?: string} $optZero
*/
public function doFoo(
array $optStr,
array $listPlusOptStr,
array $optNeg,
array $listPlusGap,
array $reqStr,
array $gapReq,
array $optZero,
): void
{
// An optional non-list key might be absent, so the array can still be
// a list ([] / the list prefix) — array_is_list() is not decidable.
assertType('bool', array_is_list($optStr));
assertType('bool', array_is_list($listPlusOptStr));
assertType('bool', array_is_list($optNeg));
assertType('bool', array_is_list($listPlusGap));

// A required non-list key is always present, so it is never a list.
assertType('false', array_is_list($reqStr));
assertType('false', array_is_list($gapReq));

// Only an optional key 0 keeps it a guaranteed list ([] and [v]).
assertType('true', array_is_list($optZero));
}

/**
* @param array{a?: string} $optStr
*/
public function narrowing(array $optStr): void
{
if (array_is_list($optStr)) {
assertType('list{}&list', $optStr);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this fix array{a?: string} had isList = No, so this branch was *NEVER* (unreachable). With isList now Maybe, the empty-array realisation survives the is list intersection, so the branch is reachable and narrows to the empty list.

} else {
// array_is_list()'s false branch is not narrowed, so `a` stays optional
// here rather than being refined to array{a: string}.
assertType('array{a?: string}', $optStr);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The a? here (rather than array{a: string}) is pre-existing and unrelated to this fix: array_is_list()'s false branch is not refined. It stays a? for sealed shapes too — e.g. even array{0: string, a?: string} keeps a? in the false branch on current stable.

}
}

public function mergedShapes(): void
{
// Merging a list with a shape that adds a string key: the empty-of-the-extra
// realization is still a list, so array_is_list() is not decidable.
$a = [0 => 'z'];
if (rand(0, 1)) {
$a['y'] = 1;
}
assertType("array{0: 'z', y?: 1}", $a);
assertType('bool', array_is_list($a));

// Two pure lists merge into a list (optional keys stay a suffix).
$b = [0 => 'z'];
if (rand(0, 1)) {
$b[1] = 'w';
}
assertType("array{0: 'z', 1?: 'w'}", $b);
assertType('true', array_is_list($b));

// Shapes disjoint except for the empty array still admit the empty list.
$c = [];
if (rand(0, 1)) {
$c['a'] = 1;
}
assertType('array{}|array{a: 1}', $c);
assertType('bool', array_is_list($c));
}

}
Loading
Loading