Control flow analysis should "work" in unreachable code

[RyanCavanaugh]

TypeScript Version: master

Search Terms: unreachable throw return narrowing

Code

function fn() {
    // TODO: Finish writing this function!
    return;

    const h = Math.random() > 0.5 ? 'h' : undefined;

    if (h) {
        // Error, wat
        const a: string = h;
    }
}

Expected behavior: No error

Actual behavior: Error - h is possibly undefined

We get "bug" reports like this every once in a while, and it's hard to understand why the narrowing just stops working because the code isn't reachable. This also makes it difficult to insert debugging throws/returns earlier in the function.

e.g. https://twitter.com/aendrew/status/1037301561062514688

Playground Link: http://www.typescriptlang.org/play/#src=function%20fn()%20%7B%0A%20%20%20%20%2F%2F%20Not%20done%20yet%0A%20%20%20%20return%3B%0A%0A%20%20%20%20const%20h%20%3D%20Math.random()%20%3E%200.5%20%3F%20'h'%20%3A%20undefined%3B%0A%20%20%20%20const%20d%20%3D%20(arg%3A%20string)%20%3D%3E%20arg%3B%0A%0A%20%20%20%20if%20(h)%20%7B%0A%20%20%20%20%20%20%20%20const%20a%20%3D%20d(h)%3B%0A%20%20%20%20%7D%0A%7D%0A

Related Issues: Believe there's one out there somewhere

[RyanCavanaugh]

See also #11821

[atg]

Just encountered this also, trying to disable a loop with a break; at the top.

Couldn't type errors be suppressed in unreachable code? If the code is provably unreachable then all type errors are false positives...

[matt-tingen]

@atg while that is strictly true, having the checks is helpful while debugging. If there is unreachable code, it seems likely that it will either be removed momentarily or that the function is being actively worked on or debugged. In the former case, the behavior of TS is largely inconsequential. However, in the latter case, having TS provide checking as usual can be quite helpful.

[googol]

I ran into a problem with conditional types, where when checking if a type extends never, the true branch evaluated to never, even though the type that was checked isn't referenced there: minimal repro in playground

Is this a type level variant of this issue?

[RyanCavanaugh]

@googol unrelated and the code there is working as intended. You are passing never through a distributive conditional type, so the output of that will always be never.

[googol]

Thanks for the reply! This issue just came straight to my head when I encountered this since we had just had a chat about this at the company.

I wasn't expecting the distributivity kick in here since there is no union type here, but it definitely is the cause, since the "wrap both sides of extend in tuples" workaround fixes it. Interesting!

[CapsAdmin]

I've also encountered this a few times and it's sometimes confusing. I use return instead of a comment as described in the issue but I've learned not to do this in typescript.

It's mostly confusing when you accidentally write code that returns. I just did this mistake now and was going to file an issue but found this issue instead. And for some reason my code is not marked as dead probably due to some bad setup in my vscode.

[cns-solutions-admin]

During debugging I often change conditions to if (false && .... However, in most cases this results in compilation errors, as suddenly the types are no longer narrowed.
Luckily for this use case you can also write if (!true && .... This works (for now) ;-)

[RyanCavanaugh]

Since someone will inevitably ask "Why isn't this fixed after n years???", let me just pre-explain that it's really not clear what "fixed" even means here.

Let's say you write this code:

function foo(x: number | string) {
  if (typeof x === "number") return;
  console.log(x.toLowerCase());
}

This is obviously fine - the only way to reach the toLowerCase call is to go "through" the control flow graph where the number case for x is ruled out.

Let's say you make the code unreachable:

function foo(x: number | string) {
  if (true) {
    return;
  }

  if (typeof x === "number") return;
  console.log(x.toLowerCase());
}

This is "obviously fine" too, right? The only way to get to the toLowerCase call is, again, through the typeof check. But there's no way to get there in the first place, so it's not clear what your starting point is. It seems obvious to just say "oh, start at the top of the function body then".

But now let's say you do this.

function foo(x: number | string) {
  if (typeof x === "number") return;

  if (true) {
    return;
  }

  console.log(x.toLowerCase());
}

If we take the approach of treating the unreachable code as starting from the top of the function body, then the code doesn't work again, because the typeof check occurs after the top of the function and thus doesn't apply here.

Now you say "Just treat always-exiting graph points as not exiting - like the return isn't there"

This would break working code that wants to verify it's exhaustively handling certain patterns. The default case here appears unreachable - x is never - but if we treat the "always exiting" subgraph of the two cases as not exiting, then x here should have type string | number

function foo(x: number | string) {
  switch (typeof x) {
    case "number": break;
    case "string": break;
    default:
      x satisfies never; // OK, today
  }
}

"Just treat anything in unreachable blocks as any to avoid type errors" - also bad! If we do this, a rename operation on the foo at line 1 will not rename the x.foo reference at j

type HasFoo = { foo: number };

function fn(HasFoo) {
  return;
  let j = x.foo;
}

So anyway this is tagged Bug but no one knows what a correct fix even looks like, so try not to take any hard dependencies on how TS treats unreachable code or pin your hopes too high. PRs welcomed but you'll have to solve a problem we've been stumped on for years first.