Conjoin alternative-form entries in SpecifiedTypes::unionWith() instead of keeping only the left one - #6173
Merged
ondrejmirtes merged 1 commit intoAug 2, 2026
Conversation
…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()`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 falseyBooleanAndmerges, so each contributes an alternative-form entry keyed on$v; the&&between them unioned the two and kept only the first, narrowing$vtoint<0, max>instead of0.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.phpunionWith()now conjoins colliding alternative-form entries via the newconjoinTerms()instead of$this->alternativeTypes + $other->alternativeTypes.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 foldingcollectTerms()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 toneverdrop out.dedupeTerms()andwidenTerms()plusALTERNATIVE_TERMS_LIMIT: pruning and dedup keep the term list flat in practice; past the limit the entry is widened to a single covering term (unionof the sures,intersectof the subtracts), which only loses precision.unionAll(): the n-ary equivalent of foldingunionWith(), 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 sounionAll()can use it.src/Type/MixedType.php—tryRemove()now subtracts unless the type is already eliminated, instead of requiringisSuperTypeOf()to beyes.src/Type/ObjectWithoutClassType.php— the same fix for the structurally identicaltryRemove().src/Analyser/ExprHandler/BooleanAndHandler.php—specifyTypesForFlattenedBooleanAnd()now callsSpecifiedTypes::unionAll().src/Analyser/ExprHandler/BooleanOrHandler.php—specifyTypesForFlattenedBooleanOr()'s falsey branch now callsSpecifiedTypes::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.
unionWith()used array+foralternativeTypes.intersectWith()hascollectTerms()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 oldnormalize()had eagerly collapsed each side to a single sure type against a scope, so the collision never arose.specifyTypesForFlattenedBooleanAnd()andspecifyTypesForFlattenedBooleanOr()(the O(N) paths taken for chains deeper thanBOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH) reimplement the merge overgetSureTypes()/getSureNotTypes()only, so they dropped alternative-form entries outright. The&&one additionally combined colliding sure types withTypeCombinator::union()whereunionWith()intersects — in a truthy&&all arms hold, so two sure constraints on one expression must intersect. Routing both throughunionAll()makes the flattened path equivalent to foldingunionWith()by construction.MixedType::tryRemove()andObjectWithoutClassType::tryRemove()shared a 4-line body gated onisSuperTypeOf($typeToRemove)->yes(). Once a previous subtraction is recorded,isSuperTypeOf()drops tomaybefor any partially-overlapping type, soTypeCombinator::remove(mixed~int<1, max>, int)returnedmixed~int<1, max>— nothing removed. Both are top types (of everything / of the object hierarchy), so removal is exactly the subtraction:T~XminusYisT~(X|Y). This is what made the correctly conjoined terms above still evaluate tointrather than0. The othertryRemove()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;0instead ofint<0, max>.chained(),doubleNegation()— the extension-styleis_int($v) && ...chain and theif (!(!(...)))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|0instead 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;intinstead offloat|int|string.subtractedObject()—object~ArrayObjectminusTraversableisobject~Traversable, notobject~ArrayObject.Nine of the ten assertion groups fail on the unpatched source. Also verified:
make tests,make phpstan, andmake csare 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-collisionfails ontests/PHPStan/Build/data/final-class-rule-pipe.phpboth with and without this change — a pre-existing parse failure unrelated to it.Fixes phpstan/phpstan#15039