feat(eslint-plugin): add switch-exhaustiveness-check rule

[drets]

Hi 👋 ,

I was talking about this thing in microsoft/TypeScript#33160 and Nathan Shively-Sanders suggested to use typescript-eslint for such things, and so I did.

By introducing this rule people may stop to write small hacks suggested in https://stackoverflow.com/questions/39419170/how-do-i-check-that-a-switch-block-is-exhaustive-in-typescript and use this rule instead.

Let me know if this rule makes sense, thank you.

[typescript-eslint]

Thanks for the PR, @drets!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

[drets]

@bradzacher hi.

Whom I can ping to get a code review?

Have a wonderful day.

[bradzacher]

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

All you need to do is raise the PR.
After that it's in the queue of stuff for us to get to when we have time to review.

[bradzacher]

Code so far LGTM.

A few things though:

---

How does this code handle enum values? Could you please add some tests to validate that they work as expected.

---

Similarly, could you please add tests for other primitives, ie:

• number: 0 | 1 | 2
• boolean: true | false (yeah this is weird, but might as well it works as expected
• mixture of all 0 | 1 | 'two' | 'three' | true

what about symbols?

---

How does this code handle type references?

type A = 'a';
type B = 'b';
type C = 'c';
type Union = A | B | C;

I don't know how checker.typeToString prints out these cases.

could you please add some tests for this so it's clear and handled properly?

---

How does this handle typeof types:

const A = 'a';
const B = 1;
const C = true;

type Union = typeof A | typeof B | typeof C;

---

How does this implementation handle object type unions: type T = { a: 1 } | { b: 2 }?
I know it's a weird edge-case, but should the allowed union types be limited in some way so it cannot report in this case?

---

One question I'd like to discuss: is it worth having the fixer?
I'm wondering how much value there is in inserting case Foo: throw new Exception(), esp when there's the implementation weirdness with intersection types, and having to deal with indentation (plus any other weirdness that comes from the above cases).

[cust0dian-old]

@drets I'd like to help get this going again: I've opened a PR into your fork with requested changes, let me know if there's anything else I can help with.

@bradzacher I think I can also answer your questions.

How does this code handle enum values?

enums are handled the same as unions; I've added tests for them in my fork and I think they work as expected.

Similarly, could you please add tests for other primitives [...]

Added tests for these as well, no surprises there.

[...] what about symbols?

This implementation only supports unions/enums of singleton types. As far as I know, TS currently doesn't have literal symbols on type level so there's no way to construct a singleton type out of specific symbols, only refer to all of them (symbol type). If programmer runs this rule against a code like

type T = true | symbol

function test(value: T): number {
  switch (value) {
    case true: return 1
  }
}

we'll return an error:

Switch is not exhaustive. Cases not matched: symbol

Only way to satisfy the rule would be to add a default branch to the switch.

How does this code handle type references?
[...]
I don't know how checker.typeToString prints out these cases.

This code handles type references correctly, in a sense that non-exhaustive switches are reported as such; however checker.typeToString seems to return underlying type, not an alias, e.g. a non-exhaustive match for the code you provided would report something like:

Switch is not exhaustive. Cases not matched: "a" | "b" | "c"

How does this handle typeof types:

As long as values behind typeof'ed variables can be represented as singleton types, no surprises there, i.e. types in your example work exactly as expected.

How does this implementation handle object type unions: type T = { a: 1 } | { b: 2 }?

This implementation doesn't handle this case at all and I'm not sure it's applicable: I can't think of a way to switch on this type since switch has to work with equality at a value-level, i.e.:

type T = { a: 1 } | { b: 2 }

const a1: T = { a: 1 }
const a2: T = { a: 1 }

a1 !== a2 //=> true
// even though `a1` and `a2` are members of `{ a: 1 }` type

However, if type is a discriminated union, this reduces the problem to union of singleton types (which is handled correctly).

One question I'd like to discuss: is it worth having the fixer?

I would agree that a fixer for this would be potentially more trouble than it's worth. For example, if programmer has non-singleton types in the union we construct invalid JS:

// Input:
type T = true | number

function test(value: T): number {
  switch (value) {
    case true: return 1
  }
}

// Autofix:
    case number: { throw new Error('Not implemented yet: number case') }

which is just asking for trouble.

[drets]

@cust0dian wow, thank you for jumping in! I will merge your PR in the next few days.
Could you please remove the fixer if you guys think it doesn't worth the play?

Personally, the fixer is a killer feature of this rule. I'd like to declare union type with N branches, then write switch (myUnion), run linter and code is written automagically 😍. But I understand that it should be done perfectly otherwise it doesn't worth it.

[bradzacher]

TS currently doesn't have literal symbols on type level so there's no way to construct a singleton type out of specific symbols, only refer to all of them (symbol type)

You're partially correct. Because each symbol is 100% unique, you can't just create a type like Symbol<'a'>.
You can, however, reference a symbol in a type, and typescript will understand its meaning.

You can also key an interface with a symbol variable, and typescript will understand it

const x = Symbol('a');

type T = typeof x | true;
declare const y: T;

switch (y) {
    case true:
        y; // typeof y === true
        break;

    case x:
        y; // typeof y === typeof x
        break;
}
if (y === x) {
    y; // typeof y === typeof x
}

interface Foo {
    [x]: 1;
}
declare const foo: Foo;
const bar = foo[x]; // typeof bar === 1

ts playground repl

Important to note that the type of the variable y in the above example is reported by typescript with the UniqueESSymbol and ESSymbolLike type flags.
ts-ast-viewer repl

---

In regards to the fixer, it might be worth adding a suggestion fixer.
These are a relatively recent addition to eslint.

https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions

They aren't automatically applied by --fix, and must be manually chosen by the developer via an IDE, so it's safer to build wacky fixers that can create broken code, because it can never be added accidentally by automation.

[cust0dian-old]

@bradzacher I see, thank you for explaining it!

The code does handle types like that, albeit reporting isn't ideal:

const a = Symbol('a')
const b = Symbol('b')
const c = Symbol('c')

type T = typeof a | typeof b | typeof c

function test(value: T): number {
  switch (value) {
    case a: return 1;
  }
}

// results in error:
//   Switch is not exhaustive. Cases not matched: unique symbol | unique symbol

Would it be appropriate to use symbol.escapedName here, so we return something like

Switch is not exhaustive. Cases not matched: typeof b | typeof c

in the case of symbols?

[bradzacher]

Would it be appropriate to use symbol.escapedName here, so we return something like

Probably, I'd say that it'd be "good enough" to check the type's flags for the ESSymbolLike flag, and use escapedName in that case.

[bradzacher]

Mostly LGTM, great work.
A few comments to address, mostly tightening tests.
Thanks for doing this.

[bradzacher]

LGTM - thanks all for working on this!