Skip to content

Unintuitive error message with 'unreachable' code #10991

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

Open
dbaeumer opened this issue Sep 19, 2016 · 7 comments
Open

Unintuitive error message with 'unreachable' code #10991

dbaeumer opened this issue Sep 19, 2016 · 7 comments
Labels
Domain: Error Messages The issue relates to error messaging Effort: Difficult Good luck. Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@dbaeumer
Copy link
Member

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
Copy link
Contributor

mhegazy commented Sep 19, 2016

I think this is the same as #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
Copy link
Contributor

mhegazy commented Sep 19, 2016

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

@dbaeumer
Copy link
Member Author

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
Copy link
Contributor

kitsonk commented Sep 20, 2016

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
Copy link
Member

RyanCavanaugh commented Sep 20, 2016

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 mhegazy added Suggestion An idea for TypeScript Domain: Error Messages The issue relates to error messaging Help Wanted You can do this labels Sep 20, 2016
@mhegazy mhegazy added this to the Community milestone Sep 20, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Sep 20, 2016

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

@dbaeumer
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Error Messages The issue relates to error messaging Effort: Difficult Good luck. Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants