Fix oversized-array self-rejection in optimizeConstantArrays

[gnutix]

Summary

Fixes a regression introduced in 2.1.52 where the oversized-array generalization in TypeCombinator produced a type that was not a super-type of the variants it was derived from, causing generator.valueType (and similar) false positives where a closure's inferred Generator value type rejected the very yields it was synthesised from.

This work was authored by Claude (Opus 4.7) — bug analysis, root-cause diagnosis, and the patch. It bisected the regression to commit 2f66c45 ("Preserve constant array when assigning a union of scalars"), found that commit was itself a correct precision improvement, and located the two latent TypeCombinator bugs it exposed.

The fix is two changes in src/Type/TypeCombinator.php:

• processArrayTypes (~line 962): when applying common array accessory types to a reduced array result that is already known empty (isIterableAtLeastOnce()->no()), skip OversizedArrayType and NonEmptyArrayType. Otherwise the intersection produces a contradictory array{}&oversized-array that rejects the very value it represents.
• optimizeConstantArrays stage 2 inner traverser (~line 1071): fall through via $innerTraverse, not the outer $traverse. The outer callback fully generalizes a sealed ConstantArrayType into array<intKey, V>&..., which is correct at the top level but wrong inside a value position. Re-entering it through the outer traverser produced asymmetric treatment — a sealed array{a: 1} reached via array{}|array{a: 1} got fully generalized while one reached directly was only wrapped, leaving processArrayTypes with a mix of shapes it could not unify cleanly. The same callback also now skips wrapping empty constant arrays for the same array{}&oversized-array reason as Fix A.

Both parts were verified to be independently necessary: reverting either alone re-fails the targeted regression test. Claude decomposed Fix B into its two sub-changes and tested each in isolation.

