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

never incorrectly inferred with type guard #10934

Closed
paldepind opened this issue Sep 15, 2016 · 5 comments · Fixed by #19671
Closed

never incorrectly inferred with type guard #10934

paldepind opened this issue Sep 15, 2016 · 5 comments · Fixed by #19671
Assignees
Labels
Fixed A PR has been merged for this issue

Comments

@paldepind
Copy link

I was refactoring some code and suddenly some type guards in my code begin causing type errors. I've reduced my problem to the small example below. I just did npm install -g typescript@next to check that this also happens with the latest version.

After some confusion and debugging it appears that the bug happens because the class Empty has no methods. If I add a random dummy method to Empty the code compiles again.

TypeScript Version: 2.1.0-dev.20160915

Code

export type FingerTree<A> = Empty | Single<A>;

export class Empty {
  constructor() {};
}

export class Single<A> {
  constructor(
    public nested: boolean
  ) {};
}

export function test(t: FingerTree<any>): boolean {
  if (t instanceof Empty) {
    return true;
  } else {
    return t.nested;
  }
}

Expected behavior:
Code compiles without errors and t has the type Singe<any> in the else clause in test.

Actual behavior:
t has the type never in the else clause and I get` the compile error:

test.ts(17,14): error TS2339: Property 'nested' does not exist on type 'never'.
@kitsonk
Copy link
Contributor

kitsonk commented Sep 15, 2016

This has to deal with assignability of the structural typing of TypeScript (see #202). Because Single is assignable to Empty, the code flow analysis is eliminating both of them from the union type. The challenge is that instanceof is nominal, but the rest of JavaScript largely behaves structural and therefore TypeScript behaves structural.

For a workaround, which is buried in #202, you can see #10882 which explains how to tag your classes so they are not substitutable.

@paldepind
Copy link
Author

Thanks for the explanation @kitsonk. It makes sense.

However, it seems to me like this issue could still be fixed by improving type guards. The type guards should realize that I am using instanceof and then shouldn't eliminate anything but the exact class from the union type.

@yortus
Copy link
Contributor

yortus commented Sep 15, 2016

@paldepind the team has been discussing it - see for example the design meeting notes in #8503. Also there's some more background in #8168.

@DouglasLivingstone
Copy link

DouglasLivingstone commented Oct 30, 2016

Here's a similar example without instanceof, where the types are differentiated by the size of the count value instead. In this case, value ends up with the type never in value.id, but count could have been high.

interface Low {
    count: number;
}

interface High {
    id: string;
    count: number;
}

function isHigh(value: High | Low): value is High {
    return value.count > 10;
}

function isLow(value: High | Low): value is Low {
    return value.count <= 10;
}

function test(value: High | Low) {
    if (isLow(value)) return "<no id>";
    if (isHigh(value)) return value.id;
    return null;
}

I've been able to work around this by swapping the order of the comparisons, since mine are mutually exclusive:

function test(value: High | Low) {
    if (isHigh(value)) return value.id;
    if (isLow(value)) return "<no id>";
    return null;
}

Another work around is to define an explicit base interface, presumably this works since then value could explicitly be neither Low nor High. I can't use this workaround since my equivalent of High is defined in a separate library.

interface Countable {
    count: number;
}

interface Low extends Countable {
}

interface High extends Countable {
    id: string;
}

function isHigh(value: Countable): value is High {
    return value.count > 10;
}

function isLow(value: Countable): value is Low {
    return value.count <= 10;
}

function test(value: Countable) {
    if (isLow(value)) return "<no id>";
    if (isHigh(value)) return value.id;
    return null;
}

@mhegazy mhegazy added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Apr 27, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 22, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@mhegazy mhegazy closed this as completed May 22, 2017
@mhegazy mhegazy added Fixed A PR has been merged for this issue and removed Design Limitation Constraints of the existing architecture prevent this from being fixed labels Nov 2, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Nov 2, 2017
@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
Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants