Do not flatten template benevolent unions in TypeCombinator::union()

[gnutix]

Closes phpstan/phpstan#7279

Root cause

@template K of array-key produces a TemplateBenevolentUnionType (array-key = benevolent int|string). When resolving a PHPDoc union such as K|null (e.g. in a conditional return type ($array is non-empty-array ? K|null : null)), TypeCombinator::union()'s "transform A | (B | C) to A | B | C" loop hit the instanceof BenevolentUnionType flattening branch before the instanceof TemplateType → continue guard that already protects TemplateUnionType (which is why @template K of int|string worked fine). The template's members were spliced into the outer union, so the template was destroyed at PHPDoc-resolution time — this was never an inference failure; by the time inference ran, K no longer existed and only its bound int|string remained.

The reconstruction path at the end of union() only re-wrapped the template when all resulting union members came from the benevolent union, which fails as soon as any other member (here null) is present.

Fix

Exclude TemplateType from the benevolent flattening branch, letting TemplateBenevolentUnionType fall through to the same TemplateType guard as TemplateUnionType. This mirrors 57e3cbf, which fixed the identical problem on the intersect() side (templates handled before benevolent flattening and re-wrapped via TemplateTypeFactory); union() never got the same treatment.

With the guard in place, the $benevolentUnionObject re-wrapping machinery in the reconstruction path became unreachable (confirmed by PHPStan self-analysis flagging instanceof between null and TemplateBenevolentUnionType will always evaluate to false) and is removed. Plain (non-template) benevolent unions still enter the flattening branch and behave exactly as before.

Semantic notes

• union(K of array-key, int) now yields int|K instead of the widened K — consistent with how non-benevolent template unions (K of int|string) already behave.
• stubs/ArrayObject.stub line 23 (@param TKey|null $offset) now resolves to a real template union instead of a flattened bound, which can make ArrayObject/ArrayIterator offset parameter checks slightly stricter.

No existing test expectations needed updating; full NodeScopeResolverTest, AnalyserIntegrationTest, TypeCombinatorTest, UnionTypeTest, BenevolentUnionTypeTest, generics tests, and PHPStan self-analysis all pass.

🤖 Generated with Claude Code, model Fable 5.

[VincentLanglet]

Hi @gnutix please target 2.2.x for all your PR

[gnutix]

@VincentLanglet done !

[staabm]

please rebase and have a look at the integration tests.

old PHPUnit test is sometimes flaky and false positives.
the e2e test failure is unrelated.

issue-bot also need to be double checked, how this PR affects other reported phpstan issues

[gnutix]

please rebase and have a look at the integration tests.
old PHPUnit test is sometimes flaky and false positives. the e2e test failure is unrelated.
issue-bot also need to be double checked, how this PR affects other reported phpstan issues

Rebased all six onto latest 2.2.x; the old-PHPUnit flakiness and the e2e failure are both accounted for (agreed they're unrelated to these changes), and the mutation/line-shift issues that were real have been fixed.

[staabm]

What about the 2 integration tests?

[gnutix]

The two integration tests are caused by a widening of the array-key type now being int|string where it used to collapse to string/TKey. I must admit I don't know if it's the right thing here, or if this PR's changes are too "aggressive". We could refine TypeCombinator::union() so it stops flattening but still collapses int|TKey correctly, or we could adapt Larastan and doctrine/collections repo to this new change. Your call, I can't tell.

(I don't understand most of these PR, I'm just guiding Claude)

[VincentLanglet]

I don't understand most of these PR, I'm just guiding Claude

I'd like to flag a concern with the approach here

A PR carries an implicit contract: the submitter has understood the change and is asking maintainers to verify it. When the submitter can't vouch for the code, that contract breaks down in a few ways:

• Review burden is inverted. Maintainers end up doing the actual engineering work instead of reviewing it. It takes minutes to generate a PR like this, but real effort to evaluate it — and maintainer time is the scarcest resource in open source.
• No one is accountable. If a reviewer asks "why did you handle this edge case this way?", the answer can only be relayed back to the AI. And if the change breaks something post-merge, there's no human who understood it well enough to help debug or maintain it.
• Risk shifts downstream. AI-generated code can look plausible while being subtly wrong. A submitter who understands their code acts as a first filter; without that, 100% of the risk lands on reviewers, who now have to review with more suspicion, not less.
• It doesn't scale. If one person does this, hundreds can. Popular projects are already dealing with floods of low-effort AI submissions.

To be clear: using Claude or any AI tool is completely fine — many great contributors do. The line is being able to say "I understand and stand behind every line of this." If you can get to that point with this PR, we would genuinely welcome the contribution. But if the purpose is just to generate PR with Claude, we already have a bot for this.

[gnutix]

I understand your stance, and it's fine if you decide to save your time and close this PR. On my end, I simply cannot allocate the time that would be required for me to "level up" my PHPStan knowledge enough to be able to be accountable for the changes in this PR. I'm relying on the quality and width of the project's test suite (which is quite impressive at this point).

I've opened these as issues months/years ago, and no one have had the opportunity to get onto them (and that's fine and expected). Today with AI, especially powerful models like Fable (which is available "at a reasonable price" only for a few days still), I thought I'd get 90% of the job done while still having a good enough level of confidence that it's not "utter shit". But that doesn't mean I can get it past the finish line alone, so I'm requesting your help for the missing 10%. And maybe you'll have time for it, and maybe not, and that's fine and expected too.

I'm much aware that if any of you guys had the time to work on that issue, you'd have done a better job (AI assisted or not) than me. But tokens cost (especially Fable's), your time cost, so I don't think it's necessarily a bad thing that I took care of some of it for you.

PS: "It takes minutes to generate a PR like this" : it took me a good 4-6 hours and ~2.7M tokens (≈$232) to do those 6 PRs, make sure I drilled the AI enough so it doesn't take shortcuts, make all the CI pass, and testing the fixes against projects I have access to.