unnecessary_parenthesis should not be so strict around cascades

[srawlins]

There are currently some allowances for "unnecessary" parenthesis around cascades:

// a..b=(c..d) is OK
if (node.expression is CascadeExpression &&
    node.thisOrAncestorMatching(
            (n) => n is Statement || n is CascadeExpression)
        is CascadeExpression) {
  return;
}

This says "If the parenthesized thing is a cascade, and the parentheses are in some expression hierarchy with a cascade above them, then allow the parens."

I think this should be changed to an "or":

• If the parenthesized thing is a cascade, leave it alone.
• If the parentheses have a cascade somewhere above them as an ancestor, leave it alone.

Here are some (simplified) examples I've seen, where this lint rule complains today, which highlight common "unnecessary" parentheses patterns, which I think we should allow:

final response = await _somethingOrOtherClient.sendThingy(
    SendThingyRequest()
      ..foo = foo
      ..verificationBar = (isSomethingSelected
          ? SendThingyRequest_VerificationBar.ONE
          : SendThingyRequest_VerificationBar.TWO));

Here the parens defensively guard against unknown cascade vs conditional (?:) precedence. They arguably make the code more readable, even when dartfmt helps as well with proper indentation.

GeneralSomethingOrOtherGraph newGraph = GeneralSomethingOrOtherGraph()
  ..setting = (setting.clone()
    ..categorySetting = (setting.categorySetting.clone())
    ..categorySetting.foo = true
    ..explicitBar = false);

These parens again guard against the precedence misunderstandings with nested cascades, method calls and assignments.

ReportSomethingEncoder get eventSomethingEncoder => _forStringField('foo',
    getValue: (definition) =>
        definition.graphDefinition.showThingyTable ? 'x' : '',
    setValue: (definition, value) => definition.graphDefinition =
        (GraphDefinition()..showThingyTable = value == 'x'));

Here the parens guard against precedence between the inner cascade and an outer assignment.

Rather than play more games with how to allow cascades, I propose to just allow "unnecessary" parens around any cascade, and below any cascade.

[srawlins]

@a14n what do you feel about this? What if I sent out a PR for the change I propose above?

[a14n]

After running dartfmt I don't see a big value in those "unnecessary" parens.

I propose to just allow "unnecessary" parens around any cascade, and below any cascade.

That means the following code would be allowed and it's not good imho:

GeneralSomethingOrOtherGraph newGraph = GeneralSomethingOrOtherGraph()
  ..setting = (setting.clone()
    ..categorySetting = (setting.categorySetting.clone())
    ..categorySetting.foo = (true) // bad
    ..explicitBar = (false));

[srawlins]

unnecessary parens are not allowed individual identifiers, such as (true) and (false).