Skip to content

Quick assist to convert switch statements to switch expressions #50417

New issue

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

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

Already on GitHub? # to your account

Closed
Tracked by #49960
munificent opened this issue Nov 9, 2022 · 7 comments
Closed
Tracked by #49960

Quick assist to convert switch statements to switch expressions #50417

munificent opened this issue Nov 9, 2022 · 7 comments
Assignees
Labels
devexp-assist Issues with analysis server assists legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on
Milestone

Comments

@munificent
Copy link
Member

munificent commented Nov 9, 2022

With the in-progress work on patterns, we're also adding a new switch expression. It would be very cool if we could support a quick fix that would allow you to convert switch statements to switch expressions when it makes sense. I think that's feasible when:

  • The switch is exhaustive over its type:
    • It's covering all cases in an enum, or
    • It has a default case.
  • And all of the cases (including default) have the same structure:
    • A single return statement, or
    • An assignment to the same variable or setter, or
    • Maybe a call to the same function but with a different argument.
  • While also allowing cases that abort too.

Then in that case, the shared structure in the cases can be hoisted out and the switch statement turned into a switch expression whose result is then applied to the shared structure. So, for example:

// Convert return:
switch (color) {
  case Color.red:
    return 'red';
  case Color.blue:
    return 'blue';
  case Color.green:
    throw 'Green is bad';
  case Color.yellow:
    return 'yellow';
}

return switch (color) {
  Color.red => 'red',
  Color.blue => 'blue',
  Color.green => throw 'Green is bad',
  Color.yellow => 'yellow'
};

// Convert assignment:
switch (color) {
  case Color.red:
    name = 'red';
    break;
  case Color.blue:
    name = 'blue';
    break;
  case Color.green:
    throw 'Green is bad';
  case Color.yellow:
    name = 'yellow';
    break;
}

name = switch (color) {
  Color.red => 'red',
  Color.blue => 'blue',
  Color.green => throw 'Green is bad',
  Color.yellow => 'yellow'
};

// Convert argument:
switch (color) {
  case Color.red:
    showName('red');
    break;
  case Color.blue:
    showName('blue');
    break;
  case Color.green:
    throw 'Green is bad';
  case Color.yellow:
    showName('yellow');
    break;
}

showName(switch (color) {
  Color.red => 'red',
  Color.blue => 'blue',
  Color.green => throw 'Green is bad',
  Color.yellow => 'yellow'
});
@munificent munificent added legacy-area-analyzer Use area-devexp instead. devexp-quick-fix Issues with analysis server (quick) fixes labels Nov 9, 2022
@bwilkerson
Copy link
Member

The question, in my mind, is whether this should be tied to a 'prefer_switch_expressions' lint, or whether this should just be available as a quick assist in places where it could be applied. (I'd be hesitant to say that all such switch statements are marked as a problem without users explicitly choosing that style.)

@bwilkerson bwilkerson added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Nov 9, 2022
@munificent
Copy link
Member Author

If it were me, I wouldn't make this a "prefer" lint. Just an assist users can manually invoke for specific switch statements if they want to. I agree with you that I don't think all switch statements should be expressions just because they could be. Sometimes a statement is just easier to read.

@bwilkerson bwilkerson added devexp-assist Issues with analysis server assists and removed devexp-quick-fix Issues with analysis server (quick) fixes labels Nov 11, 2022
@srawlins srawlins added this to the Dart 3 stable milestone Nov 30, 2022
@srawlins srawlins added P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Nov 30, 2022
@srawlins srawlins modified the milestones: Dart 3 stable, Dart 3 beta 3 Nov 30, 2022
@srawlins
Copy link
Member

srawlins commented Mar 2, 2023

CC @pq @keertip

@bwilkerson
Copy link
Member

We probably shouldn't convert a statement that isn't exhaustive, so we'll need to wait for that piece to be done before we implement the assist.

@pq
Copy link
Member

pq commented Mar 2, 2023

If it were me, I wouldn't make this a "prefer" lint.

Proposal here: #58862

@pq pq self-assigned this Mar 2, 2023
@pq pq changed the title Quick fix to convert switch statements to switch expressions Quick assist to convert switch statements to switch expressions Mar 2, 2023
copybara-service bot pushed a commit that referenced this issue Mar 6, 2023
See: #50417

Initial work to support return conversions.  Arguments and assignments to come and will likely lead to some refactoring but there's enough here to benefit from some early feedback.

Thanks in advance! :D

Change-Id: Ic3d0349aa12d8c951e3afe0da3e00e2777480e38
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/286861
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@pq
Copy link
Member

pq commented Mar 15, 2023

Implemented w/ b68719c

@pq
Copy link
Member

pq commented Mar 23, 2023

See also #51826 that adds support for case clauses w/ implicit breaks.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
devexp-assist Issues with analysis server assists legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

4 participants