microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.16k stars 12.38k forks source link

Fails to perform type narrowing on functions that return "never" #46972

Open Zamiell opened 2 years ago

Zamiell commented 2 years ago

Bug Report

πŸ”Ž Search Terms

type narrowing never

πŸ•— Version & Regression Information

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

type ShouldNeverReturnFunction = () => never;

let shouldNeverReturn: ShouldNeverReturnFunction | null = null;

function foo(): never {
  if (shouldNeverReturn === null) {
    throw new Error('The "shouldNeverReturn" function was not initialized.');
  }

  shouldNeverReturn();
}

πŸ™ Actual behavior

The TypeScript compiler refuses to compile this code.

πŸ™‚ Expected behavior

I expect the TypeScript compiler to compile this code.

The error message says that the foo function has "a reachable end point", but that is clearly false, so the TypeScript compiler seems to be drunk. 🍺πŸ₯΄

MartinJohns commented 2 years ago

32695 mentions:

the function name resolves to a function type with an asserts return type or an explicit never return type annotation.

shouldNeverReturn does not resolve to such a function, it only is later on narrowed to one. Unfortunately the PR does not mention unions, so it's not clear whether this is intentional (my guess) or not.

RyanCavanaugh commented 2 years ago

I think it's in principle possible to expand the checking to cover unions, but this seems like a fairly rare case. The old workaround of return shouldNeverReturn(); works well here. We can look into it more if it's commonly encountered

kherock commented 2 years ago

I just ran into this while refactoring a logging function I have to throw error object passed to it. e.g. I had code that looked like this:

if (!obj.requiredProp) {
  const error = new Error('requiredProp is required');
  logger.error(error.message)
  throw error;
}

someFn(obj.requiredProp);

My new code adds a throw function that is used like this

declare const logger: { throw: (error: unknown) => never };

if (!obj.requiredProp) {
  logger.throw(new Error('requiredProp is required'));
}

someFn(obj.requiredProp); // argument of type 'string | null' is not assignable to parameter of type 'string'

Here I expect some type narrowing but I get the error that I've provided in the comment.

Zamiell commented 2 years ago

Today I ran into another issue along these lines. I am not 100% sure if it is the same thing, but it smells like it.

Consider the following code snippet:

function checkFoo(something: string): boolean {
  if (something === "foo") {
    return true;
  }

  if (something === "bar") {
    return false;
  }

  throw new Error("Unknown something.");
}

It works great! πŸ‘

function error(msg: string): never {
  throw new Error(msg);
}

function checkFoo(something: string): boolean {
  if (something === "foo") {
    return true;
  }

  if (something === "bar") {
    return false;
  }

  error("Unknown something.");
}

This code fails with the "noImplicitReturns" compiler flag turned on. ❌ It says that "Not all code paths return a value", which is wrong, because all code paths do return in exactly the same way that the first code snippet does, but TypeScript isn't smart enough to realize that.

Zamiell commented 2 years ago

Ok, Retsam helped me realize that the code can compile if all you do is make a return type annotation on the error function to be never. And the reason why the TypeScript compiler can't do this automatically is discussed here by Ryan Cavanaugh.

So I guess disregard the "noImplicitReturns" thing. (But the type narrowing thing is still an issue.)

adamsuskin commented 1 year ago

@RyanCavanaugh does this strike you as something a community member can contribute? I'm particularly interested in solving for @kherock's use case above.