Skip to content
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

Fix inference to intersections #30857

Merged
merged 3 commits into from Apr 12, 2019
Merged

Fix inference to intersections #30857

merged 3 commits into from Apr 12, 2019

Conversation

ahejlsberg
Copy link
Member

Fixes #30442.

inferFromTypes(source, t);
// we want to infer string for T, not Promise<string> | string. For intersection types
// we only infer to single naked type variables.
if (target.flags & TypeFlags.Union ? typeVariableCount !== 0 : typeVariableCount === 1) {
Copy link
Member

@weswigham weswigham Apr 11, 2019

Choose a reason for hiding this comment

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

The old comment on this behavior was

                    // Next, if target containings a single naked type variable, make a secondary inference to that type
                    // variable. This gives meaningful results for union types in co-variant positions and intersection
                    // types in contra-variant positions (such as callback parameters).

should we perhaps be checking the contravariant flag here, rather than (or perhaps alongside) count?

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 the contravariant flag is relevant here (or maybe I'm just not seeing how).

Copy link
Member

Choose a reason for hiding this comment

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

The original in-code comment on this behavior said that it was inferences to intersections in contravariant positions (see above) that gave good results - it's possible that we could capture that sentiment by inferring to only one intersection parameter in covariant positions, but all of them in contravariant positions (and vice-versa for unions, I imagine?)

I dunno really; this is just speculation based on the text of a comment we removed, and assuming that there was insight in that comment which our tests didn't quite capture - without concrete examples, though, I'm not really sure.

@ahejlsberg ahejlsberg merged commit 6282645 into master Apr 12, 2019
@ahejlsberg ahejlsberg deleted the fixInferenceToIntersection branch April 12, 2019 17:34
RyanCavanaugh added a commit to RyanCavanaugh/TypeScript that referenced this pull request Apr 16, 2019
…section

Fix inference to intersections
# Conflicts:
#	src/compiler/checker.ts
@RyanCavanaugh RyanCavanaugh mentioned this pull request Apr 16, 2019
RyanCavanaugh added a commit that referenced this pull request Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type parameter inference regression in 3.4
3 participants