Degrade isList to maybe for optional non-list keys in array shapes and shape merges#6026
Degrade isList to maybe for optional non-list keys in array shapes and shape merges#6026phpstan-bot wants to merge 1 commit into
isList to maybe for optional non-list keys in array shapes and shape merges#6026Conversation
…s and shape merges - `ConstantArrayTypeBuilder::setOffsetValueType()` now only forces `isList = no` for mandatory non-list-compatible keys; an optional such key degrades list-ness to `maybe` (`$this->isList->and(createMaybe())`), because the realization where the key is absent may still be a list. - Add `ConstantArrayType::inferIsListFromShape()` — computes the list-ness trinary of a sealed shape from its keys and optionality (`yes` iff every realization is a list, `no` iff none is, `maybe` otherwise), validated against a brute-force oracle. - Use it in `ConstantArrayType::mergeWith()` and `legacyMergeWith()` instead of the too-strict `$this->isList->and($otherArray->isList)`: merging widens one-sided keys into optional keys, so the result can admit list realizations (e.g. the empty array) that neither input did. Two pure lists still merge into a list (suffix-constrained), so `yes` is preserved in that case; only applied when the merged extras are sealed. - `ArrayType::isSuperTypeOf()` returns `maybe` instead of `no` for a constant array whose offending keys are all optional (it always admits the empty array, a subtype of every array type) — this is what makes `array_is_list()` narrowing reachable. Closes phpstan/phpstan#14938
|
I opened #6025 for the same issue (#14938) before seeing this. The two PRs converge almost exactly: 1. Projection of an invalid/sealed list-shape (option A vs B)When a sealed A — keep the dead key (this PR): B — drop the unreachable keys (#6025, via B yields a well-formed list type with no key that can never be present (so e.g. 2.
|
Summary
array_is_list()was reported as always false for array shapes whose only non-list keys are optional, e.g.array{a?: string}orarray{0: int, a?: string}. Such shapes have list inhabitants ([],[0 => 1]), so the correct trinary answer is maybe, not no. The truthy branch even narrowed correctly (list{int}) while theimpossibleTypecheck simultaneously claimed the condition was always false — a self-contradiction. This is the dual of #12725.Changes
src/Type/Constant/ConstantArrayTypeBuilder.php—setOffsetValueType()forcedisList = nofor any non-list-compatible key (string key, gap int, negative int, out-of-range int) regardless of optionality. It now only forcesnofor mandatory such keys; an optional one degrades tomaybevia$this->isList->and(createMaybe()). Fixes the reported PHPDoc-shape cases directly.src/Type/Constant/ConstantArrayType.phpinferIsListFromShape(): computes the list-ness trinary of a sealed shape purely from its keys and optionality. Validated against an exhaustive brute-force oracle (2929 shapes, 0 mismatches).mergeWith()andlegacyMergeWith()computed the merged list-ness as$this->isList->and($otherArray->isList). Merging widens keys present in only one side into optional keys, so the result admits list realizations neither input did (including the empty array) — a pre-existing latent bug (array{a:1}|array{b:2}→array{a?:1,b?:2}admits[], a list). Now recomputed from the merged shape when the result is sealed. Two pure lists still merge into a list (their optionality is suffix-constrained), soyesis preserved.src/Type/ArrayType.php—isSuperTypeOf()returnednofor a constant array whose offending keys are all optional (e.g.array<int, mixed>vsarray{a?: string}). Since such a shape always admits the empty array — a subtype of every array type — it now returnsmaybe. This is what makes thearray_is_list()conditional-return narrowing produce a non-nevertruthy type.Analogous cases probed
$b = [0=>'z']; if (rand()) $b['y'] = 1;→array{0: 'z', y?: 1}): the same false positive, produced through the union/mergeWithpath rather than the builder. Fixed by themergeWithchanges.ArrayType::accepts()(sibling ofisSuperTypeOf): probed and left unchanged — it correctly returnsno, because passingarray{a?: string}wherearray<int, mixed>is required is genuinely unsafe (['a' => …]violates it).nothere is correct, not a parallel bug.list{0} ∪ list{0,1}→list{0, 1?}): verified they remainyes(guarded by the!$naiveIsList->yes()gate and covered by the existingarray-shape-list-optional.phpandbug-yield-oversized-self-rejection.phptests).Root cause
The list-ness trinary of a shape with optional keys was computed as if every key were always present. Adding, or merging in, a non-list-compatible key unconditionally set
isList = no, ignoring that an optional such key may be absent — leaving a list realization (often the empty array) unaccounted for. The same "ignore part of the value set" pattern appeared in three places: the builder's offset-write, the twoConstantArrayTypemerge paths, andArrayType::isSuperTypeOf()'s coarse key/value check.Test
tests/PHPStan/Analyser/nsrt/bug-14938.phpcovers the two reported PHPDoc shapes plus optional gap-int, optional negative-int, mandatory string, and mandatory gap-int shapes (the last two must stay*NEVER*), and the conditional-assignment analogs (array{0:'z', y?:1}→bool, pure-list merge →true, empty-union →bool).tests/PHPStan/Analyser/nsrt/array-shape-list-optional.php: the twolist{…}shapes with an optional gap/string key previously asserted*NEVER*(the buggy behavior); they now correctly resolve to non-neverlist shapes, since dropping the optional key yields a valid list.make testsandmake phpstanare green.Fixes phpstan/phpstan#14938