cascade_invocations: Don't trigger on ?? assignments

[matanlurey]

void log(String message, {Level severity}) {
  final Logger logger = Zone.current[_zoneKey] ?? _defaultLogger;
  // ignore: cascade_invocations
  logger.log(message, severity: severity);
}

Yeah, I could have written:

void log(String message, {Level severity}) {
  ((Zone.current[_zoneKey] ?? _defaultLogger) as Logger).log(...)
}

But that really harms readability just to play code golf.

[aemino]

Given #57626 and #57631, it seems like at the present moment, the cascade_invocations rule has a lot of potential to cause false positives that lead to unwieldy code as a "solution". Perhaps simply having the rule deprecate itself in the case of assignments would solve these issues?

[aemino]

@pq @bwilkerson What do you guys think about this?

[bwilkerson]

Perhaps simply having the rule deprecate itself in the case of assignments would solve these issues?

I'm not sure I understand what you're suggesting here. The rule is intended to catch (among other things) code in which the first statement is an assignment. Are you suggesting that we disable the rule for all code containing an assignment (and adjust the rule's description)? (I'm not necessarily opposed, just clarifying.)

I agree that it's a problem when the assignment causes an implicit cast or when the assignment needs to happen before the cascaded expressions are executed. We ought to be able to detect the first case, but I don't think we can reliably detect the second case.

@alexeieleusis (another issue to contemplate)

[aemino]

Actually, reading through these issues again, I would like to amend my previous comment. While I originally was indeed suggesting disabling the rule for assignments, I think that's a bit rash.

I think we should definitely disable the lint in the case of implicit casts. Whether we do so for assignments with ?? though, I'm still uncertain.

[matanlurey]

I no longer feel passionate about this issue :)