[`unnecessary_parenthesis`] false negative inside switch expressions

[FMorschel]

Describe the issue
unnecessary_parenthesis false negative.

When having a code like:

T a<T>(int x, bool test) {
  switch (x) {
    case 0:
      return test ? 1 : 2 as T;
    default:
      return 3 as T;
  }
}

The return of case 0 warns: A value of type 'Object?' can't be returned from the function 'a' because it has a return type of 'T'.

So in that case, we need to wrap that in parentheses.

return (test ? 1 : 2) as T;

This doesn't throw anymore.

When changing to the new pattern switch syntax, we can cast at the end as follows:

T a<T>(int x, bool test) {
  return switch (x) {
    0 => (test ? 1 : 2),   // Not triggering
    _ => 3,
  } as T;
}

And now that is not needed anymore and should trigger the lint.

[FMorschel]

I'm not sure that this is the same, but I've also seen:

final email = (user.email != null) ? (user.email!) : '';

This is a dumb example, but my point is that in other ternary expressions that I've used, I saw this.

I think that the (user.email!) expression should trigger this as well. I'm not sure because this might be intentional to leave ternary expressions with possible parentheses around all possible results.

If you agree that this is also another false-negative I can create another issue.

Edit

This is a false negative that occurs in any of the three places of a ternary operator.

[bwilkerson]

Related to #57126

[srawlins]

Note that the unnecessary_parenthesis lint rule does not report every case of a parenthesized expression that affects parsing or semantics of code. See the description:

Parentheses are considered unnecessary if they do not change the meaning of the code and they do not improve the readability of the code. The goal is not to force all developers to maintain the expression precedence table in their heads, which is why the second condition is included. Examples of this condition include:
[...]
expressions with whitespace between tokens [...]

The rule will not report parens around conditional expressions in some cases. E.g. the test_conditionalInside_expressionBody test case (int f() => (1 == 1 ? 2 : 3);). There are some cases where we do report the parens, like when the parens are "near" a series of other expressions, like in a list literal or an argument list (see other test_conditionalExpression test cases. We've previously had much pushback indicating that parens around a conditional expression can improve readability, because of all the internal whitespace, with other adjacent expressions.

[srawlins]

That looks like a separate issue. If you open a new ticket, make sure it's not been recently fixed with a186457 <a186457>, which landed on Oct 9.

…

On Mon, Oct 14, 2024 at 3:54 AM FMorschel ***@***.***> wrote: I'm okay with that, but I do think that the (user.email!) could be reviewed in: final email = (user.email != null) ? (user.email!) : ''; Should I open another issue or update the description for this one? — Reply to this email directly, view it on GitHub <#57137>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZF74HNDLALM4FITTWNYDZ3OPHDAVCNFSM6AAAAABILVJ2B6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJQHA2TINRRGE> . You are receiving this because you were assigned.Message ID: ***@***.***>

[dart-github-bot]

Summary: The unnecessary_parenthesis lint incorrectly identifies parentheses as necessary in a switch expression's case statement when using a ternary operator. The issue is exacerbated by the new switch expression syntax.