Skip to content

Remove redundant primitive types from intersections #23751

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

Merged
merged 5 commits into from
Apr 28, 2018

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Apr 27, 2018

This PR removes primitive types string, number, and symbol from intersections that also contain literal types from the same domain. For example string & 'a' is reduced to just 'a'. Likewise, 10 | number is reduced to just 10.

The PR eliminates the TypeIncludes enum from the type checker in favor of just using TypeFlags. This allows us to more efficiently compute the combined flags during the construction of a union or intersection type.

@ahejlsberg ahejlsberg requested review from mhegazy and weswigham April 27, 2018 23:57
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a straightforward refactor of TypeIncludes -> TypeFlags plus the small change to simplify intersections. One comment about a typo on the test, though, which may impact that strictNullChecks case.

@@ -0,0 +1,15 @@
// @strict
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// @strict: true
This isn't setting any compiler options as is and becomes just a comment in the output. Though I guess it's heartening to know that nothing in this change actually relies on strict. 😛

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. We have hundreds of tests that include // @strict: true and they definitely behave differently if you remove the comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

....this comment isn't // @strict: true, it's just // @strict. This comment does nothing.

for (const type of types) {
includes = addTypeToIntersection(typeSet, includes, getRegularTypeOfLiteralType(type));
}
return includes;
}

function removeRedundantPrimtiveTypes(types: Type[], includes: TypeFlags) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: should be removeRedundantPrimitiveTypes

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2018

Fixes #9410
Fixes #23651

@ahejlsberg ahejlsberg merged commit 25d5952 into master Apr 28, 2018
@ahejlsberg ahejlsberg deleted the reduceIntersectionTypes branch April 28, 2018 18:43
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants