dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
628 stars 170 forks source link

proposal: `uncompleted_completer` #4855

Open brianquinlan opened 8 months ago

brianquinlan commented 8 months ago

uncompleted_completer

Description

One of Completer. complete() or Completer.completeError() should be called.

Details

One of Completer. complete() or Completer.completeError() must be called for Completer.future to complete.

Kind

Guards against errors.

Bad Examples

// This script will finish without error or output because `c.future` can never complete.
void main() async {
  final c = Completer;
  await c.future;  // Lint error on this line
  print('Hi');
}
void main() async {
  final c = Completer;
  if (<something>) {
    c.complete();
  }
  await c.future;  // Lint error on this line; not all code paths complete the future
  print('Hi');
}

Good Examples

void main() async {
  final c = Completer;
  functionCall(c);
  await c.future;  // No lint error, can't determine if `c` was completed or not
  print('Hi');
}
final c = Completer;
void main() async {
  functionCall();
  await c.future;  // No lint error, can't determine if `c` was completed or not
  print('Hi');
}

Discussion

The motivating example was https://github.com/dart-lang/sdk/issues/54735, which was caused by this code.

Note how, if the if condition does trigger, then completer will never be completed.

I've seen this bug in other contexts as well.

Discussion checklist

lrhn commented 8 months ago

This is similar to "definitely assigned" for local variables. If a completer is created in a local variable, doesn't escape, and isn't completed on every path to where the variable goes out of scope, then there is probably a problem.

However, if you use a completer to begin with, it's very likely because you don't have the result available synchronously. Otherwise just use Future.value or Future.error. That means that in a synchronous function, the completer will escape, at least by being captured by a closure, which makes the lint useless. So it only really works for async functions, where the variable's scope can go past an await.

In that case, the completer should either escape, or be completed, on every path before the completer going out of scope or awaiting the completer's future inside the same function (which will block completing the completer unless it has escaped).

The same rules can be used in sync functions, but they're unlikely to ever matter, because the completer will escape if it needs to exist at all.

pq commented 8 months ago

@stereotype441