Skip to content

proposal: avoid_non_exhaustive_switches #58951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
5 tasks
stereotype441 opened this issue Dec 7, 2022 · 2 comments
Open
5 tasks

proposal: avoid_non_exhaustive_switches #58951

stereotype441 opened this issue Dec 7, 2022 · 2 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@stereotype441
Copy link
Member

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 dart-lang/language#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
Copy link
Member

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
Copy link
Member Author

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 😃

@pq pq added the P3 A lower priority bug or feature request label May 22, 2023
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 27, 2024
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-linter Issues with the analyzer's support for the linter package linter-lint-proposal linter-status-pending P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants