Rule suggestion: use of never

[NWilson]

Repro

The following code is clearly bad, but compiles just fine, because the typeof condition narrows the type of a to never, allowing it to be passed to a function that clearly can't accept it. (This is distilled from a real example in a Microsoft codebase.)

function useTheValue(a: { foo: number }) {
  return a.foo;
}

export function testFunc(a: string | number) {
  if (typeof a === 'boolean') {
    return useTheValue(a);
  }
  return 0;
}

I apologize if it exists - but I can't find a typescript-eslint rule that will detect this.

I think the rule I want is "disallow any use of a never-typed value except as an immediate operand to return".

[bradzacher]

this could definitely be added to our no-unsafe-* class of rules.
Assigning never to something is very unsafe!

[CreativeTechGuy]

I found this when looking for something similar myself and want to try my hand at implementing it. I just want to confirm what I'm thinking of implementing and get suggestions ahead of time.

I think the rule could be very simple and error on the use of never in any case. This would disable the use of never as a return value but I believe there'd be overlap in the people who want this rule and also want to prohibit a function that never returns. (I honestly haven't seen a real example of needing to explicitly type never.)

Maybe if we want to get fancy the rule could have an option to allow never return values if that's something a lot of people might want. But that could also be a follow up if the need is there.

For a name, this seems like it'd be similar to no-namespace so no-never would probably make sense although I admit the name does read a bit weird.

[bradzacher]

Instead of creating a new rule entirely, IMO it'd be much better to add handling for never to our existing no-unsafe-* rules.

The rules are already setup to prevent unsafe operations on an any, so it's a natural extension to also look for never.

[CreativeTechGuy]

I played around with prototyping it last night and I think the functionality I have is a bit different. It identifies any variable which is of type never. Either through being scoped down to never or being explicitly typed as never.

In this example below:

const obj = "abc";

if (typeof obj === "undefined") {
    const test = obj; // Both test and obj on this line are errors as they have the never type
    console.log(test); // test is an error here as it has the never type
}

function neverReturn (): never { // This is fine as it's a return type and not a variable type
    throw new Error("abc");
}

let num: never; // num is an error as it explicitly has the never type

This seems to solve the same problem as identifying operations on never while also handling other related cases.

Let me know if that makes sense and if not, which rule you were thinking of extending to add this functionality? (And what functionality specifically.)

[bradzacher]

Your first case is a no-unsafe-assignment
Your second case is a no-unsafe-argument (rule hasn't been implemented yet, but it's on the todo list)

Your third case isn't handled - as declaring a return of type never isn't a problem in of itself.
Same goes for your fourth.

no-unsafe-return handles the case where you return a never type (you have not listed this case).

[CreativeTechGuy]

I see what you mean. When I was reading the description and messages for a rule like no-unsafe-assignment, everything was very specifically referring to any. That's why I assumed the most backwards compatible option would be a new rule which would handle no-never across all of the cases.

Would you suggest rewording those existing rules to be more generic and adding this functionality to each of those? It probably wouldn't need to be an option to enable/disable since never is always unsafe.

[bradzacher]

Definitely - the docs for the rules currently only mention any because that's all they currently check for.
If you were to expand the rules to also check for never, then the docs would need to be updated appropriately.

It might be good to add an option to let people opt into it, but I don't think it's a requirement. I think just catching more cases is something everything will want.

[CreativeTechGuy]

Okay sounds like a plan. I'll put out a PR hopefully today with the updated no-unsafe-assignment and no-unsafe-return to get feedback. I'll make sure those are good before I tackle no-unsafe-argument.

[phaux]

Assigning to never has valid use cases though.

function unimplemented(variant: never) {
  throw Error(`Unimplemented variant: ${variant}`)
}

type Color = "red" | "green" // | "blue"

function handleColor(color: Color) {
  if (color === "red") {
    console.log("the color is red")
  }
  else if (color === "green") {
    console.log("the color is green")
  }
  else {
    unimplemented(color) // will show error if you add "blue"
  }
}

REPL

never shouldn't be considered unsafe on it's own. It's only unsafe when the types are wrong or you do stuff like x as never.

If it's used properly then never means "the program will never get here" so it's safe to do whatever with it.

[g-plane]

I think it's better to open an issue to TypeScript, since this invalid case should be caught by compiler (it's about type checking), not a linter.

Edit:

The reason why we should report to TypeScript is when there's a variable whose type is number | string, if we apply typeof on it, TypeScript should check if another side is valid or not (for this case, disallow boolean) and it shouldn't narrow to never.

Also, assigning never to anything is really safe. Don't ban this behavior.

[bradzacher]

Also, assigning never to anything is really safe. Don't ban this behavior.

It's literally the opposite of safe.

const x = 1 as never;
const y: string = x;

y.bold();

repl

The reason why we should report to TypeScript is when there's a variable whose type is number | string, if we apply typeof on it, TypeScript should check if another side is valid or not (for this case, disallow boolean) and it shouldn't narrow to never.

TS does ban obviously never satisfiable conditions.
However the typeof operator is currently "contextless".

const x = typeof 'a';
type T = typeof x; // "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"`

Which means that typeof a === 'boolean' is not an invalid logical, as far as the types are concerned.
However, TS's control-flow logic understands how the check narrows the type of a, and it applies that narrowing within the if - narrowing it to never.

---

Feel free to raise an issue with TS regarding typeof not being contextual, but the issue still stands that assigning never to anything IS NOT SAFE, and is no better than assigning any to anything.

[g-plane]

Just as @phaux said, it's unsafe that you cast x to never by using x as never. never means "the program never get here" (your program would panic and exit here), and it's Bottom type at type system.

Assigning never is very reasonable. Here are examples:

Playground

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

function doStuff(): number {
    return unimplemented()
}

Bottom type in Rust:

Playground

fn main() {
    do_stuff();
}

fn do_stuff() -> i32 {
    unimplemented!()
}

Bottom type in Haskell:

(undefined function is similar to unimplemented in other languages.)

main = putStrLn doStuff

doStuff :: String
doStuff = undefined

[bradzacher]

Just as phaux said, it's unsafe that you cast x to never by using x as never.

But that's the point.

What's the difference between x and y if both variables have type never?
It's entirely contextual the fact that x was assigned with an assertion to never, and y was assigned by narrowing.
Ultimately the result is the same - the variable is of type never.

Assigning never to anything that is not never is as unsafe as assigning an any because you can easily create invalid scenarios that can cause runtime errors.

By your logic - it's completely safe to use any - as long as you're careful with it, and using any is very reasonable.
Whilst that statement is true - any sufficiently careful engineer can make good use of any to escape complicated types and write completely safe code with it. However it's much, much easier to do unsafe behaviours with it accidentally.

never is exactly the same (to a lesser extent though).
Whilst, yes, a sufficiently careful engineer can make good use of never to create safe code as long as they follow some rules when using it - it's easier to accidentally create unsafe code with it.

In general assigning never to anything that's not never represents a bug in your program's logic.

Taking your example - the correct code is to not return at all:

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

function doStuff(): number {
    unimplemented()
}

Which represents safe code, because you're not assigning the never to anything.

Taking another example of switching on a union type:

function test(x: 'a' | 'b'): number {
  switch (x) {
    case 'a': return 1;
    case 'b': return 2;
    default:
      return x;
  }
}

This code is inherently unsafe, but it typechecks fine, because never is assignable to number. But this OFC can cause a runtime bug if a new value leaks in at runtime (which can happen easily - for example, say x comes from an API, and a new value 'c' is added to the API and deployed before the front-end code is updated).

This should not happen and is a bug.
OTOH - this slightly different code is entirely safe:

function throwNever(value: never): never {
    throw new Error(`Unexpected value ${value}`);
}

function test(x: 'a' | 'b'): number {
  switch (x) {
    case 'a': return 1;
    case 'b': return 2;
    default:
      throwNever(x);
  }
}

Why? Because you're assigning a never to a never - meaning the target code is expecting the code and won't do anything unsafe with it.

[mmis1000]

The only valid ways in typescript to get never without a as never are

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

function a (): never {
  while (true) {}
}

And either of them will never return anything (thus 'never')

So any code after them didn't really matter, your program can't go there because it is already dead.

A function that returns never is technically works like a throw new Error statement, anything execute after them is outright unreachable.

[bradzacher]

The only valid ways in typescript to get never without a as never are

That's incorrect - you've forgotten about the type narrowing case that I mentioned in my comment.

function test(x: 'a' | 'b'): number {
  switch (x) {
    case 'a': return 1;
    case 'b': return 2;
    default:
      x // typeof === never
  }
}

Which is the case that's discussed in the OP.

[mmis1000]

Then you need a variable that typed as 'never' but actually have value to go there. (the value you passed as 'x')

However you can't.
A function returns never always throws as above explained.
So you are still safe.

The actual unsafe code is as never because that breaks the assumption of never completely.

[bradzacher]

However you can't.

Again, as I mentioned in my comment.

If you have front-end code which talks to a backend API, and you do not push the two in sync (which is a very common deployment strategy), then it's very easy to introduce new values on the backend without having the front-end code handle it.
Or a user may not refresh their code after a backend push occurs, meaning they're on an old version of the code which does not handle the API value.

Just because TS says it's never doesn't mean it's guaranteed to be never at runtime; it simply means "based on the types you've provided, it shouldn't be reachable".

Types are not infallible, which is why it is not safe to assign never to a location that does not expect never.

[DanielRosenwasser]

I think the assertNever use-case is pretty big. People rely on exhaustiveness checks to help build robust code, even if in some cases you can't guarantee that external data sources match in their types. I would definitely hope that this isn't turned on by default for people.

[bradzacher]

to clarify:

I am suggesting that this code should be flagged:

function doesSomething(arg: string) {
  return arg.toUpperCase();
}

function test(x: 'a') {
  switch (x) {
    case 'a': return;
    default: doesSomething(x);
  }
}

Because assigning never to a non-never (in this case string) is incorrect and very much unsafe.

I am not suggesting that this code should be flagged:

function assertNever(arg: never) {
  throw new Error(`never: ${arg}`);
}

function test(x: 'a') {
  switch (x) {
    case 'a': return;
    default: assertNever(x);
  }
}

That's assigning a never to a never, which is correct and safe.

[DanielRosenwasser]

Gotcha - I would agree with you that it's generally undesirable to hit that situation. You're basically writing dead code at that point.

I do still feel a bit split because even within our own codebase we've written stuff like

doStuff({
  someNonOptionalProperty: undefined!, // TODO
});

which is a bit less heavy-handed than undefined as any. I don't know if that is the only special-case where I wouldn't want this to activate.

[bradzacher]

I do still feel a bit split because even within our own codebase we've written stuff like

Yeah I totally get that. In the "real world" sometimes you do need to do weird, potentially unsafe stuff to get across the line (either due to complex types, bugs in TS, stuff that needs to be done in time crunch, etc).

These rules (specifically our no-unsafe-* rules) are built for an "ideal" world; they're built to push towards that "perfect code" world.
But they can easily be silenced with an eslint-disable if you need to do something unsafe as an escape hatch.

Lint rules are better to err on the noisy side and catch all the cases (including intentional ones) rather than being quiet and accidentally letting bugs slip through to production.

[mmis1000]

About that unreachable condition, I though rust and kotlin done it more correctly here.

A Nothing in kotlin terminate the control flow by simply present as an type of expression.

While typescript think it is still reachable after that point (I remembered that it is cause by a design decision or so that separate the control flow analysis and typing?)

Probably this issue:
microsoft/TypeScript#10470

I probably understand what are you talk about, but I think it is at most a warning instead (it is dead code) of a error as long as you validated server response before use it.

Typescript assume input type is correct and never generate runtime guard for you due to their design philosophy.
And you need to keep this correct, otherwise it is pointless to use typescript.

[bbarry]

I am not suggesting that this code should be flagged:

function assertNever(arg: never) {
  throw new Error(`never: ${arg}`);
}

function test(x: 'a') {
  switch (x) {
    case 'a': return;
    default: assertNever(x);
  }
}

I think that code should be flagged:

function assertNever(arg: never) {
  throw new Error(`never: ${arg}`); // <- unsafe use of never right here
}

function test(x: 'a') {
  switch (x) {
    case 'a': return;
    default: assertNever(x);
  }
}

based on the way that code is, if that function executes, the value is clearly not never and the function should instead be

function assertNever(arg: never) {
  throw new Error(`never: ${arg as unknown}`);
}

I think the only safe ways to use a variable that is typed or narrowed to be never are:

1. assigned to a variable/member with type never
2. passed as a function/method parameter typed as never
3. cast as unknown

[mmis1000]

I think that code should be flagged:

function assertNever(arg: never) {
  throw new Error(`never: ${arg}`); // <- unsafe use of never right here
}

function test(x: 'a') {
  switch (x) {
    case 'a': return;
    default: assertNever(x);
  }
}

more like

function assertNever(arg: never) { // <- Error: rule 1, non construable never parameter, a function that has such type can't be normally actually called.
   throw new Error(`never: ${arg}`);  // <- Error: rule 2, use of a never value is nonsense, do you really mean to do it?
}

And also

 function test(x: 'a') {
   switch (x) {
     case 'a': return;
     default: assertNever(x); // <- Error: rule 2, assertNever will not be called because a never value is not construable
   }
 }

