"Add missing case clauses" only inserts first case for switch pattern statements #52180

Activity

1. DanTup

   added

   legacy-area-analyzerUse area-devexp instead.Use area-devexp instead.

   devexp-serverIssues related to some aspect of the analysis serverIssues related to some aspect of the analysis server

   devexp-quick-fixIssues with analysis server (quick) fixesIssues with analysis server (quick) fixes

   on Apr 26, 2023
2. DanTup

   changed the title [-]"Add missing case clauses" does not work correctly for switch pattern statements[/-] [+]"Add missing case clauses" only inserts first case for switch pattern statements[/+] on Apr 26, 2023
3. bwilkerson commented on Apr 26, 2023

   

   bwilkerson

   on Apr 26, 2023

   Member

   More actions

   Unfortunately, that behavior is the best we can do at the moment. With the switch to patterns we had to write a much more general algorithm to compute exhaustiveness, and that algorithm currently only finds the first case that isn't covered. There have been discussions about extending the algorithm to find all of the missing cases so that we can do a better job, but I don't know what the state of that work is.

   @scheglov

   👍React with 👍1Reacted by Phil Quitslund
4. bwilkerson

   added

   P3A lower priority bug or feature requestA lower priority bug or feature request

   on May 5, 2023
5. deimantasa commented on May 21, 2023

   

   deimantasa

   on May 21, 2023

   More actions

   Any updates to this? It feels like a serious bummer, especially when having several (5+) enum cases to deal with. Auto-fill used to save tons of time.

   P.S. Android Studio (Flamingo) does not even generate one case. Only by hovering, first case is hinted.
   
   

   👍React with 👍3Reacted by Rajesh, Petru Lutenco and Luca Venir
6. pq commented on Jul 6, 2023

   

   pq

   on Jul 6, 2023

   Member

   More actions

   There have been discussions about extending the algorithm to find all of the missing cases so that we can do a better job, but I don't know what the state of that work is.

   @scheglov?
7. lhengl commented on Jul 12, 2023

   

   lhengl

   on Jul 12, 2023

   Last edited by lhengl

   More actions

   Just upgraded to dart 3 and stumbled into this problem. There used to be fixes available for exhaustiveness, now there isn't. With the prevalent use of enum and switch statements, I'm surprised this issue doesn't have more upvotes.
8. AbdulazizRasulbek commented on Jul 12, 2023

   

   AbdulazizRasulbek

   on Jul 12, 2023

   More actions

   same issue for sealed classes

   👍React with 👍2Reacted by Felix and Stryder Crown
9. srawlins

   added

   type-bugIncorrect behavior (everything from a crash to more subtle misbehavior)Incorrect behavior (everything from a crash to more subtle misbehavior)

   on Mar 15, 2024
10. bwilkerson commented on Jun 13, 2024

    

    bwilkerson

    on Jun 13, 2024

    Last edited by DanTup

    Member

    More actions

    This has been fixed by https://dart-review.googlesource.com/c/sdk/+/371062 and should be available in the next stable release.

    🎉React with 🎉1Reacted by Abdulaziz Rasulbek
11. bwilkerson

    closed this as completedon Jun 13, 2024
12. DanTup commented on Jun 13, 2024

    

    DanTup

    on Jun 13, 2024

    Last edited by DanTup

    CollaboratorAuthor

    More actions

    @bwilkerson I tested this and it seems to work great 🥳 but I noticed the test I linked up top is still marked @FailingTest:

    sdk/pkg/analysis_server/test/src/services/correction/fix/add_missing_enum_case_clauses_test.dart

    Line 41 in 498dcc0

    @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/49759')

    It fails because of "Unnecessary error filter" (a test bug), but if I remove the WithFilter, it fails with:

    Expected to find fix dart.fix.add.missingEnumCaseClauses in
    Fix(kind=dart.fix.add.missingSwitchCases, change={"message":"Add missing switch cases","edits":[{"file":"C:\home\test\lib\test.dart","fileStamp":0,"edits":[{"offset":49,"length":0,"replacement":" case E.a:\r\n // TODO: Handle this case.\r\n case E.b:\r\n // TODO: Handle this case.\r\n case E.c:\r\n // TODO: Handle this case.\r\n"}]}],"linkedEditGroups":[],"id":"dart.fix.add.missingSwitchCases"})

    It looks like the fix provided is dart.fix.add.missingSwitchCases but this test is for dart.fix.add.missingEnumCaseClauses. It's not clear to me what the difference is here (I thought maybe one is for the original switch statement parsing and the other is since switch expressions were added, but I don't think that explains how any of other other tests in this class would be passing).

    Oh, as I was about to send this I noticed all the non-failing tests in that class are marked // @dart=2.19.

    So.. should all of the @FailingTests in the class AddMissingEnumCaseClausesTest now be moved into AddMissingSwitchCasesTest_SwitchStatement since it seems for >2.19 they'll all be handled by that fix instead?
13. bwilkerson commented on Jun 13, 2024

    

    bwilkerson

    on Jun 13, 2024

    Member

    More actions

    The fix kind DartFixKind.ADD_MISSING_ENUM_CASE_CLAUSES is associated with AddMissingEnumCaseClauses, which is associated with StaticWarningCode.MISSING_ENUM_CONSTANT_IN_SWITCH. That warning code is produced only in pre-pattern-enabled code.

    The fix kind DartFixKind.ADD_MISSING_SWITCH_CASES is associated with AddMissingSwitchCases, which is associated with both CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_EXPRESSION and CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_STATEMENT, both of which are only produced in pattern-enabled code.

    ... should all of the @FailingTests in the class AddMissingEnumCaseClausesTest now be moved into AddMissingSwitchCasesTest since it seems for >2.19 they'll all be handled by that fix instead?

    If the tests are intended to be run with patterns enabled, then yes, they should be moved because they will produce the CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_EXPRESSION and CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_STATEMENT diagnostics, and AddMissingSwitchCasesTest is where those tests should live.

    👍React with 👍1Reacted by Danny Tuppeny
14. copybara-service

    added a commit that references this issue on Jun 14, 2024

    Move tests for switch enum cases for >= 2.19 from `AddMissingEnumCase…

    ...

    1b302b8