Skip to content

Conjoin alternative-form entries in SpecifiedTypes::unionWith() instead of keeping only the left one - #6173

Merged
ondrejmirtes merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-3hz2v6f
Aug 2, 2026
Merged

Conjoin alternative-form entries in SpecifiedTypes::unionWith() instead of keeping only the left one#6173
ondrejmirtes merged 1 commit into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-3hz2v6f

Conversation

@phpstan-bot

Copy link
Copy Markdown
Collaborator

Summary

SpecifiedTypes::unionWith() — the both-sides-hold merge behind the truthy narrowing of && and the falsey narrowing of || — merged its two sides' alternative-form entries with array +. Array + keeps the left operand on key collision, so when both sides carried an alternative-form entry for the same expression, the right one was silently dropped and one conjunct of a composed && narrowing was lost.

In the reported snippet both !(is_int($v) && $v < 0) and !(is_int($v) && $v >= 1) are falsey BooleanAnd merges, so each contributes an alternative-form entry keyed on $v; the && between them unioned the two and kept only the first, narrowing $v to int<0, max> instead of 0.

The fix cross-products the two term lists, and repairs the two tryRemove() implementations that otherwise make the conjoined terms evaluate back to the unnarrowed type.

Changes

  • src/Analyser/SpecifiedTypes.php
    • unionWith() now conjoins colliding alternative-form entries via the new conjoinTerms() instead of $this->alternativeTypes + $other->alternativeTypes.
    • New conjoinTerms(): an entry's value is the union of its terms, so the conjunction distributes into the cross-product — each pair contributes (sureA and sureB) minus (subtractA or subtractB), the same folding collectTerms() already does for a sure/sure-not pair on one key. A fixed base with a subtraction is folded into the narrower base; pairs that collapse to never drop out.
    • New dedupeTerms() and widenTerms() plus ALTERNATIVE_TERMS_LIMIT: pruning and dedup keep the term list flat in practice; past the limit the entry is widened to a single covering term (union of the sures, intersect of the subtracts), which only loses precision.
    • New unionAll(): the n-ary equivalent of folding unionWith(), combining each expression's constraints in one pass so the flattened chain paths stay linear in the number of arms. It carries alternatives, overwrite, root expr, conditional-expression holders, holder recipes and deferred augments, all of which the hand-rolled merges dropped.
    • mergeRootExpr() made static so unionAll() can use it.
  • src/Type/MixedType.phptryRemove() now subtracts unless the type is already eliminated, instead of requiring isSuperTypeOf() to be yes.
  • src/Type/ObjectWithoutClassType.php — the same fix for the structurally identical tryRemove().
  • src/Analyser/ExprHandler/BooleanAndHandler.phpspecifyTypesForFlattenedBooleanAnd() now calls SpecifiedTypes::unionAll().
  • src/Analyser/ExprHandler/BooleanOrHandler.phpspecifyTypesForFlattenedBooleanOr()'s falsey branch now calls SpecifiedTypes::unionAll().
  • tests/PHPStan/Analyser/nsrt/bug-1233.php, tests/PHPStan/Analyser/nsrt/bug-3991.php — expectations updated to the now-more-precise subtractions (mixed~array<mixed, mixed>mixed~iterable, array{}array<mixed> inside a subtraction).

Root cause

Three instances of the same pattern: a merge that reconciles same-kind constraints but silently keeps only one side of a cross-kind (alternative-form) constraint.

  1. unionWith() used array + for alternativeTypes. intersectWith() has collectTerms() to reconcile constraints of differing kinds; unionWith() had no equivalent for alternative-vs-alternative. Regression from the switch to symbolic alternative-form entries (Replace SpecifiedTypes::normalize() with symbolic alternative-form entries #6133), where the old normalize() had eagerly collapsed each side to a single sure type against a scope, so the collision never arose.

  2. specifyTypesForFlattenedBooleanAnd() and specifyTypesForFlattenedBooleanOr() (the O(N) paths taken for chains deeper than BOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH) reimplement the merge over getSureTypes() / getSureNotTypes() only, so they dropped alternative-form entries outright. The && one additionally combined colliding sure types with TypeCombinator::union() where unionWith() intersects — in a truthy && all arms hold, so two sure constraints on one expression must intersect. Routing both through unionAll() makes the flattened path equivalent to folding unionWith() by construction.

  3. MixedType::tryRemove() and ObjectWithoutClassType::tryRemove() shared a 4-line body gated on isSuperTypeOf($typeToRemove)->yes(). Once a previous subtraction is recorded, isSuperTypeOf() drops to maybe for any partially-overlapping type, so TypeCombinator::remove(mixed~int<1, max>, int) returned mixed~int<1, max> — nothing removed. Both are top types (of everything / of the object hierarchy), so removal is exactly the subtraction: T~X minus Y is T~(X|Y). This is what made the correctly conjoined terms above still evaluate to int rather than 0. The other tryRemove() implementations (IntegerType, IntegerRangeType, ArrayType, IterableType, BooleanType, StringType, UnionType) dispatch on the type being removed rather than gating on a subtraction and are unaffected.

Test

tests/PHPStan/Analyser/nsrt/bug-15039.php, covering the reported bug plus every analogous case probed:

  • repro() — the playground snippet verbatim; 0 instead of int<0, max>.
  • chained(), doubleNegation() — the extension-style is_int($v) && ... chain and the if (!(!(...))) form from the report.
  • threeAlternatives() — three colliding alternative forms, int<0, 2>|int<4, 5>.
  • alternativeWithExtraTerm() — one side's alternative form carrying the extra term of an inner ||, exercising a 3×2 cross-product; -5|0 instead of -5|int<0, max>.
  • alternativeAndSureType() — alternative form and a plain sure entry on the same expression (this one already passed; kept as a guard on the same code path).
  • flattenedAnd(), flattenedLogicalAnd(), flattenedOr(), shallowOr() — the flattened deep-chain paths and their shallow controls.
  • flattenedAndCollidingSureTypes() / shallowAndCollidingSureTypes() — the union-vs-intersect bug in the flattened && path; int instead of float|int|string.
  • subtractedObject()object~ArrayObject minus Traversable is object~Traversable, not object~ArrayObject.

Nine of the ten assertion groups fail on the unpatched source. Also verified: make tests, make phpstan, and make cs are green, and a 120-arm || chain and a 40-conjunct alternative-form chain analyse in the same wall-clock time as before (no cross-product blowup). make name-collision fails on tests/PHPStan/Build/data/final-class-rule-pipe.php both with and without this change — a pre-existing parse failure unrelated to it.

Fixes phpstan/phpstan#15039

…tead of keeping only the left one

* `unionWith()` merged the two sides' alternative-form entries with array `+`,
  which keeps the left operand on key collision and silently dropped the right
  one. Both sides now cross-product their terms: an entry's value is the union
  of its terms, so conjoining two entries distributes over both lists into
  `(sureA and sureB) minus (subtractA or subtractB)` per pair. Impossible pairs
  drop out, duplicates are deduped, and `ALTERNATIVE_TERMS_LIMIT` widens an
  entry to a single covering term if a long chain ever grows the cross-product.
* `MixedType::tryRemove()` and `ObjectWithoutClassType::tryRemove()` gave up
  whenever an earlier subtraction had lowered `isSuperTypeOf()` from yes to
  maybe, removing nothing at all. Both are top types, so removal is exactly the
  subtraction; they now only bail when the type is already eliminated. Without
  this, the conjoined terms above still evaluate to the unnarrowed type.
* Added `SpecifiedTypes::unionAll()` and used it from the flattened deep-chain
  paths in `BooleanAndHandler` and `BooleanOrHandler`, which reimplemented the
  merge by hand: both dropped alternative-form entries entirely, and the `&&`
  one combined colliding sure types with `union()` where `unionWith()`
  intersects, widening `(is_int($v) || is_string($v)) && (is_int($v) ||
  is_float($v))` back to `float|int|string`.
* Probed and found already correct: an alternative form meeting a plain
  sure/sure-not entry on the same expression (they conjoin at the application
  point), `intersectWith()`'s own alternative handling, and the flattened
  truthy `||` path, which folds `intersectWith()`.
@ondrejmirtes
ondrejmirtes merged commit 0af438b into phpstan:2.2.x Aug 2, 2026
739 of 745 checks passed
@ondrejmirtes
ondrejmirtes deleted the create-pull-request/patch-3hz2v6f branch August 2, 2026 21:30
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.

SpecifiedTypes::unionWith() drops alternative-form entries on key collision

2 participants