-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Normalize union/intersection type combinations #11717
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question about isRelatedTo and a possible improvement for reporting errors with intersections of boolean.
@@ -1,7 +1,8 @@ | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(17,5): error TS2322: Type 'A & B' is not assignable to type 'number'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(18,5): error TS2322: Type 'A & B' is not assignable to type 'boolean'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(19,5): error TS2322: Type 'A & B' is not assignable to type 'string'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(21,5): error TS2322: Type 'number & boolean' is not assignable to type 'string'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(21,5): error TS2322: Type '(number & true) | (number & false)' is not assignable to type 'string'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some way to present this error as number & boolean
still? I feel like the distributed form is unexpected here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual representation needs to be (number & true) | (number & false)
, but we could play tricks in the type-to-string logic to turn it into number & boolean
. Not sure it's worth it though as those types are all rather meaningless anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, type-to-string is what I was thinking of. But you're right, & boolean
probably only shows up in our tests.
return result; | ||
} | ||
} | ||
else if (source.flags & TypeFlags.Intersection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not check the source first here too? I think that's the normal pattern for isRelatedTo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment above the initial if
statement explains why. For intersection types, the target side is an "each" relationship and the source side is a "some" relationship. We need to deconstruct "each" relationships first to get the correct results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so I think I misunderstood the parts of the code that are independent post-distribution. It would work to put intersections before unions now, where it wouldn't before, right?
return result; | ||
} | ||
} | ||
else if (source.flags & TypeFlags.Intersection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so I think I misunderstood the parts of the code that are independent post-distribution. It would work to put intersections before unions now, where it wouldn't before, right?
@@ -1,7 +1,8 @@ | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(17,5): error TS2322: Type 'A & B' is not assignable to type 'number'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(18,5): error TS2322: Type 'A & B' is not assignable to type 'boolean'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(19,5): error TS2322: Type 'A & B' is not assignable to type 'string'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(21,5): error TS2322: Type 'number & boolean' is not assignable to type 'string'. | |||
tests/cases/compiler/errorMessagesIntersectionTypes04.ts(21,5): error TS2322: Type '(number & true) | (number & false)' is not assignable to type 'string'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, type-to-string is what I was thinking of. But you're right, & boolean
probably only shows up in our tests.
With this PR we normalize combinations of intersection and union types based on the distributive property of the
&
type operator over the|
type operator. Specifically, becauseX & (A | B)
is equivalent toX & A | X & B
, we can transform intersection types with union type constituents into equivalent union types with intersection type constituents and thus ensure that union types are always at the top level in type representations and equivalent ways of writing the same type are treated identically.In the example above,
X1
,X2
, andX3
are all identical types that reference the normalized formA & C | A & D | B & C | B & D
.Fixes #9919.