GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Normalize union/intersection type combinations by ahejlsberg · Pull Request #11717 · microsoft/TypeScript

Conversation

Anders Hejlsberg (ahejlsberg) commented Oct 18, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

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, because X & (A | B) is equivalent to X & 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.

interface A { a: string }
interface B { b: string }
interface C { c: string }
interface D { d: string }

// Identical ways of writing the same type
type X1 = (A | B) & (C | D);
type X2 = A & (C | D) | B & (C | D);
type X3 = A & C | A & D | B & C | B & D;

In the example above, X1, X2, and X3 are all identical types that reference the normalized form A & C | A & D | B & C | B & D.

Fixes #9919.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

HerringtonDarkholme, alitaheri, ethanresnick, and grushetsky reacted with thumbs up emoji

RyanCavanaugh, aluanhaddad, yortus, blakeembrey, svieira, siegebell, GoToLoop, and justindujardin reacted with hooray emoji

Igmat reacted with heart emoji

All reactions

👍 4 reactions
🎉 8 reactions
❤️ 1 reaction

Anders Hejlsberg (ahejlsberg) added 4 commits

October 18, 2016 11:53


          Normalize intersection and union types

ef5f3c9


          Accept new baselines

eb7d2cb


          Simplify logic in checkTypeRelatedTo

6a0f729


          Add tests

bf7f2e2

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label

Oct 18, 2016

Anders Hejlsberg (ahejlsberg) mentioned this pull request

Oct 18, 2016

Intersecting discriminated union breaks type narrowing #9919

Closed

Nathan Shively-Sanders (sandersn) requested changes

Oct 18, 2016

View reviewed changes

Nathan Shively-Sanders (sandersn) left a comment

Copy link

Copy Markdown

Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tests/baselines/reference/errorMessagesIntersectionTypes04.errors.txt

		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'.

Nathan Shively-Sanders (sandersn) Oct 18, 2016

Copy link

Copy Markdown

Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Oct 18, 2016

Copy link

Copy Markdown

Member Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Nathan Shively-Sanders (sandersn) Oct 19, 2016

Copy link

Copy Markdown

Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/compiler/checker.ts

		return result;
		}
		}
		else if (source.flags & TypeFlags.Intersection) {

Nathan Shively-Sanders (sandersn) Oct 18, 2016

Copy link

Copy Markdown

Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Anders Hejlsberg (ahejlsberg) Oct 18, 2016

Copy link

Copy Markdown

Member Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Nathan Shively-Sanders (sandersn) Oct 19, 2016

Copy link

Copy Markdown

Member

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Marin Marinov (gcnew) mentioned this pull request

Oct 19, 2016

Attempt to fix #9919 - Intersecting discriminated union breaks type narrowing #9939

Closed

Nathan Shively-Sanders (sandersn) approved these changes

Oct 19, 2016

View reviewed changes

Comment thread

src/compiler/checker.ts

		return result;
		}
		}
		else if (source.flags & TypeFlags.Intersection) {

Nathan Shively-Sanders (sandersn) Oct 19, 2016

Copy link

Copy Markdown

Member

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tests/baselines/reference/errorMessagesIntersectionTypes04.errors.txt

		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'.

Nathan Shively-Sanders (sandersn) Oct 19, 2016

Copy link

Copy Markdown

Member

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mohamed Hegazy (mhegazy) approved these changes

Oct 19, 2016

View reviewed changes

Anders Hejlsberg (ahejlsberg) merged commit 66857b5 into master

Oct 19, 2016

Anders Hejlsberg (ahejlsberg) deleted the normalizeIntersectionTypes branch

October 19, 2016 20:15

Troy Gerwien (yortus) mentioned this pull request

Oct 21, 2016

User-defined type guards seemingly unused when given an intersection type #10144

Closed

Anders Hejlsberg (ahejlsberg) mentioned this pull request

Nov 2, 2016

Discriminated Unions Incompatible with Intersection Types #11989

Closed

Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request

Nov 11, 2016

type guard does not narrow intersection over union #12180

Closed

Ethan Resnick (ethanresnick) mentioned this pull request

Nov 16, 2016

Control Flow Type Inference: Inferred Type Varying by Condition Order #11426

Closed

Herrington Darkholme (HerringtonDarkholme) mentioned this pull request

Nov 18, 2016

Flow Analyze don't recognize combined Intersection and Union Type #12345

Closed

Mohamed Hegazy (mhegazy) mentioned this pull request

Nov 18, 2016

2.1 Regression: (A | undefined) & B is possibly undefined #12354

Closed

Anders Hejlsberg (ahejlsberg) mentioned this pull request

Nov 28, 2016

Compiler hangs on union of enums with a large number of elements #12535

Closed

Anders Hejlsberg (ahejlsberg) mentioned this pull request

May 17, 2017

Generic constraint not playing well with new 'object' type. #15920

Closed

Anders Hejlsberg (ahejlsberg) mentioned this pull request

Sep 13, 2017

Remove empty intersection types in union types #18438

Merged

Microsoft (microsoft) locked and limited conversation to collaborators

Jun 19, 2018

Reviewers

Nathan Shively-Sanders (sandersn)

sandersn approved these changes

+1 more reviewer

Mohamed Hegazy (mhegazy)

mhegazy approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Anders Hejlsberg (ahejlsberg) commented Oct 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nathan Shively-Sanders (sandersn) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Anders Hejlsberg (ahejlsberg) commented Oct 18, 2016 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.