Skip to content

Degrade isList to maybe for optional non-list keys in array shapes and shape merges#6026

Open
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-vzsajw9
Open

Degrade isList to maybe for optional non-list keys in array shapes and shape merges#6026
phpstan-bot wants to merge 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-vzsajw9

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

array_is_list() was reported as always false for array shapes whose only non-list keys are optional, e.g. array{a?: string} or array{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 the impossibleType check simultaneously claimed the condition was always false — a self-contradiction. This is the dual of #12725.

Changes

  • src/Type/Constant/ConstantArrayTypeBuilder.phpsetOffsetValueType() forced isList = no for any non-list-compatible key (string key, gap int, negative int, out-of-range int) regardless of optionality. It now only forces no for mandatory such keys; an optional one degrades to maybe via $this->isList->and(createMaybe()). Fixes the reported PHPDoc-shape cases directly.
  • src/Type/Constant/ConstantArrayType.php
    • Added inferIsListFromShape(): 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() and legacyMergeWith() 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), so yes is preserved.
  • src/Type/ArrayType.phpisSuperTypeOf() returned no for a constant array whose offending keys are all optional (e.g. array<int, mixed> vs array{a?: string}). Since such a shape always admits the empty array — a subtype of every array type — it now returns maybe. This is what makes the array_is_list() conditional-return narrowing produce a non-never truthy type.

Analogous cases probed

  • Conditional-assignment shapes ($b = [0=>'z']; if (rand()) $b['y'] = 1;array{0: 'z', y?: 1}): the same false positive, produced through the union/mergeWith path rather than the builder. Fixed by the mergeWith changes.
  • ArrayType::accepts() (sibling of isSuperTypeOf): probed and left unchanged — it correctly returns no, because passing array{a?: string} where array<int, mixed> is required is genuinely unsafe (['a' => …] violates it). no there is correct, not a parallel bug.
  • Pure-list merges (list{0} ∪ list{0,1}list{0, 1?}): verified they remain yes (guarded by the !$naiveIsList->yes() gate and covered by the existing array-shape-list-optional.php and bug-yield-oversized-self-rejection.php tests).

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 two ConstantArrayType merge paths, and ArrayType::isSuperTypeOf()'s coarse key/value check.

Test

  • New tests/PHPStan/Analyser/nsrt/bug-14938.php covers 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 two list{…} shapes with an optional gap/string key previously asserted *NEVER* (the buggy behavior); they now correctly resolve to non-never list shapes, since dropping the optional key yields a valid list.
  • Verified the new test fails without the source change and passes with it; full make tests and make phpstan are green.

Fixes phpstan/phpstan#14938

…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
@zonuexe

zonuexe commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

I opened #6025 for the same issue (#14938) before seeing this. The two PRs converge almost exactly: ArrayType::isSuperTypeOf() is identical, and the ConstantArrayTypeBuilder change (an optional non-list key degrades isList to maybe) is the same modulo a helper method. Two points are worth an explicit decision.

1. Projection of an invalid/sealed list-shape (option A vs B)

When a sealed list{…} shape carries an optional key that can never appear in a list (a gap, or a string key), intersecting with list can be handled two ways:

A — keep the dead key (this PR):

list{0: string, 1: int, 2?: string, 4?: string}   // 4? survives past the gap at 3
array{a?: string}  ->  list&list{a?: string}       // dead a? on a list

B — drop the unreachable keys (#6025, via ConstantArrayType::makeList()):

list{0: string, 1: int, 2?: string, 4?: string}  ->  list{0: string, 1: int, 2?: string}
array{a?: string}  ->  list{}

B yields a well-formed list type with no key that can never be present (so e.g. $x[4] is an undefined offset rather than string), at the cost of a small projection step in makeList(). A is simpler but keeps a dead optional key on the type.

2. mergeWith() / legacyMergeWith()

This PR additionally fixes the merge-side dual — array{a: 1}|array{b: 2} widens to array{a?: 1, b?: 2}, which admits [] and so should be maybe, not no — backed by the oracle-validated inferIsListFromShape(). #6025 does not cover this yet, so that part is a clear plus for #6026.

Suggestion

The only real fork is A vs B for the shape projection; everything else can be a single PR. I'd lean B (the type stays well-formed), but I'm happy to go either way — merge whichever projection you prefer, and I can fold the mergeWith fix from here into #6025, or add the makeList() projection here, whichever PR you'd rather keep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

array_is_list() reported as always false for shapes whose only non-list keys are optional

2 participants