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

Add an IDE refactor to convert switch syntaxes #52798

Closed
rrousselGit opened this issue Jun 27, 2023 · 9 comments
Closed

Add an IDE refactor to convert switch syntaxes #52798

rrousselGit opened this issue Jun 27, 2023 · 9 comments
Labels
analyzer-assist Issues with analysis server assists area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@rrousselGit
Copy link

Currently, a switch can be written in two different ways:

An expression:

return switch (obj) {
  Type() => 'hello world',
};

A statement:

switch (obj) {
  case Type():
    return 'hello world';
}

It is quite common to want to switch between one or the other. Such as for example, initially writing the => syntax. But refactoring to the case syntax because some cases need more than one expression

But although the syntaxes look similar, there are tons of small differences. This makes the refactoring very painful.

Could a quick-fix be added to automate this?

@bwilkerson
Copy link
Member

We already have a refactoring for some structures of switches. For example, given a file containing

int f(int i) {
  switch (i) {
    case 0:
      return 0;
    case 1:
      return 1;
    default:
      return 2;
  }
}

Positioning the cursor in the switch keyword will produce a refactoring to "Convert to switch expression". The inverse is also supported.

The current refactorings are, however, somewhat limited in the sense that they will only be offered when the code matches a pattern we know how to convert (without changing the semantics or introducing errors). If you remove the default from the example above the refactoring is no longer offered because we can't produce valid code.

Are there other code patterns that the refactorings should support?

@bwilkerson bwilkerson added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-assist Issues with analysis server assists labels Jun 27, 2023
@rrousselGit
Copy link
Author

I've tried multiple time using assists to see if one was available, but never saw one.

So either my dart version is somehow out of date, or the current one is probably too limited.

One issue is that it's unclear to me what exactly causes the assist to not show up. In that sense, the discoverability of the feature is poor, and it may be missing important use cases. I'll see if I can make a compilation of the cases I'd need.

@bwilkerson
Copy link
Member

I've tried multiple time using assists to see if one was available, but never saw one.

Do you see the assist using the code I posted above?

I'll see if I can make a compilation of the cases I'd need.

Thanks, that would be very helpful.

@rrousselGit
Copy link
Author

I do see it. Although surprisingly, it appears after the "convert to if chain" refactor, which is likely way less useful overall.
Anyway, I'll investigate why I didn't see it before.

Although I would say, in the case of a missing default you described earlier: IMO the refactor should still be offered.

Users of pattern-matching very much desire compilation errors. I would treat an "unhandled case" error differently than a "missing variable identifier" error.

Alternatively, the refactor could generate a throwing default case:

// TODO handle default
_ => throw UnimplementedError(),

Or if the switch could be exhaustive, it could rely on the "add missing cases" fix to generate all the missing cases.

I would assume that that's what people most likely want. Worst-case scenario, they can ctrl+z or delete the unwanted cases, but I would assume that it would be fairly rare

@rrousselGit
Copy link
Author

int f(int i) {
  switch (i) {
    case 0:
      return 0;
    case 1:
      return 1;
    default:
      return 2;
  }
}

Is this refactor dependent in any way on the installed packages?

I tried this in a different project from the one where I tried earlier, and the refactoring doesn't show up.
I didn't change my Dart version in the meantime, so I don't see an obvious reason for this.

@bwilkerson
Copy link
Member

Is this refactor dependent in any way on the installed packages?

Not to the best of my knowledge.

It is dependent on the language version (we won't suggest converting a switch statement into a switch expression in a library that's pre-3.0), but I don't see anything in the code that would suggest any other reason for it not showing up (other than the already stated possibility that the code doesn't match the heuristics used to decide how to do the conversion.

@rrousselGit
Copy link
Author

rrousselGit commented Jun 28, 2023

There's something fishy going on then. Because in Freezed on the branch "ast", I've been using pattern-matching. Yet when copy-pasting the code in my project, the assist fails to appear.

I'm not really sure what causes this.

@srawlins srawlins added P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug labels Jun 30, 2023
@FMorschel
Copy link
Contributor

Hey @rrousselGit. I did found this only today and it is relatively old. Have you been able to reproduce any cases that the assist doesn't cover? I also found #52958 that is opened but as I stated there:

I believe this specific ask was already solved by #54567. I also created my request to it #55861 missing both.

Current related issues that I know of (1 request to separate the assist to merge the cases and 2 bugs with the current assist respectively):

There are also some other issues I created related to the formatter and some suggested lints all linked to the above issues.

@rrousselGit
Copy link
Author

I think that's fixed now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-assist Issues with analysis server assists area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants