Answer UnionType comparisons from an identity-keyed FiniteTypeSet instead of scanning every member

[phpstan-bot]

Summary

Comparing a union of constant strings against another one cost one isSuperTypeOf() call per
pair of members. Membership among such values is exact-value equality, so it is a set lookup:
this PR keys a union's members by value identity once, caches that on the union, and answers
isSuperTypeOf(), isSubTypeOf(), accepts(), isAcceptedBy(), equals() and tryRemove()
from the map. O(n*m) member comparisons become O(n+m), and a single-value lookup becomes O(1).

The issue asked for a trie. A trie earns its keep on prefix or pattern queries — a router
matching URIs — and none of these comparisons are that (ConstantStringType::isSuperTypeOf()
on another constant string is value === value). As noted in the issue's comment thread, an
identity-keyed hash is the structure this actually wants, and it answers the same question for
constant ints, bools, null and enum cases, so all of them are keyed, not just strings.

Measured on 400-member unions (UnionType API directly, before → after):

operation	identical unions	disjoint unions
isSuperTypeOf	204 ms → 8 ms	575 ms → 6 ms
isSubTypeOf	203 ms → 6 ms	575 ms → 6 ms
accepts	2139 ms → 7 ms	2355 ms → 13 ms
isAcceptedBy	1504 ms → 6 ms	1731 ms → 12 ms
isSuperTypeOf(single value)	240 ms → 2 ms

End to end, analysing tests/bench/data/big-constant-string-union.php goes from 1.46 s to
0.76 s. PHPStan's own self-analysis is unchanged (55.8 s → 55.4 s), so ordinary code pays
nothing for the map.

Changes

• src/Type/FiniteTypeSet.php (new) — a union's members indexed by value identity. Keys
  null, constant scalars other than floats, and bare enum cases; everything else goes to
  $others, so a single object type next to fifty constant strings does not defeat the
  optimization, it only means callers still consult those few members the slow way. Also
  exposes containedIn() for union-vs-union answers and hasClassStringMember() for callers
  that hand a member back rather than only comparing values.
• src/Type/UnionType.php — builds the set lazily once per instance
  (getFiniteTypeSet()) and uses it in isSuperTypeOf(), isSubTypeOf(), accepts(),
  isAcceptedBy(), equals() and tryRemove().
• src/Type/TypeCombinator.php — finiteUnionMembers() reuses the shared, cached set
  instead of keying the union again on every intersect(). Its class-string bail is kept: the
  class-string flag is part of a constant string's representation but not of its value, and
  intersect() hands a member back.
• build/PHPStan/Build/SkipTestsWithRequiresPhpAttributeRule.php — the rule demanded an
  inverse comparison operator before checking that the comparison was about PHP_VERSION_ID
  at all, so a test method whose first statement is an if on any other binary operator (a
  || chain, say) crashed the analysis with ShouldNotHappenException. The new test file hit
  it. The PHP_VERSION_ID check now comes first.
• tests/bench/data/big-constant-string-union.php (new) — benchmark input.

Root cause

UnionType answered every comparison by walking $types and asking each member, and the
callers that pass a union walk it again — so union-vs-union was O(n*m) virtual calls. For
members that stand for one concrete value that scan is pure waste: two of them are
interchangeable iff they are equals(), and any two that are not equals() are disjoint. A
key derived from the value settles both questions with one hash lookup.

The same shape repeated across the whole comparison family, which is why the fix does too:

• isSuperTypeOf(value) — yes if the map holds it; otherwise every keyed member answers no,
  so only unkeyed members are left to ask.
• isSubTypeOf(union) / equals(union) — set containment between the two maps.
• accepts(value) — only the positive answer is value identity; a member that does not hold
  the value can still accept it through scalar coercion. So the miss is not simply no, but it
  is the same for every member of a kind (all constant strings behave alike as acceptors, and
  so do all cases of one enum), and consulting one member per kind is enough.
• accepts(union) — a member standing for a single value never accepts more than one of the
  other union's values, so the member-by-member or() cannot come out yes and its result is
  discarded for the compound answer anyway; go straight to isAcceptedBy().
• tryRemove(value) — drop the one member under that key instead of running
  TypeCombinator::remove() against every member.

Floats are the one finite scalar left out on purpose: equals() does not agree with value
identity for them (-0.0 === 0.0, NAN !== NAN). Template types are excluded outright, their
comparison semantics are not value identity, and a type that merely contains a value — an
intersection with an accessory type, a whole single-case enum, a conditional type resolving to
a constant — is excluded by an equals() check against its own constant scalar type.

Test

tests/PHPStan/Type/FiniteTypeSetTest.php. The map is an optimization, so the property worth
testing is that it never disagrees with comparing member by member: each test runs the real
operation against a reference implementation that spells out the loop UnionType used before,
over a matrix of 17 unions × 18 query types (and 17 × 17 union pairs) — 2397 cases covering
constant strings, ints, bools, null, enum cases, mixed-kind unions, unions diluted with an
object / a float / a general string / a class-string constant, and BenevolentUnionType.
accepts() additionally asserts that no member attaches a reason to its answer, which is what
lets one member of a kind stand in for the rest without changing error messages.

The reference implementations caught two real mistakes while writing them (the accepts()
compound-delegation tail and BenevolentUnionType::isAcceptedBy()'s or-semantics), and
mutating the fix afterwards — inverting the accepts() union shortcut, dropping the
completeness guard in isSuperTypeOf(), dropping the equals() guard in
FiniteTypeSet::key() — fails 254, 6 and 12 cases respectively.

Being an optimization, none of this fails without the fix; the benchmark data file and the
numbers above are what show the change did something. make tests (20029 tests), make phpstan and make cs are green. make name-collision fails identically with and without the
change (tests/PHPStan/Build/data/final-class-rule-pipe.php uses PHP 8.5 pipe syntax that the
detector's parser rejects on this PHP version).

Probed and deliberately left alone:

• TypeCombinator::union() already collapses constant scalars and enum cases through
  describe-keyed maps, so it is O(n) — no change needed.
• TypeCombinator::doRemove()'s finite-type loop is O(n*m), but for a union of values it is
  unreachable: isSuperTypeOf() settles the removal before it, and tryRemove() now handles
  the hit.
• IntersectionType, the composite sibling, gets nothing: its members are not
  mutually-disjoint values, so identity keying does not apply.
• UnionType::getFiniteTypes() dedupes by describe(VerbosityLevel::cache()). Switching it to
  identity keys would collapse a class-string constant with a same-valued plain one, which
  describe() keeps apart — left as is.

Fixes phpstan/phpstan#15008

[SanderMuller]

Nice work. It follows the issue thread faithfully (identity-keyed map, not a trie, extended to the sibling operations) and the benchmarks look right. I checked the correctness-critical parts and they hold up: the six fast paths reproduce the slow-path results (all-in yes, none-in no, mixed maybe), isComplete()/$others keeps mixed unions on the slow path, and the accepts() representatives-of-other-kinds path is sound (I verified the numeric-string coercion case that could break it).

What keeps it from green, all outside the optimization:

1. Self-analysis: 15 "Class ManyCasesTestEnum not found" errors (and old-PHPUnit 7.4). The test imports the enum fixtures with use + ::class, forcing resolution under the <8.1 phpVersion analysis. UnionTypeTest avoids this by referencing them as string literals (new EnumCaseObjectType('PHPStan\Fixture\ManyCasesTestEnum', 'A')); matching that should clear both jobs.
2. The SkipTestsWithRequiresPhpAttributeRule change looks out of scope, only there so the test's version guard does not trip the rule. Better split out or restructured away.
3. Mutation testing is red: the subtle branches (representative loop, containedIn thresholds, $others gating) want tests that pin them directly.

(Rector/shipmonk are the repo-wide flakes, not this PR.) The algorithm and edge handling are solid; these are all peripheral.

[phpstan-bot]

Pushed. All three review points addressed, one commit each.

1. Self-analysis: 15 class.notFound errors (90b4a0a)

Confirmed and reproduced first — analysing the test file with phpVersion: 80000 and build/enums.neon (which excludes the three enum fixture files below PHP 8.1) gave exactly 15 Class PHPStan\Fixture\ManyCasesTestEnum not found. Switched to string literals as UnionTypeTest and EnumCaseObjectTypeTest do, and left a note in the class doc saying why, so it doesn't get tidied back into an import. Re-run under the same simulated config: 0.

2. SkipTestsWithRequiresPhpAttributeRule (abed2ab)

Reverted to the pre-PR state, byte-identical. I checked before reverting whether the test still needs it: with the rule restored, bin/phpstan analyse -c build/phpstan.neon tests/PHPStan/Type/FiniteTypeSetTest.php is clean. The reordering was carried over from an earlier draft that skipped on a PHP_VERSION_ID guard; the test uses #[RequiresPhp] now. The latent crash it fixed is real but unrelated to keying unions, so it belongs in its own change with its own test.

3. Mutation testing (4c0234d)

Worth flagging: infection is configured with --source-directory='build/PHPStan/Build', so the only mutable source in this PR was the rule file — the red MSI came from the untested lines the reordering added, and reverting it in #2 removes the mutants. Your underlying point still stands, though: the matrix reached those branches only sideways. So I pinned them head on — containedIn() at all three thresholds including both boundaries, the single-kind representative case, the four isComplete() gates, and hasClassStringMember().

I verified these are load-bearing rather than decorative by mutating the source ten ways and confirming each dies against the new tests alone (filtered to just them):

mutation	killed
containedIn no-threshold off $other's count	✓
containedIn swapped array_diff_key() args	✓
containedIn === 0 → >= 0	✓
representative loop without the kind continue	✓
hasClassStringMember without the isClassString() guard	✓
isComplete() → true	✓
isSuperTypeOf / tryRemove / finiteTypeSetContainedIn / equals without their gate	✓ (4)

Sources restored to pristine afterwards (git diff src/ empty).

make tests (20348, 97 skipped — same as before), make phpstan and make cs are green.

[staabm]

how is the impact of this change on phpstan/phpstan#15004?

[phpstan-bot]

Pushed as 032c1284a. Here's the answer to the review question.

Impact on phpstan/phpstan#15004

I measured it rather than estimated it. Reproducer is the issue's own shape — a 1|2|…|N parameter narrowed by a chain of N !== clauses — run through analyse -l 8:

N	before this PR	PR as you reviewed it	+ the commit I just pushed
50	1.30 s	1.20 s	1.20 s
100	3.11 s	2.70 s	2.41 s
200	15.02 s	12.92 s	10.42 s
300	46.27 s	39.57 s	30.95 s

The PR as you reviewed it helped #15004 by about 15%. That is less than I'd have guessed, so I profiled instead of leaving it there.

What the profile showed

Counting calls at N=100: 79 674 TypeCombinator::doRemove() calls costing 0.92 s, and that 0.92 s splits almost evenly into UnionType::isSubTypeOf() at 0.40 s and UnionType::tryRemove() at 0.47 s.

tryRemove() is the one this PR already keyed. isSubTypeOf() was not — doRemove() opens with $typeToRemove->isSuperTypeOf($fromType), which lands on UnionType::isSubTypeOf() through the CompoundType delegation, and finiteTypeSetContainedIn() only ever compared two unions. So a union of values asked about one value still walked every member. Roughly half the remaining hot path was left on the table by the PR's own logic.

What I pushed

A single value is a one-member set, so the map answers it: the union is under that value only if it has nothing else in it, and disjoint from it if the key is missing. FiniteTypeSet::containedInKey() is containedIn() for that case; the helper now treats a non-union other type as the set holding just that value. That's the third column above — 1.5× versus main at N=300, and it's the same identity-map idea, not a new mechanism.

isAcceptedBy() shares the helper and keeps its yes-only guard. Worth flagging honestly: no constant value accepts another one — not a numeric string an int, in either direction — so that guard provably cannot change an answer today, and mutating it away leaves all tests green. I kept it and said so in a comment, because that's a fact about the current value types rather than something the map guarantees.

What it does not do

This is a constant factor. It is not the fix #15004 wants. Growth stays at roughly O(N^2.7) — base scales 3.08× from N=200→300, mine 2.97×. Both isSubTypeOf() and tryRemove() are inherently O(n) per call (the latter has to build an (n−1)-member union), against an O(n) chain of O(n) removals. Only the issue's Option B — not making the removals at all — changes the exponent. A 400-clause case will still not be single-digit seconds.

Verification

make tests 21039 tests / 97 skipped (same skips as before), make phpstan and make cs green. The reference-implementation matrix now runs isSubTypeOf() and isAcceptedBy() over the union/non-union pairs too, and asserts no member attaches a reason to an answer the map now gives on its behalf; four mutations of the new code fail 399, 1, 92 and 36 cases.

Two measurement notes, since both nearly misled me:

• I first read a 15% regression on big-constant-string-union.php and a 30% one on self-analysis. Both were artifacts — the first from measuring immediately after a git stash pop (cold), the second from a background test suite. Re-measured clean, six runs each: bench file 2.81 s either way, self-analysis 1m23/1m24 with the change against 1m25/1m25 without.
• I deliberately did not add a tests/bench/data/bug-15004.php. Sized to run in CI (N≈60, 1.3 s) the effect is ~7% and sits in the harness's ±20% noise band; sized to show it (N≥200) it costs >10 s per rev and the suite runs every file six times.