Skip to content
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

Analyzer does not warn of missing exhaustiveness when combining sealed classes and enums. #52164

Closed
lrhn opened this issue Apr 25, 2023 · 2 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@lrhn
Copy link
Member

lrhn commented Apr 25, 2023

The analyzer correctly warns if any of the cases are removed from these switches, and not if all the cases are present:

enum E {a, b;}
sealed class S {}
class C1 implements S {}
class C2 implements S {}

void main() {
  E e = E.a as dynamic;
  switch (e) {
     case E.a: 
     case E.b:
  }
  S s = C1() as dynamic;
  switch (s) {
    case C1 _:
    case C2 _:
  }
}

However, it does not recognize a missing enum entry if it's combined with another type:

sealed class SE {}
class C3 implements SE {}
enum EE implements SE {a, b;}

void main() {
  SE se = EE.a as dynamic;
  switch (se) {
    case C3 _:
    // case EE.a:
    case EE.b:
  }
}

Here commenting out the EE.a or EE.b case does not give any errors in the analyzer. The CFE does give the correct error of a missing EE.a case.

Even commenting out both EE.? cases doesn't give an error.

It looks like the analyzer simply misses that an enum declaration can also introduce a subtype of a sealed type which should be included in exhaustiveness.

Checking the code ... yes, that is the problem. And it also mixes mixins:

sealed class S {}

class C1 implements S {}
mixin M2 implements S {}
class C3 with M2 {}

void main() {
  S s = C1() as dynamic;
  switch (s) {
    case C1 _:
    // case M2 _:  // No error from analyzer.
  }
} 
@lrhn lrhn added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Apr 25, 2023
@bwilkerson
Copy link
Member

@scheglov

@scheglov scheglov self-assigned this Apr 25, 2023
@scheglov scheglov added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Apr 25, 2023
@scheglov
Copy link
Contributor

copybara-service bot pushed a commit that referenced this issue Apr 25, 2023
Bug: #52164
Change-Id: Icd6ac232d6356b7e05995b79e5f7d8b33b88a719
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298260
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
…lass.

Bug: #52164
Change-Id: Icd6ac232d6356b7e05995b79e5f7d8b33b88a719
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/298260
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298340
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants