Fix parameter type intersection for union/intersection method types#4936
Open
ondrejmirtes wants to merge 4 commits into2.1.xfrom
Open
Fix parameter type intersection for union/intersection method types#4936ondrejmirtes wants to merge 4 commits into2.1.xfrom
ondrejmirtes wants to merge 4 commits into2.1.xfrom
Conversation
Previously, getSelfOutType(), getAttributes(), and getResolvedPhpDoc()
on Union/IntersectionType{Method,Property}Reflection only used the first
member ($this->methods[0] / $this->properties[0]), losing information
from other members. This applies the same merging pattern from PR #4920
(which fixed assertion merging) to these other metadata methods:
- getSelfOutType(): Union self-out types for union types, intersect for
intersection types. Returns null if any member lacks a self-out type.
- getAttributes(): For union types, only keep attributes present in ALL
members (intersection semantics). For intersection types, collect
attributes from ANY member (union semantics), deduplicating by name.
- getResolvedPhpDoc(): Return null instead of arbitrarily returning the
first member's PHPDoc block, which could be misleading.
https://claude.ai/code/session_01KdvoAVF2YDBnuJ3gmgU9hQ
…rom all methods Previously, getVariants() used only $this->methods[0]->getVariants() for parameter structure, ignoring parameters from other methods in the intersection. This caused false positives when calling methods on intersection types where the member interfaces have different parameter types or parameter counts. For an intersection type A&B, the object satisfies both contracts, so: - Parameters should be UNIONED (the implementation handles both) - Return types should be INTERSECTED (must satisfy both) The fix uses ParametersAcceptorSelector::combineAcceptors() to properly merge parameter types and counts across all methods, then overrides the return types with the intersection (which combineAcceptors would union). Example: given AcceptsInt&AcceptsString where AcceptsInt::process(int) and AcceptsString::process(string), the combined method now correctly accepts int|string instead of only int. https://claude.ai/code/session_01KdvoAVF2YDBnuJ3gmgU9hQ
Union type method calls union parameter types instead of intersecting them. Entity1::setFoo(string) | Entity2::setFoo(?string) combined becomes setFoo(string|null), so passing null is not flagged. The sound behavior would intersect params to string, rejecting null. This is tracked in the test with a comment noting the open issue. https://claude.ai/code/session_01KdvoAVF2YDBnuJ3gmgU9hQ
…ypes For a union type A|B, we don't know which runtime type the object is, so arguments must be valid for ALL possible methods. This means parameter types should be intersected, not unioned. Previously, combineAcceptors() unioned parameter types, making string|null the combined type for string vs ?string — allowing null through when Entity1::setFoo(string) doesn't accept it. Now parameter types are intersected across methods: - string & (string|null) = string — correctly rejects null - int & string = never — NeverType::accepts() returns yes (bottom type semantics: unreachable code), so no false positives Performance: added a fast-path that skips intersection when all methods come from the same declaring class (e.g. 250-case enum unions), plus result caching on the instance. Fixes phpstan/phpstan#9664 https://claude.ai/code/session_01KdvoAVF2YDBnuJ3gmgU9hQ
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
2 participants
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
This PR fixes parameter type handling for union and intersection types by properly intersecting parameter types instead of unioning them. When a method is called on a union or intersection type, the argument must be valid for ALL possible methods, not just one.
Key Changes
UnionTypeMethodReflection:
$cachedVariantscombineAcceptorswhich unions types, now explicitly intersects parameter types across all methodsgetSelfOutType()to union the self-out types from all methods instead of returning nullgetAttributes()to return only attributes present in all methods (intersection semantics)getResolvedPhpDoc()to return null instead of just the first method's docIntersectionTypeMethodReflection:
getVariants()to return a single combined variant instead of mapping over the first method's variantsgetSelfOutType()to intersect the self-out types from all methods instead of returning nullgetAttributes()to return attributes from all methods (union semantics)getResolvedPhpDoc()to return null instead of just the first method's docUnionTypePropertyReflection and IntersectionTypePropertyReflection:
getAttributes()to properly handle attributes across multiple properties using intersection (union) semanticsImplementation Details
The core fix addresses issue #9664 where a union type
Entity1|Entity2with methodssetFoo(string)andsetFoo(?string)should intersect the parameter types tostring(the common type), not union them. This ensures type safety: an argument must satisfy ALL possible method signatures.For union types: parameter types are intersected (stricter requirement)
For intersection types: parameter types are unioned (more flexible requirement)
Added comprehensive test cases in
union-intersection-method-variants.phpandbug-9664.phpto verify the behavior.https://claude.ai/code/session_01KdvoAVF2YDBnuJ3gmgU9hQ