Skip to content

No implicit returns error in exhaustive if statement #17358

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

Closed
OliverJAsh opened this issue Jul 22, 2017 · 9 comments
Closed

No implicit returns error in exhaustive if statement #17358

OliverJAsh opened this issue Jul 22, 2017 · 9 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Jul 22, 2017

TS 2.4.1 with noImplicitReturns enabled

    class A {}
    class B {}

    // error: Not all code paths return a value.
    const fn = (x: A | B) => {
        if (x instanceof A) {
            x // A
            return 1
        } else if (x instanceof B) {
            x // B
            return 2
        } else {
            x // never
        }
    }

I would not expect this to error because the if statement is exhaustive, therefore all code paths do return a value.

@j-oliveras
Copy link
Contributor

Is not exhaustive you can call fn function as fn({}) or fn({ x: 1 }). These calls are valid and are not A nor B.

So, I think, that works as intended.

@OliverJAsh
Copy link
Contributor Author

@j-oliveras I think what you're saying makes sense, but I don't see how that applies to this example:

    interface A { type: 'A' }
    interface B { type: 'B' }

    // error: Not all code paths return a value.
    const fn = (x: A | B) => {
        if (x.type === 'A') {
            x // A
            return 1
        } else if (x.type === 'B') {
            x // B
            return 2
        } else {
            x // never
        }
    }

If using a switch statement instead of the if, the error goes. I would like to understand why we get this error when using if statements.

@jcalz
Copy link
Contributor

jcalz commented Jul 23, 2017

@j-oliveras It is exhaustive as far as the type checker is concerned, since it infers a never type for x in the final else clause. Of course, as you note, you can fool it, due to an inconsistency between the nominal typing of instanceof at runtime and the structural typing of TypeScript (see, e.g., #11664). But that's not the issue here, as @OliverJAsh's followup demonstrates.

The simplest repro I can imagine is something like:

function foo(x: boolean) {
  if (x) return 1;
  if (!x) return 2;
  // x is inferred as never here, but ts still thinks some paths don't return a value
}

I'm guessing that the real issue is that the control flow analysis just isn't clever enough to realize that, once it has narrowed something to the never type, the surrounding code is unreachable and shouldn't count as a code path for the purposes of noImplicitReturns. Obviously there are workarounds (rework the if/else clauses to be obviously exhaustive to the compiler; throw an exception in any code you know is unreachable; etc.) but the question is whether the reported issue is a design limitation or a bug. I don't think it should be intended behavior.

@DanielRosenwasser
Copy link
Member

I believe the problem is that only switch statements are special cased here. Basically, if the last statement in a function is a switch statement, we do an extra check for exhaustiveness, but we decided that if/elses might be too complex of a check. You'd have to check with @ahejlsberg for specifics.

You can always return the result of calling assertNever (documented here, available on npm here.

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

mhegazy commented Aug 17, 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 Aug 17, 2017
@OliverJAsh
Copy link
Contributor Author

This also affects switch statements when they exist within an if statement. You can workaround it by wrapping the switch in an IIFE.

Is there anything TS can do to help here?

{
    enum Input { foo, bar }

    // Unexpected error: Not all code paths return a value.
    const fn = (input: Input) => {
        if ('foo') {
            switch (input) {
                case Input.bar: return 0;
                case Input.foo: return 1;
            }
        } else {
            return 1
        }
    }

    // Workaround: wrap switch in IIFE
    const fn2 = (input: Input) => {
        if ('foo') {
            return (() => {
                switch (input) {
                    case Input.bar: return 0;
                    case Input.foo: return 1;
                }
            })();
        } else {
            return 1
        }
    }
}

@jcalz
Copy link
Contributor

jcalz commented Oct 19, 2017

@OliverJAsh said:

Is there anything TS can do to help here?

@DanielRosenwasser said:

You can always return the result of calling assertNever (documented here, available on npm here.)

@OliverJAsh
Copy link
Contributor Author

@jcalz Thanks, that works.

TypeScript already special cases switch statements, as mentioned by @DanielRosenwasser, and so I was wondering if it could extend this behaviour to switch statements within if statements?

@eucaos
Copy link

eucaos commented Nov 27, 2017

I have the same problem with nested switch clauses :
const reducer = (state: "A", action: "act1"): "A" => { switch (state) { case "A": switch (action) { case "act1": return state; } } };
I get function lacks ending return statement error message.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

6 participants