Enhancement: [restrict-template-expressions] allow expressions with never type

[mikeyhew]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Link to the rule's documentation

https://typescript-eslint.io/rules/restrict-template-expressions#allownullish

Description

The restrict-template-expressions rule currently complains if the template expression has type never. This occurs, for example, if you have code like this:

// more variants may be added to Foo in the future
type Foo =
  | {type: "a", value: number};

function checkFoosAreMatching(foo1: Foo, foo2: Foo) {
  if (foo1.type !== foo2.type) {
    // since Foo currently only has one variant, this code is never run,
    // and `foo1.type` has type `never`.
    throw new Error(`expected ${foo1.type}, found ${foo2.type}`);
                                                  // ^ Invalid type "never" of template literal expression.
  }
}

I don't see the benefit of the lint warning here, which complains that foo1.type and foo2.type have type never and says, Invalid type "never" of template literal expression.

I am suggesting changing the lint rule's behaviour to allow template expressions to have the type never. This could be done as a new option called allowNever, although I personally don't see the point in giving the option to disallow never in template expressions. Why should this lint rule be complaining about dead code?

Fail

const foo: "a" | null;
const bar: "a";

if (foo !== bar) {
    // `foo` has type `null`, which is not allowed
    // with the default options for this rule
    throw new Error(`${foo} and ${bar} are not equal);
}

Pass

const foo: "a";
const bar: "a";

if (foo !== bar) {
    // `foo` and `bar` have type `never`, which should be allowed
    // by this rule
    throw new Error(`${foo} and ${bar} are not equal);
}

Additional Info

This same logic should also apply to similar rules like restrict-plus-operands (no need to complain that an operand has type never), although I haven't checked to see how that rule behaves for never operands.

[bradzacher]

The entire point of the lint rule is to ensure you're properly formatting your values for your users so that you don't leak "user-hostile" strings (like booleans, unconstrained decimals, improperly formatted lists, etc).

This does mean, however, that in places you're creating strings that don't get seen by the user - for example console logs or thrown errors - then you'll see reports that you may not care about because you as the dev are happy to get "user-hostile" values in your strings.

never is the absence of any type - TS believes the code to be unreachable. So obviously leaking a completely unknown value to users is going to be bad, which is why reporting on this is good in the general case.

Is it a good idea to turn on an option to allow never always when you really only want it specifically for the internal error-reporting usecase? I don't think that it is a good idea to weaken your entire codebase in this way for a handful of never usecases that would exist.

Also given that this rule has existed for ~2 years and nobody has previously wanted such an option - it's likely not a case that too many people want to always allow?

[Josh-Cena]

I second this. I wonder how many people got deterred (myself, for one) because the last request for the exact same thing was closed as WAI: #3069 The demand is certainly always there. I have yet to see a case where a never in a template literal is not deliberate for debugging purposes.

[phaux]

I think it should be changed because strict-boolean-expression also allows never by default and I think it makes sense because of reasons outlined above.

It's impossible to check if use of never is valid because if the types are right then the program should never reach here, so theoretically everything should be allowed.

If you are paranoid about something being never when it shouldn't then a new rule could be more useful e.g. to ban as never hack.

Here's another use case:

type Opt = "foo" | "bar"

function doThing(opt?: Opt) {
  if (opt == null) {
    return
  }
  if (opt === "foo") {
    doFooThing()
    return
  }
  if (opt === "bar") {
    doBarThing()
    return
  }
  // opt is `never` here.
  // If we add another variant to Opt then a new error will show up here 
  // to remind us to handle the new case.
  // If an unexpected value is passed anyways then we get an exception:
  // "Invalid opt: value".
  assertNever("opt", opt)
}

function assertNever(kind: string, value: never) {
  throw new Error(`Invalid ${kind}: ${value}`)
}

[JoshuaKGoldberg]

I can't find the PR on demand 😕 but I've experienced this annoyance before and think it's reasonable to want an option for the never case because of these exhaustive switch statements that throw errors. We don't have to make it on-by-default in v5 but could consider that along with all other major rule options changes for the next major version if we want.

allowNever seems like a good name. 👍

[lgrahl]

I kinda have the same question for unknown, considering it almost falls into the same category and that allowAny already exists. (Sorry for slightly hijacking this issue.)

[domdomegg]

I've implemented allowNever in #6554

[bradzacher]

My biggest issue with this is that ultimately you have absolutely no idea what you might get in your string.

As an example - if for some reason the thing is an object, you'll end up with `Invalid: ${value}` resulting in the string `Invalid: [object Object]` which is... completely and utterly useless for users, consumers of an API, or even yourself in your own logs.

However if you instead did something like `Invalid: ${JSON.stringify(value)}` - not only would the lint rule be satisfied, but it would also result in an actually useful string for logs / api consumers (though it would still be useless for users).

Ultimately by allowing never you're beholden to whatever the toString implementation is for the actual runtime value that's leaked in accidentally... which is entirely the thing the rule is trying to stop in the first place!

It just seems like this is going against the philosophy of what the rule is trying to enforce - which is why, IMO, this is a valid error that should be reported in all cases, and should not be configurable.

[mikeyhew]

My biggest issue with this is that ultimately you have absolutely no idea what you might get in your string.

I mean, the type system is telling you that you won't be getting a string, period. Sure, TypeScript is unsound so sometimes code can run where something has a type that's different from what it's supposed to, but if that's true for never then that's true for any other type. A value in a template expression is just as capable of printing out [object Object] when the template expression has type string as when it has type never. I prefer that ESLint trust the type system in this case, and it sounds like there's a few other people in here who prefer that approach too, so it won't hurt to at least add the option.

As for your other comment that this issue hasn't come up in the past: I can only speculate, but I know that TypeScript has gotten better in recent versions at inferring never in more places, so it could be that this is happening more now as people upgrade.

[bradzacher]

I mean, the type system is telling you that you won't be getting a string, period.

It's not though, unless your never branch is specifically typeof thing !== 'string'.

TypeScript is unsound so sometimes code can run where something has a type that's different from what it's supposed to, but if that's true for never then that's true for any other type. A value in a template expression is just as capable of printing out [object Object] when the template expression has type string as when it has type never.

That is completely a strawman argument. In a nutshell you're saying "we can't trust the type system ever so why does it matter now?".
TS is unsound in many ways - for sure, esp when using as assertions.
However unless you've got some seriously bad code (or untyped callers) then it's very, very likely that a string is a string and the types match things at runtime correct. If it's not then it's more likely that your entire application has blown up before getting to a never location.

OTOH never is the absence of a type - it's TS saying "IDK I think this should be unreachable". Put another way - when you are acting on a never you are in a location in your code wherein you're in "undefined behaviour" - the types say the code shouldn't ever run, and if it does run you literally don't know what the value is.

In a lot of cases you can make an educated guess; for example if you're switching over a disjoint union's discriminant then the never case is probably something like a new case the backend is sending that the frontend types haven't yet been updated to understand - thus it's likely the never is actually an object.

The way I see it your never will fall into one of three camps:

1. "I can make educated guess that the never is likely of type T..."
   1. "... and T is a nicely stringifiable type"
   2. "... and T is NOT a nicely stringifiable type"
2. "There's no way to know what the never could be"

For (1.i) - you should also probably assert it's of a specific type `Invalid ${value as string}` to clearly document your assumptions to other engineers.

For (1.ii) - you know it's going to print garbage inside the template string - so you're going to want to nicely stringify it in some way.

For (2) - you have no idea whether or not it'll create garbage - so you're going to want to nicely stringify it in some way.

So by my reckoning it looks like there's not really a case where you want a never to be passed directly to a template literal. Which is why I say that I don't really think it makes sense to have an option for this - it seems like there are just better ways to do this that will result in less potential garbage.