Need new Lint for improper use of cascade operator with methods returning new instances

[Kavinkumar-97]

Description

The cascade operator (..) is a powerful feature in Dart, but its misuse with methods that return new instances (instead of modifying the original object) can lead to subtle and hard-to-detect bugs. Currently, there is no lint to warn developers about such scenarios, which could improve code reliability and readability.

Problem Example

The following code uses the cascade operator in a way that appears valid but leads to unintended behavior:

final uri = Uri.parse('https://api.example.com/products')..replace(
    queryParameters: {
      'category': 'electronics',
      'sort': 'price_asc',
    }
  );

print(uri); // Incorrect: Prints https://api.example.com/products
print(uri.queryParameters); // Incorrect: Prints {}

In this example:

• Uri.replace creates a new Uri instance but does not modify the original object.
• The cascade operator (..) applies the replace method, but the result is discarded, leading to incorrect behavior.

Correct Example

To achieve the intended behavior, the method’s result must be explicitly assigned to the variable:

final uri = Uri.parse('https://api.example.com/products').replace(
    queryParameters: {
      'category': 'electronics',
      'sort': 'price_asc',
    }
);

print(uri); // Correct: Prints https://api.example.com/products?category=electronics&sort=price_asc
print(uri.queryParameters); // Correct: Prints {category: electronics, sort: price_asc}

---

Proposed Solution

Introduce a new lint that warns when:

1. The cascade operator (..) is used with methods that return a new instance (instead of modifying the original object).
2. The return value of such methods is not used, leading to potentially unintended behavior.

Expected Behavior

For the following code:

final uri = Uri.parse('https://api.example.com/products')..replace(
    queryParameters: {'category': 'electronics', 'sort': 'price_asc'}
);

The lint should produce a warning, such as:

"Avoid using the cascade operator with methods that return new instances. The result of replace is ignored."

---

Rationale

1. Developer Experience:

   • This lint would help developers catch unintended usage of the cascade operator during development.
   • It aligns with Dart's philosophy of developer-friendly tools and clarity.

2. Common Pitfalls:

   • Misuse of cascade operators with immutable objects is a common issue, especially for newcomers to Dart.

---

Challenges

• Complexity:

  • Detecting all methods that return new instances (versus those with side effects) might require additional metadata or analysis.
  • The lint should not flag valid uses of the cascade operator with side-effecting methods.

• Performance:

  • Ensuring this lint does not introduce significant performance overhead during static analysis.

---

Additional Notes

This lint could be particularly useful when working with:

• Immutable classes (e.g., Uri, String, DateTime, etc.).
• APIs that follow a builder or fluent pattern, where methods return modified instances instead of modifying the original.

---

Request

Please consider adding this lint to help improve code quality and reduce subtle bugs caused by improper use of the cascade operator. If this suggestion is deemed infeasible, a documentation update clarifying this behavior would also be valuable.

[lrhn]

The warning would trigger when you use a method using a cascade, and that method returns something of the "same type" as the original object, probably unless that type is dynamic.
That is, when you wouldn't get a different type if you had used ..

Maybe a method returning a subtype should also give warning. Doing var list = someNumList..whereType<int>().toList() is probably a mistake.
Maybe a non-trivial supertype too, like only differeing by generics. Doing var list = sometIntList..cast<num>(); is probably a mistake.

On the other hand, any void method returns a super-type, and returning being (or not being) Object? instead of List<int> is probably also something you'd notice if you wanted it. Some heurisitics might be needed.

It's not going to be perfect, the only real way to know that using a cascade is a mistake is to know that the method is not being called for its side effects, which isn't something you can see from the type signature.

[eernstg]

Linear types and related concepts like 'affine' types and 'relevant' types are all about using the result of a computation in a restricted manner: For a relevant type the value must be used at least once (that is, we can't just ignore and discard it), for a linear type it must be used exactly once (which enables something like ownership and transfer of ownership), and for an affine type it must be used at most once (so we know that there is no aliasing). All potentially useful stuff.

I'm not suggesting that a traditional formulation of these kinds of types would fit into Dart's type system (at all! ;-), but the underlying concepts might still be useful to have in mind.

One way to approach a relevant type is to annotate a given function/method declaration with metadata indicating that the returned value should not be discarded. We might even say that this is an 'anti-void' function (because 'relevant' is kind of bland).

We could then mark Uri.replace as anti-void such that every invocation where the returned value from this method is discarded would be flagged. A cascade is one of those situations, an expression statement is another, and the third element in a C-style for-loop part is a comma separated list of expressions whose values are discarded. So there are lots of places in a typical Dart program where returned results are discarded.

It could also be useful to mark a type-introducing declaration (a class/mixin/etc) as anti-void. In particular, Future is an obvious example where we might want every function/method that returns a future to be considered anti-void. (Who knows, we already have a lot of lint support in this area, so maybe it isn't useful.)

In any case, I tend to think about the topic of this issue as rather closely related to "usage control" along these lines.

[bwilkerson]

My intuition is that such a lint would have a high enough false positive rate that it wouldn't be accepted, but there might be ways to improve the situation. Heuristics might help, but it's difficult to know without enumerating them.

As Erik mentioned, an annotation like useResult could be used for this purpose, but it has the limitation that only the library/package owner can add it, and we can't put annotations from package:meta on the dart: libraries, so it won't help in the case of Uri.replace. That said, you could use it in your own code if you have similar methods whose value shouldn't be ignored.

[eernstg]

Haha! I really thought there was something like useResult but I couldn't find it. Thanks, @bwilkerson! ;-)

[srawlins]

We won't be implementing this as a warning or a lint rule in the Dart analyzer. We now have a high bar for implementing built-in static analysis rules, and this rule is too specific, and does not have enough impact. As of Dart 3.10 (Flutter 3.38), a new analyzer plugins system is available, and the requested static analysis here could be implemented as an analyzer plugin.