Decompose union offset types in ConstantArrayType::flipArray() and fillKeysArray() to preserve per-key value precision

[phpstan-bot]

Summary

array_flip() on a constant array whose values are union types (e.g. array{0: 'a'|'b'|'c', 1: 'a'|'b'|'c', 2: 'a'|'b'|'c'}) produced wrong results — the last key's index overwrote all previous values, yielding non-empty-array{a?: 2, b?: 2, c?: 2} instead of the correct non-empty-array{a?: 0|1|2, b?: 0|1|2, c?: 0|1|2}.

The same underlying issue also caused array_fill_keys to degrade to a general array type when overlapping union key types were involved.

Changes

• src/Type/Constant/ConstantArrayType.php: In flipArray(), when a value type decomposes into multiple constant scalar types, iterate over each individually with optional=true instead of passing the full union to setOffsetValueType. Applied the same decomposition in fillKeysArray().
• src/Type/Php/ArrayFillKeysFunctionReturnTypeExtension.php: Added NonEmptyArrayType wrapping when the input array is non-empty, mirroring the logic already present in ArrayFlipFunctionReturnTypeExtension.
• tests/PHPStan/Analyser/nsrt/bug-14656.php: Added regression tests for both array_flip and array_fill_keys with union value types, including overlapping unions and mixed union/constant cases.

Root cause

ConstantArrayType::flipArray() called ConstantArrayTypeBuilder::setOffsetValueType() with a union type as the offset and optional=false. The builder's logic for union offsets matching existing optional keys (line 283 of ConstantArrayTypeBuilder) replaces existing values rather than unioning them when $optional=false. Each iteration of flipArray() passed a different key index as the value, so the last iteration's value (2) overwrote all earlier values (0, 1).

For overlapping unions (e.g. 'a'|'b' followed by 'b'|'c'), the builder's partial-match path ($hasMatch && !$match) degraded to a general array type, losing constant array precision entirely.

The fix decomposes union offsets at the caller level — flipArray() and fillKeysArray() now iterate over each constant scalar type individually with optional=true, which:

1. Correctly unions values across iterations (the builder's else branch at line 286)
2. Avoids partial-match degradation (each call has a single scalar offset)

Analogous cases probed:

• fillKeysArray() — same degradation bug found and fixed
• intersectKeyArray(), diffKeyArray(), changeKeyCaseArray(), filterArrayRemovingFalsey(), sliceArray(), spliceArray() — these use original key types (always constant, never union) as offsets, so they are not affected

Test

• tests/PHPStan/Analyser/nsrt/bug-14656.php covers:
  • array_flip with all-union values (array{0: 'a'|'b'|'c', 1: 'a'|'b'|'c', 2: 'a'|'b'|'c'}) — the reported bug
  • array_flip with overlapping unions (array{0: 'a'|'b', 1: 'b'|'c'})
  • array_flip with mixed union and constant (array{0: 'a'|'b', 1: 'c'})
  • array_fill_keys with the same three union patterns

Fixes phpstan/phpstan#14656

[VincentLanglet]

Shouldn't the fix be directly inside ConstantArrayTypeBuilder::setOffsetValueType ?

[ondrejmirtes]

This touches similar places the unsealed branch touches. Please fix this only on 2.2.x after the unsealed PR is merged (soon).

[phpstan-bot]

Pushed. Here's a summary of the changes made to address the reviewer's feedback:

What changed: Moved the union offset value-merging logic from the callers (flipArray() and fillKeysArray()) into ConstantArrayTypeBuilder::setOffsetValueType() itself, as the reviewer suggested.

How: Added a bool $mergeUnionValues = false parameter to setOffsetValueType() that controls two behaviors in the union-offset code path:

1. "All match" case (line 283): When $mergeUnionValues is true, values are always unioned with existing values instead of being replaced. Without it, !$optional writes to optional keys replace values (correct for $result[$k] = 2 semantics).

2. "Partial match" case: When $mergeUnionValues is true, matched keys get their values unioned and unmatched keys are added as new optional entries — instead of degrading to a general array type.

Why a parameter is needed: The builder can't distinguish between "same runtime key, new value" (variable assignment — replace is correct) and "independent source elements mapping to overlapping output keys" (flipArray — union is correct) without caller context. The default false preserves existing behavior for all current callers.

Side benefit: The ArrayFillKeysFunctionReturnTypeExtension no longer needs its own NonEmptyArrayType wrapping, since the builder's isNonEmpty is now correctly set via the $optional=false parameter that callers pass through.

[VincentLanglet]

Given the fact we want to simulate the $optional behavior, what about calling the builder with something like

$builder->setOffsetValueType(
				$valueType->toArrayKey(),
				$keyType,
				$this->isOptionalKey($i)->or(...),
			);

instead of introducing a new param

[phpstan-bot]

Pushed. Here's what changed to address VincentLanglet's review:

Removed the $mergeUnionValues parameter from ConstantArrayTypeBuilder::setOffsetValueType() entirely. Instead, the callers (flipArray() and fillKeysArray()) now compute $optional correctly: when the offset type has multiple constant scalar types (i.e., it's a union), each individual key is inherently optional, so $optional = true.

Builder changes (2 lines):

• Line 284: reverted to the original if (!$optional && ...) — since union offsets now pass $optional=true, values are unioned instead of replaced
• Line 306: changed if (!$hasMatch || $mergeUnionValues) to if (!$hasMatch || $optional) — partial matches with optional offsets keep matched values and add unmatched entries instead of degrading

Extension change: Since $optional=true prevents the builder from setting isNonEmpty, ArrayFillKeysFunctionReturnTypeExtension now wraps with NonEmptyArrayType when the input is non-empty (matching what ArrayFlipFunctionReturnTypeExtension already does), with a guard for non-array results (ErrorType).

All tests pass (12122 tests, 79747 assertions) and make phpstan reports no errors.

[VincentLanglet]

Remove any changes in the ConstantArrayBuilder

[phpstan-bot]

Pushed. The change reverts all modifications to ConstantArrayTypeBuilder.php and instead decomposes union offset types in the callers (flipArray() and fillKeysArray()) — each individual scalar type is passed to the builder separately with optional=true, which correctly unions values and avoids partial-match degradation without touching the builder. All 12122 tests pass and make phpstan reports no errors.

[VincentLanglet]

This touches similar places the unsealed branch touches. Please fix this only on 2.2.x after the unsealed PR is merged (soon).

I simplified the fix in order to minimize the possible conflict ; is it simple enough to be merge on 2.1.x branch now @ondrejmirtes ?

[ondrejmirtes]

I don't know yet. Please already think about how this change/fix would impact unsealed array shapes, what the fix would be on 2.2.x once merged.

[VincentLanglet]

I don't know yet. Please already think about how this change/fix would impact unsealed array shapes, what the fix would be on 2.2.x once merged.

Looking at the implementation of unsealed array shaped on 2.2.x. I would say the answer is simple ; there is no conflict between this PR and the unsealed changes.
I change this line

phpstan-src/src/Type/Constant/ConstantArrayType.php

Line 1518 in f7fb170

$this->isOptionalKey($i),

While you added extra logic about sealed later

phpstan-src/src/Type/Constant/ConstantArrayType.php

Line 1522 in f7fb170

if ($this->isUnsealed()->yes() && $this->unsealed !== null) {

[ondrejmirtes]

What I mean is that - if you produce more similar NSRT tests with unsealed types involved, does this mean we need to write some more code on 2.2.x in order to fix them? So my question is - is there a similar bug that needs a fix for example when the unsealed key or value is a union of constant scalars...

[VincentLanglet]

What I mean is that - if you produce more similar NSRT tests with unsealed types involved, does this mean we need to write some more code on 2.2.x in order to fix them? So my question is - is there a similar bug that needs a fix for example when the unsealed key or value is a union of constant scalars...

Yes, I opened #5722 for the similar bug with unsealed keys. I'd like your comment on the right behavior to you (especially in how the array should be described)

[ondrejmirtes]

It'd be cool to finish this but some work still has to be done.

[VincentLanglet]

Duplicates the tests written in bug-14656.php

but with unsealed arrays.

[phpstan-bot]

Done. Here's a summary of how I addressed VincentLanglet's review.

What the review asked

Add the bug-14656 tests duplicated with unsealed arrays (the ...<int, V> tail syntax introduced by the 2.2.x unsealed-array-shapes work).

What I found

While adding the unsealed mirrors, I discovered the two original (sealed) expectations in bug-14656.php were already failing on this branch — the 2.2.x merge made array_flip/array_fill_keys on overlapping union values more precise. Flipping array{0: 'a'|'b', 1: 'b'|'c'} now yields the exact shape non-empty-array{a?: 0, b?: 0|1, c?: 1} ('a' only comes from key 0, 'b' from either, 'c' only from key 1) instead of the degraded non-empty-array<'a'|'b'|'c', 0|1>. The new output is correct, so I updated the expectations rather than the code.

Changes (two commits)

1. Update stale expectations — the two sealed cases now assert the precise array shapes the 2.2.x branch produces.
2. Add unsealed variants — two new classes (ArrayFlipUnsealedUnionValues, ArrayFillKeysUnsealedUnionValues) mirroring all the sealed cases with ...<int, V> tails, covering:
   • finite union tail (...<int, 'a'|'b'|'c'>) → decomposes into optional explicit slots merged with the explicit keys
   • non-finite tail (...<int, string>) → survives as unsealed extras (...<string, int> / ...<string, 'x'>), with overlapping explicit values widened

Verification

• make phpstan — no errors
• Full NodeScopeResolverTest (1648 tests) — all pass, including bug-14656.php

The existing source handling of unsealed extras in flipArray()/fillKeysArray() already produces correct results for these cases, so no source change was needed — confirming ondrejmirtes's question that the union-of-constant-scalars value case is already covered for unsealed arrays (the open unsealed-key bug is tracked separately in #5722).

[VincentLanglet]

It'd be cool to finish this but some work still has to be done.

It's finished if you want to take a look @ondrejmirtes

[ondrejmirtes]

Thank you!