Because an actual never value can't be create without a faulty runtime type assert or a bad type cast.
If that ever exist, your program is faulty and typescript can't actually guarantee your code works correctly at that point.
(Just like you cast a number to boolean and pass it to random function, what do you expect from ts if you ever do that?)

Probably the secondary rule can be silent by a never type parameter, because typescript programmers use it as a bug check to alert discrepancy between typing and actual value type since forever.

And still, this is not an error, at most a warning.
It is safe if you use typescript in its intended way.
Since you can't construct such value without another big bug in your program.

[adri326]

I must object with the idea that never is unsafe. The issue in the first example provided isn't that the branch where the value has as type never is entered, but rather that the typing of the function wasn't respected:

function useTheValue(a: { foo: number }) {
  return a.foo;
}

export function testFunc(a: string | number) {
  if (typeof a === 'boolean') {
    // If `a === true`, then `useTheValue` will be called with it, causing undefined behavior
    // But really, calling `testFunc` with `true` is already undefined behavior:
    // Calling `a.length` would already cause errors
    return useTheValue(a);
  }
  return 0;
}

This class of bug can also happen without having any never reported by typescript:

function useTheValue(a: {foo: number}) {
  return a.foo;
}

export function testFunc(a: {foo: number} | undefined) {
  if (a) {
    type A = typeof a; // {foo: number}
    
    // if `a === true`, then the function will be called in this branch, causing undefined behavior
    return useTheValue(a);
  }
}

The fact that never is a supertype of any other type T is the very definition of never, and it isn't unsafe, as never cannot be instanced.
never instead serves as a strong tool for handling type negation.

When this rule is broken, however, then instancing never is equivalent to saying that "false is true" (which combined with the principle of explosion would make any logical statement both true and false).
Putting a boolean in a variable whose type is string | number is also equivalent to instancing never, as the transformation if (typeof a === 'boolean') is enough to instanciate never with a boolean value.

I think that a rule that forbids the cast as never would be a reasonable addition, though.

[bbarry]

I'm sorta thinking this is an XY problem.

In

export function testFunc(a: string | number) {
  if (typeof a === 'boolean') {

This type check should already give a warning because the expression typeof a === 'boolean' is always false. Fixing it by expanding the type information on the parameter would make the error appear

export function testFunc(a: {foo: number} | undefined) {
  if (a) {

Maybe there should be a lint rule like strict-boolean-expressions (hint)?

[akomm]

I don't see a single example here that is actually a problem, or something is missing. A never that was not created using as never will never exist in a situation where actual code is executed at runtime. This is what it indicates. The branch in which xis never, will never be reached. All the example I see here correctly fail. For example:

function test(x: 'a' | 'b'): number {
  switch (x) {
    case 'a': return 1;
    case 'b': return 2;
    default:
      x // typeof === never
  }
}

Will fail with Function lacks ending return statement and return type does not include 'undefined'.(2366).

Even if you write default: callFunction(x) the branch is never going to be reached.

In case of the initial example, you won't be able to pass argument a as true because the function signature prohibits it.

Maybe you talk about running typescript outside of strict?

[bradzacher]

A never that was not created using as never will never exist in a situation where actual code is executed at runtime.

That is objectively false. Example:

function doSomethingWithString(arg: string) {
  return arg.charAt(0);
}

function foo(arg: string) {
  // big OOPs - I accidentally used !== instead of ===
  if (typeof arg !== 'string') {
    return doSomethingWithString(arg);
  }
  return '';
}

playground

This code typechecks completely fine, yet at runtime it will crash.
This is clearly a bug in the code that cannot be caught with types because never is assignable to all types.

Another example that happens a lot in the real world where values can come from outside JS (i.e. from an API) - it's possible to have the boundary types wrong.

const result = await fetchFromApi('/api');
switch (result.type) {
  case ResultType.One:
    // do something
    break;

  // ...

  default:
    doSomethingWithNever(result);
}

This code, much like the previous code, will typecheck fine because according to the types we would have handled all of the cases. But say that we add a new enum value to the backend and push that to production before we update the front-end... well now our values at runtime don't match the types we wrote them against - and we'll start operating on never.
This is a very, very common problem, often referred to as "push safety".

[akomm]

The code doesn't break at runtime because you never invoke it and when you invoke it you get type error, for example:

function doSomethingWithString(arg: string) {
  return arg.charAt(0)
}

function foo(arg: string) {
  // big OOPs - I accidentally used !== instead of ===
  if (typeof arg !== "string") {
    return doSomethingWithString(arg)
  }
  return ""
}

foo("ss")

foo(200) // Argument of type 'number' is not assignable to parameter of type 'string'.ts(2345)

Unless you turn off ts strict maybe invoke the plain JS wrongly, but with this argument the whole TS idea is turned upside down. Now you suddenly have to rethink all TS and start testing things that you shouldn't test because TS enforces the types.

This code, much like the previous code, will typecheck fine because according to the types we would have handled all of the cases. But say that we add a new enum value to the backend and push that to production before we update the front-end... well now our values at runtime don't match the types we wrote them against - and we'll start operating on never.
This is a very, very common problem, often referred to as "push safety".

Well this is because you've casted (as) the API response instead of actually parsing it. Which circles us back to "TS can't help you if you make assertions without enforcing it".

[akomm]

Demonstration, what all the examples here are doing in some way (namespace Wrong) and where TS will never give you any runtime safety, and an example (namespace Correct) how it should properly be done:
TS playground

As you can see same type of problem without never being involved, because the source of the problem isn't never, but simply type assertion that doesn't hold. There are N issues that derive from this same cause.

You won't find a general rule to lint whether and assertion is correct and safe or not, by nature. And all issues that derive from it will transitively inherit this property.

[bradzacher]

The code doesn't break at runtime because you never invoke it and when you invoke it you get type error, for example

// module.js
export function unsafe(): any {
  return 1;
}

// local.ts
import {unsafe} from 'module';

foo(unsafe()); // TS check fine, looks fine as well, but crashes

how it should properly be done

It's all fine and dandy to say "you should check every single field that your API sends you to ensure that the value exactly matches the types you've defined", but the reality is that is simply not efficient or even good programming in a lot of cases, definitely not at scale.

There's a tradeoff to be had and you usually need to add some degree of trust. A lot of webapps can send hundreds of API requests in minutes - that could be megabytes of data that you would need to do deep runtime-level assertions on to ensure it's completely correct. This would be horribly slow and just bad for everyone involved given that 99.9% of the time the checks will pass.
Instead it's much more efficient to trust the types are correct and assume that TS will help ensure you aren't doing anything bad with the cases that might be incorrect.

As a real-world example - at Meta they use Relay to generate strict types for their API responses based on the GraphQL queries defined in code and the GraphQL types defined for the server.
So everything is very strictly typed - the types are derived directly from the server's types which are defined within the same repository - it's physically impossible to change the server types without changing the generated types as well.

So how could it be possible that things are out-of-sync? Version pinning. The client's JS version gets pinned to a specific release for a period of time (a few days or so). However the server doesn't get pinned in the same way - it's always on the latest code.

So there are some cases where you might make a change on the server that adds an enum value, and have that enum value be sent and processed by old JS code!
Now what do you want that old JS code to do? Should it crash and burn because it's got an invalid value, or should it handle it gracefully and continue working?

The same thing happens with mobile apps - mobile apps often aren't running on the latest code and instead need to be updated via an app store. Out of date app leads to out of date code.

---

The point I'm getting at is that yes, in a completely idealistic, perfect world - types would exactly match the runtime values and never code would truly NEVER run.
However in the real world there are many cases where it can and will occur: anys, casts through unknown, bad generic types, unsafe object assertions, bugs in code, server mismatches, bad library API calls... the list goes on.

Shit happens - so helping people do the right thing by informing them of potentially unsafe code is a much better course of action than doing nothing.

---

Case-in-point, all of our no-unsafe-* rules. They purely exist to help inform people when they're operating on any. But if you're writing perfect code, there is never any anys, so the rules are useless! Well no - in the real world - cases like bad generics or poorly written library types can easily leak any into your code, often unknowingly.
Better to have a rule that flags it so you can be safe, then to have no rule because anys shouldn't be written.

[akomm]

I'm aware libraries often explicitly define any or simply provide wrong typing. Running strict doesn't help in both cases.

But the nature of those two problems is different. The any is predictably detectable where it occurs: you get lint error, it doesn't leak through your code as you have to somehow parse or at least assert the type.

The later problem isn't really detectable, as it has to be assumed everywhere. You have to assume there is no typing as it can always be an assertion. You basically have to write pure JS code always assuming types wrong. How do you want to lint that?

It is the same with the never, because it derives from it as explained above. Hence the moment, when you have all the possible N rules to handle all potential failure of this nature, type-wise actually safe, you will have parsed you input anyway. But that is exactly what you wanted to prevent.

The parsing will just be spread across the code.

To me it appears such logical checks make more sense, instead of going with never.