Typescript: Unintuitive error message with 'unreachable' code

[dbaeumer]

TypeScript Version: 2.0.02

Code

const enum Keys {
    Tab = 10,
    Shift = 13
}

function bug() {
    let key: Keys;

    if (key === Keys.Tab) {
        return;
    }

    if (key === Keys.Tab || key === Keys.Shift) {
        console.log('Bug');
    }
}

On line 13 you get an error message that === can't be applied as an operator. Took me quite some time to find out that the same comparison was above (the real code was a little bit more complicated) and was leaving the function.

[mhegazy]

I think this is the same as https://github.com/Microsoft/TypeScript/issues/10989. Not sure what would be a better error message...

the issue is not that the compiler decided that this condition is unreachable, it is just that the type has been narrowed down in a way to make this comparison invalid.

[mhegazy]

I should add, we are open to suggestions on making this better.

[dbaeumer]

The word that would helped me here would have been 'superfluous'. But I understand that crafting a good error message here is hard since you treat it as a type checking problem (e.g. key in that position can never have the value Keys.Tab) whereas for the user it looks like a 'flow analysis' problem. I will think if I can come up with something that would have helped me here. The problem I see is that 2.0 users seeing this problem will not have your compiler internal backgrounds :-)

[kitsonk]

I guess when the left hand side is a never type and it is an expression with a comparison operator, something like: Left hand side of expression is of type 'never'. It is invalid to use a comparison operator to a 'never' type.

There could be all sorts of logic errors that result in a never type, not only CFA but someone attempting to use a return result from a never function, so I would suspect explaining it further would lead to a lot of misdirection.

[RyanCavanaugh]

The problem is that there's no never type in play here.

If we wanted to get really fancy, when this kind of check fails, we could recompute the relation using the original declared types instead of the narrowed types. If that relation succeeded, then the error message could say something like

Type `X` is not assignable type `Y`
  Earlier code in this block changed the type of the expressions
  in a way that makes this operation invalid. Do you have a logic error?

[mhegazy]

@RyanCavanaugh's proposal looks better than what we have today. A PR would be appreciated.

[dbaeumer]

+1 for @RyanCavanaugh proposal. Although I would leave the 'Do you have a logic error' out.