proposal: `avoid_non_exhaustive_switches`

stereotype441 commented 1 year ago

avoid_non_exhaustive_switches

Description

With the introduction of patterns support, some switch statements are going to be required to be exhaustive. The criterion is, if the type of the switch expression is a so-called "exhaustive type", the switch must be exhaustive. Otherwise, it's not required to be exhaustive. The language team is still deciding exactly what "exhaustive type" should mean (see https://github.com/dart-lang/language/issues/2693) but currently it's any of the following types:

bool
Null
A enum type
A type whose declaration is marked sealed
T? where T is exhaustive
FutureOr<T> for some type T that is exhaustive
A record type whose fields are all exhaustive types

This means, for instance, that the warning you currently get for failing to cover all possible enum values is going to turn into a compile-time error.

We considered requiring all switch statements to be exhaustive, but decided against it because that would be too much of a breaking change for customers migrating their code to Dart 3.0.

Nonetheless, some customers might want the benefit of a compile-time check to make sure all their switch statements are exhaustive, so we would like to have an optional lint.

Details

During analysis of a switch statement, the resolver (possibly in cooperation with the shared analysis logic) would record a boolean in the AST indicating whether the static type of the switch statement's expression was an exhaustive type. For any switch statements on an exhaustive type, no lint is needed (because there will be a compile error if the switch is not exhaustive).

During exhaustiveness checking (a later compilation stage) the analyzer would record information into the AST about whether any given switch statement is exhaustive (whether exhaustiveness is required or not). For any switch statements on a non-exhaustive type, the lint would fire if the switch is not exhaustive.

We might decide to include additional information in the AST for switches that aren't exhaustive, telling precisly which cases are missing. If so, the lint should format this information nicely for human consumption.

Note that the lint should have no effect on code that has pattern support disabled.

Kind

This lint helps guard against errors by ensuring that users don't forget to write default: clauses.

Bad Examples

f(int i) {
  switch (i) {
    case 0: ...
    case 1: ...
    case 2: ...
  } // Note: no default clause; not all ints are handled
}

abstract class A {}
class B extends A {}
class C extends A {}

f(A a) {
  switch (a) {
    case B(): ...
    case C(): ...
  } // Note: there might be some other class that extends A, and it is not handled
}

Good Examples

f(int i) {
  switch (i) {
    case 0: ...
    case 1: ...
    case 2: ...
    default: ...
  } // Note: any switch statement with a default clause is by definition exhaustive
}

f(int i) {
  switch (i) {
    case 0: ...
    case 1: ...
    case 2: ...
    case num n: ...
  } // Note: `num n` covers all the remaining possibilities so the switch is exhaustive
}

enum E { e1, e2, e3 }

f(E e) {
  switch (e) {
    case E.e1: ...
    case E.e2: ...
  } // Note: E.e3 is not handled, but no lint is needed because this is a compile-time error
}

f(int? i) {
  switch (i) {
    case null: ...
    case int j: ...
  } // Even though there's no default clause, all possible cases are covered
}

f((bool, Object) r) {
  switch (r) {
    case (true, var obj): ...
    case (false, var obj): ...
  } // Even though there's no default clause, all possible cases are covered
}

Discussion

Add any other motivation or useful context here.

Discussion checklist

[ ] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
[ ] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
[ ] If there's any prior art (e.g., in other linters), please add references here.
[ ] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
[ ] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

bwilkerson commented 1 year ago

Will this also apply to switch expressions, or is it limited to switch statements?

For the good example

f(int i) {
  switch (i) {
    case 0: ...
    case 1: ...
    case 2: ...
    case num n: ...
  } // Note: `num n` covers all the remaining possibilities so the switch is exhaustive
}

Should there be a diagnostic warning that num is more general than it needs to be?

stereotype441 commented 1 year ago

Will this also apply to switch expressions, or is it limited to switch statements?

No need for it to apply to switch expressions; those are always required to be exhaustive, so we can rely on a compile-time error to tell users if they aren't.

For the good example
f(int i) {
  switch (i) {
    case 0: ...
    case 1: ...
    case 2: ...
    case num n: ...
  } // Note: `num n` covers all the remaining possibilities so the switch is exhaustive
}
Should there be a diagnostic warning that num is more general than it needs to be?

Personally I sometimes find it useful to give variables explicit types that are more general than what type inference would supply, because either:

I know I'm going to reassign to the variable later, and the reassignment will require the more general type to be valid
I want to use the general type to prevent accientally taking advantage of some functionality (e.g. maybe I know that I have an instance of a private Impl class, but right now I want to make sure I only call methods that are avaiable on the public API).

I can easily imagine either of those two situations applying equally well to pattern variables as well, so my gut feeling is no. But that's just my opinion 😃

dart-lang / linter