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

unnecessary_parenthesis false positive for switch expressions #5008

Open
JaffaKetchup opened this issue Jul 1, 2024 · 4 comments
Open

unnecessary_parenthesis false positive for switch expressions #5008

JaffaKetchup opened this issue Jul 1, 2024 · 4 comments
Assignees
Labels
false-positive P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@JaffaKetchup
Copy link

Describe the issue
When using a switch expression that is immediately followed by a method on the return result (such as .then when the expression returns Futures, parenthesis are required, but the linter creates a false positive.

To Reproduce

void valid() {
  const v = 0;
  (switch (v) { _ => Future.value() }).then((_) {}); // Lint thrown, syntax valid
}

void invalid() {
  const v = 0;
  switch (v) { _ => Future.value() }.then((_) {}); // Lint not thrown (suggested fix), invalid syntax
}

Expected behavior
The lint should not be thrown

@JaffaKetchup JaffaKetchup changed the title False positive for unnecessary_parenthesis unnecessary_parenthesis false positive for switch expressions Jul 1, 2024
@JaffaKetchup
Copy link
Author

Would appriciate a triage :) No rush ofc.
Kinda feels similar to #4871 (an expression that should be wrapped with brackets in an unexpected place).

@srawlins srawlins added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) P3 A lower priority bug or feature request false-positive labels Jul 15, 2024
@srawlins
Copy link
Member

I'm not sure why no one is triaging this repo. But I threw some labels on.

@srawlins
Copy link
Member

Here's another interesting example. The code could remain legal when removing the parens, if we're not looking at an expression-statement. For example:

Future<void> invalid(Object v) {
  return switch (v) { _ => Future.value() }.then((_) {});
}

This is legal code. However, I think it might be surprising that parentheses aren't required around the switch expression, so I think they should be allowed, even if it doesn't cause a parse error.

@srawlins srawlins self-assigned this Oct 12, 2024
@JaffaKetchup
Copy link
Author

I'm surprised that that is legal but the version without return isn't. I guess the answers can be found in the language definition, but I agree that does seem like a case where you might want to allow unnecessary brackets.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Oct 14, 2024
Fixes dart-lang/linter#5008

Change-Id: I423dbc4a431b5495b032c7803d6b691b65179022
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/389783
Auto-Submit: Samuel Rawlins <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
Reviewed-by: Phil Quitslund <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false-positive 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

2 participants