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

require_trailing_commas is not triggered on switch expressions #57138

Closed
FMorschel opened this issue May 28, 2024 · 5 comments
Closed

require_trailing_commas is not triggered on switch expressions #57138

FMorschel opened this issue May 28, 2024 · 5 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. linter-false-negative P3 A lower priority bug or feature request triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

Describe the issue
require_trailing_commas is not triggered on switch expressions.

To Reproduce

enum E {
  a,
  b,
  c,
  d,
  e,
  f;
}

int a(E e) {
  return switch (e) {
    E.a => 0,
    E.b => 1,
    E.c => 2,
    E.d => 3,
    E.e => 4,
    E.f => 5     // Should trigger
  };
}

Expected behavior
Triggering of require_trailing_commas.

@FMorschel
Copy link
Contributor Author

Also, the current assist to transform switch statements into switch expressions doesn't place a trailing comma there even if the lint is enabled.

If the reviewer agrees, I can make a new issue if necessary.

@bwilkerson
Copy link
Member

I'm not sure we want to trigger the lint in this case.

My understanding is that the lint exists because of the way a trailing comma in a parameter or argument list impacts the formatter, and some users wanted their code to always be formatted per the trailing-comma style. To the best of my knowledge, the formatter behaves the same whether of not there's a trailing comma in a switch expression, making this an odd choice.

If the formatter is updated so that the trailing comma no longer impacts the behavior, then we're likely to consider deprecating this lint.

@FMorschel
Copy link
Contributor Author

FMorschel commented May 28, 2024

I'm sorry but it behaves differently if the switch expression is short:

enum E {
  a,
  b,
  c;
}

int a(E e) {
  return switch (e) { E.a => 0, E.b => 1, E.c => 2 };
}

I'm sorry if I was not clear enough to start.

Edit

And if the trailing comma is added, it breaks the switch:

return switch (e) {
  E.a => 0,
  E.b => 1,
  E.c => 2,
};

@bwilkerson
Copy link
Member

Thanks, I don't think I've written a switch expression short enough to fit on one line.

My expectation, given that, is that the future release of the formatter will likewise add and remove the trailing comma from switch expressions, making it unnecessary to extend this lint rule to support that case.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label May 31, 2024
@bwilkerson bwilkerson added the P3 A lower priority bug or feature request label Jun 24, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 2024
@dart-github-bot
Copy link
Collaborator

Summary: The require_trailing_commas linter rule doesn't flag missing trailing commas in switch expressions, despite being expected behavior.

@dart-github-bot dart-github-bot added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 18, 2024
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. linter-false-negative P3 A lower priority bug or feature request triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants