Lint that warns about uses of cascade notation that are counter intuitive

[jacob314]

Combining dart cascade notation with normal method invocations results in expressions that don't evaluate how users expect. dartfmt helps a bit by enforcing indentation that clarifies the precedence a bit but results can still be confusing.

Examples of confused users:
https://stackoverflow.com/questions/17025769/how-do-method-cascades-work-exactly-in-dart

myList..clear().addAll(otherList)

gives an error that it cannot call addAll on null (if myList is dynamic).

A().b..c.d..runtimeType

A().b.c..foo()..bar(); // For discussion. Including all . before all .. might be ok or we could force parens in this case as well to fully disambiguate.

does not return runtimeType.

This is because cascade has an extremely low operator precedence which is surprising to users as . and .. seems similar but have very different precedence.
https://dart.dev/guides/language/language-tour#operators
The only operator that .. plays with consistently is = as it is given lower precedence than .

A lint and quickfix could help ease the way towards future Dart support for more intuitive precedence rules for cascades. The most aggressive version of the lint could force adding parenthesis for all cases where the expression order for cascades is required to get the current behavior.

Possible lint messages:
"If you end a cascade section with a getter you're going to have a bad time"
"Expression contains cascades without parenthesis to disambiguate the order of operations"

These lints would be used by any Dart user that uses cascades. We are seeing more Dart users use cascades as some Dart test frameworks now strongly benefit from using cascades but also result in more abuse of cascades.

An existing nice lint for cascades is
https://dart-lang.github.io/linter/lints/avoid_single_cascade_in_expression_statements.html

A quickfix could add parens so that the code no longer depends on the surprising order of operations for cascades.

Examples

A().b..c.d..runtimeType // Error

A().b..c.d..someMethod() // Error

((A().b..c).d)..someMethod() // No error as parens disambiguate

A()..b()..c()..d() // no error as calls do not mix . and ..

[jacob314]

Fyi @jakemac53 who mentioned the runtimeType example.

[bwilkerson]

@lrhn

[lrhn]

For precedence, I'd agree that something like a + b..foo or a ? b : c..foo should probably get a warning and be convertible to (a + b)..foo. It's misleading when read (see also dart-lang/language#608).

The dynamic case is probably unhelpable. There is nothing obviously wrong with foo..bar().baz(); (other than being a single section cascade), and if foo has type dynamic, you get what you asked for (no static help).

Whether to warn about a getter at the end of a cascade section is more heurisitic. I guess there can be situations where you do a get for a side effect (perhaps to trigger a lazy computation). Still, it's probably rare compared to just doing something unnecessary.
It's also usually a bad idea to call a factory or converted function as the last part of a cascade section, like foo..bar.baz.toList(). Those are harder to detect, since calling methods for side-effects is perfectly reasonable, so we'd need to have a list of "known non-side-effect functions".

The example

(A().(b..c).d)..someMethod() // No error as parens disambiguate

is invalid syntax. You might mean:

(A().b..c.d)..someMethod() // No error as parens disambiguate

which means the same thing as the unparenthesized expression.
I don't see any advantage though. If we warn about final getters in cascade sections, then that ..c.d is already a warning.
So, I'd warn about the final getter call, not the mixing of . and ... Mixing those is perfectly normal.
Consider:

foo?
 ..bar.update((x) => ...)
 ..baz.update((x) => ... );

That's completely fine and idiomatic code for updating two maps on the same object.

[jakemac53]

foo?
 ..bar.update((x) => ...)
 ..baz.update((x) => ... );

That's completely fine and idiomatic code for updating two maps on the same object.

Imo it's only fine because the formatting makes it obvious, consider this:

foo?..bar.update((x) => ...)..baz.update((x) => ... );

Now its pretty unclear what the receiver of the 2nd cascade is.

If everybody uses the formatter then I generally agree that we don't have an issue, the indentation makes the semantics clear.

[srawlins]

Duplicate of #57656