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

Disallowing `throw`ing expressions that aren't assignable to `Error` #33596

Open Ginden opened 4 years ago

Ginden commented 4 years ago

Search Terms

throw, throwing non-error

Suggestion

Add typechecking to throw statement, allowing it to only throw Errors and anys.

Code currently allowed, but with strong potential for bugs:

throw null;
throw undefined;
throw 'foobar';
throw ''; 
throw 42;
throw {foo: 'bar'};
throw potentiallyNullable; // with –strictNullChecks

After implementing this feature request, such code wouldn't be allowed.

Use Cases

Even if it's possible to throw non-errors in JavaScript code, it's extremely confusing and hard to debug. Failing to properly handle such cases happened even to Mozilla.

Examples

Forbidden code:

throw null;
throw undefined;
throw 'foobar';
throw 42;
throw {foo: 'bar'};
throw potentiallyNullable; // with –strictNullChecks

Allowed:

throw new Error();
throw null as any;

Checklist

My suggestion meets these guidelines:

AnyhowStep commented 4 years ago

I have an open question.

Is this a lint rule thing or a TS thing?

Honestly, some checks that require type information kind of blur the line.

Like, at what point is a new strict flag desired over recommending it be a lint rule instead?

Also, note that the type check API isn't publicly exposed yet. So, even simple boolean-in-if-statement-only lint rules are not robust.


More on topic, I really like this idea and I'm almost always of the opinion that type safety "linting" should be part of the TS type system, rather than being a lint rule of an external tool.

I know I've seen projects that throw strings and then not handle it properly because downstream catchers assume they are getting an Error object.

kkarczmarczyk commented 4 years ago

Also,

return Promise.reject();
return Promise.reject(undefined);
return Promise.reject(null);

should be forbidden

zacnomore commented 4 years ago

Also,

return Promise.reject();
return Promise.reject(undefined);
return Promise.reject(null);

should be forbidden

@kkarczmarczyk This seems like an interesting and possibly helpful check but should probably be spun off into its own discussion.


As for the original proposal I think it aligns with the Typescript goals well, specifically these ones:

Statically identify constructs that are likely to be errors. Impose no runtime overhead on emitted programs. Emit clean, idiomatic, recognizable JavaScript code. Produce a language that is composable and easy to reason about