A symptomatic playground reproducer is at phpstan.org/r/5b811780-35b4-4ebf-b2a6-cecb90f02b2b (discarded example - with numeric keys - at https://phpstan.org/r/916330d2-806c-426c-9220-cf99aba58a38).

Test plan

• New testBugYieldOversizedSelfRejection regression test in tests/PHPStan/Rules/Generators/YieldTypeRuleTest.php passes — tests/PHPStan/Rules/Generators/data/bug-yield-oversized-self-rejection.php is a domain-agnostic version of the playground reproducer that exhibits the bug at level max with bleedingEdge: true.
• Verified the new test data file still triggers the bug without the fix (9-error variant) and is clean with the fix.
• Reproduces and clears the original full reproducer (11 errors → 0) on the file the contributor first hit the bug on.
• make tests — 11993 tests, 79545 assertions, 0 new failures (2 pre-existing unrelated failures in AccessPropertiesInAssignRuleTest::testBug14063 and CatchWithUnthrownExceptionRuleTest::testBug14479 confirmed pre-existing on the unmodified 2.1.x HEAD).

[VincentLanglet]

The issue isn't simply the fact you're using numeric-string as key ?
https://phpstan.org/r/45ec940c-c633-4956-9ca6-36f1cd4699b3

See
https://phpstan.org/r/e41bc4d8-f494-48d4-922e-547bd424c3c6
vs
https://phpstan.org/r/218d2d56-edba-4e9b-a779-488c4e82569e

[gnutix]

@VincentLanglet Sorry, misleading playground example. Our project uses textual keys, Claude simplified with numeric keys when making a "generic" example. Here's a new one :

https://phpstan.org/r/5b811780-35b4-4ebf-b2a6-cecb90f02b2b

I've also adapted the test added in this PR to not use numeric keys, and it fails (locally) without the fix part too.

Edit: tested 2.1.54, same issue.

[VincentLanglet]

when applying common array accessory types to a reduced array result that is already known empty (isIterableAtLeastOnce()->no()), skip OversizedArrayType and NonEmptyArrayType. Otherwise the intersection produces a contradictory array{}&oversized-array that rejects the very value it represents.

While I understand it, I'm surprise by the fact we ending trying to add NonEmptyArrayType to an empty array in the first place... I'm not sure a dedicated check for this accessory is the right thing to do cause we could have the same with some HasOffsetType accessory for instance no ?
array{}&hasOffset('foo') makes no sens.

Do you mind opening a dedicated issue on https://github.com/phpstan/phpstan ?
I'll try to run our bot to see his analyse.

[gnutix]

Issue opened at phpstan/phpstan#14560. Claude is running on a deeper analysis from your feedback, I'll keep you posted on what it finds here as to not pollute your bot's investigation.

[gnutix]

Pushed 4e94226 — switches Fix A's predicate to !$accessory->accepts($emptyArray, true)->no() per your suggestion. Each claim below verified empirically before posting:

• The obvious predicate $accessory->isIterableAtLeastOnce()->yes() does not work for OversizedArrayType. (new OversizedArrayType())->isIterableAtLeastOnce()->describe() === "Maybe" (set in your 8d87c671c, "Solve 11703"). So the ->yes() predicate would fail to filter it in this fix's hot path. The accepts(array{}) predicate is the right one because the regression actually surfaces via accepts — YieldTypeRule::processNode calls RuleLevelHelper::accepts (src/Rules/Generators/YieldTypeRule.php:77).

• Today only OversizedArrayType leaks into the common accessory set via partial presence. The only branch in processArrayAccessoryTypes that adds an accessory when count($accessoryType) !== $arrayTypeCount (lines 832-837) is gated by instanceof OversizedArrayType. NonEmptyArrayType, HasOffsetType, HasOffsetValueType and AccessoryArrayListType are only added when all inputs carry them (lines 845, 840-843) or, for NonEmptyArrayType, via the all-isIterableAtLeastOnce()->yes() check at 848-849. So no empty input can reach those intersections via this path today — the general predicate is forward-defensive against new accessories rather than currently load-bearing for the existing ones.

• There's an accepts ↔ isIterableAtLeastOnce mismatch in OversizedArrayType. isIterableAtLeastOnce() returns Maybe, but accepts() at line 79 gates on $type->isIterableAtLeastOnce(), so (new OversizedArrayType())->accepts(new ConstantArrayType([], []), true)->no() === true. And TypeCombinator::intersect(new ConstantArrayType([], []), new OversizedArrayType()) produces list{}&oversized-array (not collapsed to Never), whose accepts(new ConstantArrayType([], []), true)->no() === true. That's the proximal cause of the regression: the union member rejects the very value it represents.

  I tested the obvious reconciliation — switching line 79 to use $type->isOversizedArray() (matching what isSuperTypeOf already does on line 92). Empirical result: that change alone does not fix the regression (the structural cause Fix B addresses is upstream of accepts), and combined with Fix A + Fix B it actively breaks the regression test. New failure mode: at level max with treatPhpDocTypesAsCertain, sibling union members like (list{...}&oversized-array) go from accepting concrete small lists with Yes (under the old isIterableAtLeastOnce predicate) to Maybe (under isOversizedArray), which RuleLevelHelper::accepts then rejects. So the simple reconciliation isn't sound — accepts can't just defer to isOversizedArray without weakening the sibling-union case. Outside our regression test, the change broke 0 other tests in make tests (3 total failures: ours + the 2 pre-existing unrelated ones). Reverted, not pursuing further here.

make tests is clean with the PR as it stands (same 2 pre-existing unrelated failures). Holding off on touching Fix B's $innerTraverse swap until your bot's results are in.

[VincentLanglet]

Issue opened at phpstan/phpstan#14560. Claude is running on a deeper analysis from your feedback, I'll keep you posted on what it finds here as to not pollute your bot's investigation.

Thanks, I'll run our bot on it to see his analyse too.

[VincentLanglet]

I dunno if our bot copied yours but it did the exact same fix https://github.com/phpstan/phpstan-src/pull/5575/changes ^^'

[gnutix]

It could have been polluted by reading this PR and all the details in it ? 🤷‍♂️

[ondrejmirtes]

I've got inspired by this and commited my own very similar fix (along with one more test): 9743346

Thank you.