microsoft / TypeScript

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

Incorrect return type considered valid in functions that never return #49549

Closed flowstate247 closed 2 years ago

flowstate247 commented 2 years ago

Bug Report

💻 Code

function f(): void {
  throw new Error()
}

🙁 Actual behavior

The TypeScript compiler considers this code as totally valid. The compiler doesn't complain at all.

🙂 Expected behavior

I would have expected the TypeScript compiler to detect an error because the function f never returns. So the void return type is incorrect. The corrected version of function f is:

function f(): never {
  throw new Error()
}

The correct typing should be enforced by the TypeScript compiler I think.

fatcerberus commented 2 years ago

In principle this is correct because the effective return type of a function that always throws is never, and being the bottom type never is assignable to all other types. Because the function never returns in practice, you (and the compiler) are free to treat it as if it returned void, or indeed, whatever its declared return type actually is (ex falso quodlibet, or the principle of explosion).

Speaking less theoretically and more practically, the lack of a compile-time error here is useful:

function getFoo(): number {
    // need this to compile to test other stuff, but if it does get called by accident we want to know about it
    throw Error("getFoo is not yet implemented");
}
flowstate247 commented 2 years ago

The praise of a lack of compile-time errors is definitely not something I expected here. I mean, what is the point of the TypeScript project if not for raising errors at compile-time to improve code quality.

fatcerberus commented 2 years ago

The praise of a lack of compile-time errors

What? It's not an error, it's completely type-theoretically sound for the compiler to not error here, that was my entire point. It doesn't improve code quality for the compiler to produce error diagnostics in cases that aren't actually errors. never is assignable to number, so an implementation that "returns" never is not an error either.

RyanCavanaugh commented 2 years ago

This isn't wrong from a theoretical perspective, and it's not wrong from a practical perspective either. Flagging this as an error would prevent you from, for example, unconditionally throwing from a base class method that must be overridden in a derived class.

flowstate247 commented 2 years ago

I experimented with the Hegel playground here: https://hegel.js.org/try#GYVwdgxgLglg9mABMOcAUAPAXIsIC2ARgKYBOAlDnkWYgD67EButA3gLABQiiMwimRAD4AvIgAsAJnKJSxKCFJIMXHlAAWpOAHdGugKKktpNOS4BfLl1CRYCRIQCGAL0xUCJCu5qlEHbrz8ggA8iAAMMnIKSogqAVGKSCjoGGac5kA

We can see that Hegel does a proper job of checking the types and raising an error: image

While TypeScript is hopelessly lax about it, as demonstrated in the TypeScript playground here: https://www.typescriptlang.org/play?#code/GYVwdgxgLglg9mABMOcAUAPAXIsIC2ARgKYBOAlDnkWYgD67EButA3gFCKIzCKaIA+ALyIALACZyiUsSghSSDJ0RQAFqTgB3RtoCipDaTTl2AX3btQkWAkSEAhgC9MVAiQquapRBy48+GIgAPIgADFIycgqISlyR8kgo6BgmpkA

So I presume that the real underlying issue here is the lax philosophy of how TypeScript views its type system.

fatcerberus commented 2 years ago

Most type theory says the bottom type is a subtype of all other types, and indeed this is the case in, e.g. Haskell. The above example being an error indicates that Hegel is treating never as something other than a bottom type. It looks suspiciously like a checked-exceptions mechanism posing as a bottom type.

Does Hegel also error in that example if you just call foo() without the return? Because there’s also the practical concern that almost everything in JS can throw at any time, even something seemingly innocuous like x = foo.y. Pretty much all your functions would need to be annotated as | never in practice if TS enforced that.

flowstate247 commented 2 years ago

The point is that if we want TypeScript to be a safe language it must have a way to declare whether a function can throw or not.

As I see it it could be done either by adding a throws keyword like Java does, or by using the TypeScript never type for this purpose as shown in Hegel in the example above.

I would argue that using never in the return type of the function is more lightweight and feels more natural to TypeScript devs than the throws Java-esque mechanism.

fatcerberus commented 2 years ago

What you want is some form of checked exceptions. So then we’re on the same page. But the point still stands that almost any operation in JS (and therefore TS) can throw. Enforcing this would mean you’d be declaring almost every single function as “this might throw” (since, as mentioned before, due to the existence of getters and proxies, even a property access on an object that was passed in can throw), and you’d be right back to where you started.

typescript-bot commented 2 years ago

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.