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

Enhanced enums: incorrect missing case clause analyzer warning #49188

Closed
olof-dev opened this issue Jun 6, 2022 · 12 comments
Closed

Enhanced enums: incorrect missing case clause analyzer warning #49188

olof-dev opened this issue Jun 6, 2022 · 12 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

@olof-dev
Copy link

olof-dev commented Jun 6, 2022

The analyzer raises the warning missing_enum_constant_in_switch for the code below, saying Missing case clause for 'data' and similarly for moreData. Of course, they shouldn't have case clauses.

enum E {
  a, b;
  
  static final data = "I'm not a real enum member";
  static const moreData = 'Neither am I';
  
  String get name {
    switch(this) {
      case a:
        return 'AAAA';
      case b:
        return 'BBBB';
    }
  }
}

(Dart SDK version: 2.17.1 (stable))

@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 Jun 6, 2022
@olof-dev olof-dev changed the title Enhanced enums: Enhanced enums: incorrect missing case clause analyzer warning Jun 6, 2022
@bwilkerson bwilkerson added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Jun 6, 2022
@bwilkerson
Copy link
Member

@scheglov @srawlins

I confirmed that this is still happening on head.

Also note the unfortunate "potentially non-nullable" for the diagnostic on name. We should consider dropping the "potentially" when there's no question about nullability.

@scheglov scheglov self-assigned this Jun 6, 2022
@scheglov
Copy link
Contributor

scheglov commented Jun 6, 2022

copybara-service bot pushed a commit that referenced this issue Jun 7, 2022
Bug: #49188
Change-Id: Ie23327fe813de45b2ad63d25f038d070de243d53
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247421
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@mono0926
Copy link

@olof-dev

The code example is missing E..
The following should be correct:

enum E {
  a, b;
  
  static final data = "I'm not a real enum member";
  static const moreData = 'Neither am I';
  
  String get name {
    switch(this) {
      case E.a:
        return 'AAAA';
      case E.b:
        return 'BBBB';
    }
  }
}

@olof-dev
Copy link
Author

olof-dev commented Jun 13, 2022

@mono0926 Thanks for the input. The issue is not that the code doesn't run, but rather about an incorrect analyzer warning about missing case clauses for data and moreData. If you test your code sample e.g. on dartpad.dev you'll see the analyzer warning. I believe a fix has been pushed; see a893703.

@mono0926
Copy link

mono0926 commented Jun 13, 2022

@olof-dev

I think I understand your intention.
I then pointed out that the case specification contains another error that is confusing.
In the current code, removing the static field as shown below causes another error:

enum E {
  a, b;
  
  String get name {
    switch(this) {
      case a: // `E.a` is correct
        return 'AAAA';
      case b: // `E.b` is correct
        return 'BBBB';
    }
  }
}

image

@olof-dev
Copy link
Author

Ah sorry, now I understand what you mean. Just to be clear, I don't think it's the sample code that is incorrect: inside the enum definition, a resolves to E.a. The thing that's incorrect is the analyzer error shown in your screenshot. I think @scheglov's fix (see above) takes care of this too, but I didn't look very closely.

@scheglov
Copy link
Contributor

Yes, the fix covers exactly this issue - understanding of usage of just simple identifiers in case for switch exhaustiveness.

copybara-service bot pushed a commit that referenced this issue Jun 20, 2022
Bug: #49188
Change-Id: I01ab5bb0cb279f40ce3c7917720e1232e00fddea
copybara-service bot pushed a commit that referenced this issue Jun 20, 2022
This is a patch release that fixes:

- Improve analysis of enums and switch (issue [#49188]).
- Fix compiler crash when initializing Finalizable objects (issue [#49075]).

[#49188]: #49188
[#49075]: #49075

Change-Id: If2059474ce2acbadf7f3c6e407f0087d262e2842
copybara-service bot pushed a commit that referenced this issue Jun 20, 2022
This is a patch release that fixes:

- Improve analysis of enums and switch (issue [#49188]).
- Fix compiler crash when initializing Finalizable objects (issue [#49075]).

[#49188]: #49188
[#49075]: #49075

Change-Id: If2059474ce2acbadf7f3c6e407f0087d262e2842
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249161
Reviewed-by: Michael Thomsen <mit@google.com>
@mono0926
Copy link

@olof-dev @scheglov

I've confirmed that this bug has been fixed in Dart 2.17.5 included with the latest stable v3.0.3 of Flutter, and that the code of #49188 (comment) works fine without any warnings.
I think this Issue should be closed.

DartPad: https://dartpad.dev/?id=caa4217a7363edd4b8ea3299cbdfdc48

renggli added a commit to renggli/dart-data that referenced this issue Jun 24, 2022
@scheglov
Copy link
Contributor

Thank you for the confirmation.

@olof-dev
Copy link
Author

Confirmation from here too. Thanks for the quick fix!

@Levi-Lesches
Copy link

Levi-Lesches commented Jan 6, 2023

EDIT: It appears this has been fixed in Dart 2.19, sorry for the confusion.

@scheglov Hate to say but I don't think this is fixed:

enum E {
  a, b;
  
  static final data = "I'm not a real enum member";
  static const moreData = 'Neither am I';
  // NEW CODE
  static const otherData = E.a;  // not a new entry in E
  
  String get name {
    switch(this) {
      case E.a:
        return 'AAAA';
      case E.b:
        return 'BBBB';
    }
  }
}

In this case, the analyzer doesn't complain, but the code doesn't compile on Dart 2.18.6 either:

Error compiling to JavaScript:
Info: Compiling with sound null safety
lib/main.dart:12:14:
Error: A non-null value must be returned since the return type 'String' doesn't allow null.
  String get name {
             ^
Error: Compilation failed.

It's worth noting that adding case E.otherData: return "CCCC"; works as intended: only "AAAA" is returned and the E.otherData case is skipped, but the analyzer complains of a duplicate case.

@srawlins
Copy link
Member

srawlins commented Jan 7, 2023

Thanks for checking @Levi-Lesches . Confirmed on https://dartpad.dev/?channel=beta. The code analyzes cleanly and compiles.

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

7 participants