Skip to content

Switch Expressions conversion assist should use logical-or when multiple cases share a body #55861

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
FMorschel opened this issue May 28, 2024 · 4 comments
Labels
devexp-assist Issues with analysis server assists legacy-area-analyzer Use area-devexp instead. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@FMorschel
Copy link
Contributor

From the docs:

Logical-or patterns are useful for having multiple cases share a body in switch expressions or statements:

var isPrimary = switch (color) {
  Color.red || Color.yellow || Color.blue => true,
  _ => false
};

When we have a switch statement like the following, the assist for turning it into a switch expression doesn't show:

int a(int x) {
  switch (x) {
    case 0:
      return 0;
    case 2:
    case 1:
      return 1;
    default:
      return throw UnimplementedError();
  }
}

image

Only if there is a single case with a specific body:

image

In this case, I believe the assist should appear and create a logical-or pattern for them.

@bwilkerson bwilkerson added legacy-area-analyzer Use area-devexp instead. devexp-assist Issues with analysis server assists labels May 28, 2024
@FMorschel
Copy link
Contributor Author

I'm not sure if you agree, but I think maybe there could be an assist for merging all of those cases with a logical-or pattern. And that could be run separately or with this assist.

So cases like the above could become:

switch (x) {
  case 0:
    return 0;
  case 1 || 2: // or swapped I'm not sure what to put first here (not sure it matters, just an UX question)
    return 1;
  default:
    return throw UnimplementedError();
}

And then the assist for replacing the switch does appear.

@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label May 31, 2024
@keertip keertip added the P3 A lower priority bug or feature request label Jun 5, 2024
@FMorschel
Copy link
Contributor Author

Created #56180 for the above comment.

@FMorschel
Copy link
Contributor Author

Looking at the code and blame for this feature and trying to understand it to implement it by myself, I just wanted to point out that this is already something working but for some reason not all cases.

Here is the original issue: #54567

If I try to do something like:

enum A {
  a,
  b,
  c,
  d
}

int a(A x) {
  switch (x) {
    case A.a:
      return 0;
    case A.b:
    case A.c:
      return 1;
    default:
      return throw UnimplementedError();
  }
}

Then the assist does appear. So something is not working as it should. Maybe something related to integers not having a finite amount of options?

Will take a look into it whenever I can, just a little update here.

@bwilkerson
Copy link
Member

In the latest version (on HEAD), if I paste the original example into a file:

int a(int x) {
  switch (x) {
    case 0:
      return 0;
    case 2:
    case 1:
      return 1;
    default:
      return throw UnimplementedError();
  }
}

When I click on the switch keyword and open the lightbulb menu, I see a 'Convert to switch expression' option that produces the following:

int a(int x) {
  return switch (x) {
    0 => 0,
    2 || 1 => 1,
    _ => throw UnimplementedError()
  };
}

It appears to me that the limitation has been fixed, but the issue wasn't updated appropriately.

If I'm missing something please let me know and I'll re-open the issue, but for now I'm going to close it.

# 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. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

4 participants