镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Type guard failures with expressions that can use for type guards #9862

Description

TypeScript Version: 2.0.0

Code

class A<T> {
  private _: T;
}
class B<T> {
  private _: T;
}
function f() {
  var o: A<void> | B<void> = new A<void>();
  o instanceof A || o instanceof B;
  o; // A<any>, should be A<void> | B<void>
}

Expected behavior:

A type of o after expr is A<void> | B<void>.

Actual behavior:

A type of o after expr is A<any>.

Activity

  1. yortus commented on Jul 21, 2016

    @yortus
    Contributor

    Looks like same thing as #9861. It's due to the new A<void> assignment. The type of o from that point onward is narrowed to A<void>. Add an extra line before the expression to see this:

    class A<T> {
      private _: T;
    }
    class B<T> {
      private _: T;
    }
    function f() {
      var o: A<void> | B<void> = new A<void>();
      o; // A<void>
      o instanceof A || o instanceof B;
      o; // A<any>, should be A<void> | B<void>
    }

    I'm not sure why it changes from A<void> to A<any> though....

  2. mhegazy commented on Jul 22, 2016

    @mhegazy
    Contributor

    Same as #9861. please keep the discussion in #9859

  3. yortus commented on Jul 22, 2016

    @yortus
    Contributor

    Mohamed Hegazy (@mhegazy) there is something odd about the inferred type of o changing from A<void> to A<any> after the expression, which is not captured in #9861. That part is a different issue - could it be a bug? Or otherwise is there an explanation for the change to the inferred type after the expression?

  4. mhegazy commented on Jul 22, 2016

    @mhegazy
    Contributor

    you are right. it should be A<void>. this looks like a bug.

  5. added
    BugA bug in TypeScript
    and removed
    DuplicateAn existing issue was already created
    on Jul 22, 2016
  6. yortus commented on Jul 22, 2016

    @yortus
    Contributor

    Another repro:

    function f() {
        var s: Set<string> | Set<number>;
    
        s = new Set<number>();
        s; // Set<number>
        s instanceof Set;
        s; // Set<number>
    
        s = new Set<number>();
        s; // Set<number>
        s instanceof Set || s instanceof Set;
        s; // Set<any>                                <===== inferred type changed
    
        s = new Set<number>();
        s; // Set<number>
        s instanceof Set && s instanceof Set;
        s; // Set<any>                                <===== inferred type changed
    
        s = new Set<number>();
        s; // Set<number>
        s instanceof Set, s instanceof Set;
        s; // Set<number>
    
        s = new Set<number>();
        s; // Set<number>
        typeof s === 'object' || typeof s === 'object';
        s; // Set<number>
    }
  7. added this to the milestone on Aug 1, 2016
  8. sandersn commented on Aug 3, 2016

    @sandersn
    Member

    In master, the inferred type is now Set<number> | Set<any>. Which is still wrong.

  9. sandersn commented on Aug 3, 2016

    @sandersn
    Member

    Surprisingly, it is supposed to work this way! And after a lot of discussion with Anders Hejlsberg (@ahejlsberg) I think I can explain why.

    You need to understand three facts:

    1. if affects control flow by splitting it in half and then unioning the result type of the two halves. See example.
    2. Conditional expressions are treated just like ifs, even if they're just by themselves in a statement, because side effects can happen in any expression.
    3. instanceof can only narrow the generic type A<T> to A<any> because, at runtime, there's no such thing as a type parameter and instanceof operates on the prototype, which will let any A through, regardless of the type of its members.

    Example: If control flow

    function f(x: string | number) {
        if (typeof x === 'string') {
            throw new Error("I don't handle strings after all!");
        }
        else {
            console.log('numbers are ok');
        }
        return x; // x: number
    }

    The then-branch of the if narrows x: string but then throws, so it results in x: never. The else-branch narrows x: number and then does nothing to alter the type. That means that the type of x after the if is x: number | never = number.

    Complete example

    Now let's look at a miniature version of the examples above:

    function f(s: Set<string> | Set<number>) {
      s = new Set<number>();
      s instanceof Set || s instanceof Set;
      s; // Set<number> | Set<any>
    }

    The type of the lone s; gets Set<number> from the assignment s = new Set<number>();. The line s instanceof Set || s instanceof Set is treated like the then-branch of an if, and adds Set<any> to the type. Then else-branch is not present so it doesn't add any type to the union.

  10. added
    Working as IntendedThe behavior described is the intended behavior; this is not a bug
    and removed
    BugA bug in TypeScript
    on Aug 3, 2016
  11. falsandtru commented on Aug 4, 2016

    @falsandtru
    ContributorAuthor

    I understood, thanks.

  12. yortus commented on Aug 4, 2016

    @yortus
    Contributor

    Thanks Nathan Shively-Sanders (@sandersn) for the explanation, which makes it much clearer what's going on.

    But isn't the compiler incorrectly performing narrowing in a clearly side-effect-free context? All of the following statements, on their own, cause s to be narrowed in subsequent statements, despite clearly having no effect on s:

    • s instanceof Set || 42
    • if (s instanceof Set) {/*empty*/}
    • if (s instanceof AnyRandomClass) {/*empty*/}

    You've pointed out the reason for this in the current implementation, but perhaps there's scope for future improvement here? The core issue is the compiler's arguably strange 'narrowing' inference below:

    let u = new Set<number>();
    if (u instanceof Set) {
        u // u is Set<any>  !?
          // but we already knew u was a Set<number>,
          // so this type guard has widened it, not narrowed it!
    }

    That leads to the following version that I find hard to describe as 'working as intended':

    function f(s: Set<string> | Set<number>) {
    
        // (1)
        s = new Set<number>(); // s is Set<number> after this assignment
        if (s instanceof Set) { } // Clearly no side-effects here. No possible changes to s.
        s.add(42); // ERROR: Cannot invoke an expression whose type lacks a call signature.
                   // inferred type of x here is Set<number> | Set<any>
    
        // (2)
        s = new Set<number>(); // s is Set<number> after this assignment
        if (s instanceof Promise) { } // Clearly no side-effects here. No possible changes to s.
        s.add(42); // Works! No error this time!?
                   // inferred type of x here is Set<number> | (Set<number> & Promise<any>)
    }
  13. yortus commented on Aug 4, 2016

    @yortus
    Contributor

    Another example of instanceof narrowing strangeness:

    // This part looks good:
    let s: Set<string> | Set<number>;
    if (s instanceof Set) {
        s // s is Set<string> | Set<number>, as expected
    }
    else {
        s.add(42); // ERROR 'add' does not exist on never, as expected
    }
    
    // This is not so good:
    s = new Set<number>();
    if (s instanceof Set) {
        s // s is Set<any>
    }
    else {
        s.add(42); // s is Set<number>
        // ^^^ no error this time, even though we couldn't possibly have a Set instance here
    }
  14. sandersn commented on Aug 4, 2016

    @sandersn
    Member

    In your (2), Anders Hejlsberg (@ahejlsberg) has a PR (merged, I think?) that removes the weird behaviour when narrowing would produce never but instead unions on the guarded type plus the original type. It now produces never, which means that (2) would produce the correct type. I think it would fix the else-branch of your example (4) as well.

    Maybe Anders Hejlsberg (@ahejlsberg) can comment on the efficiency (and design) concerns of analyzing blocks so that empty blocks would not have the bad behaviour in (1) and the others. I don't understand the overall design well enough to say.

  15. sandersn commented on Aug 4, 2016

    @sandersn
    Member

    I also also don't get why an un-initialized variable in (3) doesn't widen to Set but the assignment does in (4). I think it might be due to quirks in the handling of unions versus single (normal) types, but I'd have to read through the code to make sure.

  16. yortus commented on Aug 5, 2016

    @yortus
    Contributor

    Nathan Shively-Sanders (@sandersn) I've opened a new issue #10167 for discussion of the (1)-(4) cases above, since this is closed and the instanceof issues are not directly related to the OP problem here anyway.

  17. locked and limited conversation to collaborators on Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Working as IntendedThe behavior described is the intended behavior; this is not a bug

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions