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 instanceof narrowing #10194

Merged
merged 6 commits into from
Aug 8, 2016
Merged

Fix instanceof narrowing #10194

merged 6 commits into from
Aug 8, 2016

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Aug 7, 2016

This PR fixes three issues related to narrowing of types with the instanceof operator and user defined type guards:

  • In an operation x instanceof T where x is of a non-union type, we now narrow the type of x to never in the false branch if the type of x is not a subtype of T. Previously we didn't change the type of x in the false branch.
  • In an operation x instanceof T where the types of x and T are both assignable to each other, we now pick T only if it is a subtype of the type of x. Previously we would always pick T.
  • Following a branching construct with a type guard that introduces a "foreign" type (i.e. a type that isn't part of a variable's declared type) we now perform subtype reduction to remove the foreign type. Previously the foreign type would accumulate in the control flow type of the variable.

These fixes also affect similar scenarios with user defined type predicates (since the narrowing logic is shared).

Fixes #10145, #10167.

@DanielRosenwasser
Copy link
Member

In an operation x instanceof T where x is of a non-union type, we now narrow the type of x to never in the false branch. Previously we didn't change the type of x in the false branch.

Does this mean that the comments in this example are accurate?

class Animal { /* ... */ }
class Dog extends Animal { /* ... */ }
class Cat extends Animal { /* ... */ }

declare var x: Animal;
if (x instanceof Dog) {
    // 'x' has type 'Dog'
}
else {
    // 'x' has type 'never'
}

@ahejlsberg
Copy link
Member Author

@DanielRosenwasser Actually, no. I missed the part in italics in the wording:

  • In an operation x instanceof T where x is of a non-union type, we now narrow the type of x to never in the false branch if the type of x is not a subtype of T. Previously we didn't change the type of x in the false branch.

@DanielRosenwasser
Copy link
Member

Right, it didn't look like that's what was happening in the implementation, so I just wanted to clarify.

// Otherwise, if the current type is assignable to the candidate, keep the current type.
// Otherwise, the types are completely unrelated, so narrow to the empty type.
// If the candidate type is a subtype of the target type, narrow to the candidate type.
// Otherwise, narrow to whichever of the target type or the candidate type that is assignable
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a little more precision than the wording lends to here. If the candidate isn't a subtype, but is assignable, we keep the original type in place. (e.g. with Array.isArray, you preserve the original type instead of giving back Array<any>).

@RyanCavanaugh
Copy link
Member

👍

@ahejlsberg ahejlsberg merged commit 8ea90ab into master Aug 8, 2016
@ahejlsberg ahejlsberg deleted the fixInstanceofNarrowing branch August 8, 2016 18:51
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

Different narrowing behaviour of user-defined type guards vs discriminated unions
4 participants