Customers would like a lint to prevent the "async" modifier w/ @mustBeSynchronous annotation

[matanlurey]

AngularDart and ACX have lifecycle events that are added via implements:

abstract class OnDestroy {
  /// Implement to receive a notification when the component is being removed.
  ///
  /// This is an ideal time to cleanup streams, cancel pending RPCs, clear caches, etc.
  void ngOnDestroy();
}

A frequently observed problem is users wanting to use await, so they add async:

class MyComp implements OnDestroy {
  StreamSubscription<void> _windowResizeSub;

  @override
  void ngOnDestroy() async {
    await doSomethingAsync();
    // The user might continue to receive window.onResize events after the component has been
    // effectively destroyed because they used "await" above instead of just "doSomethingAsync()".
    _windowResizeSub.cancel();
  }
}

... and this has been exasperated by allowing async on void methods unfortunately.

We would like a metadata annotation discouraging any use of async / await on certain methods:

import 'package:meta/meta.dart';

abstract class OnDestroy {
  /// Implement to receive a notification when the component is being removed.
  ///
  /// This is an ideal time to cleanup streams, cancel pending RPCs, clear caches, etc.
  @mustBeSynchronous
  void ngOnDestroy();
}

class MyComp implements OnDestroy {
  StreamSubscription<void> _windowResizeSub;

  @override
  // LINT: "OnDestroy.ngOnDestroy should not use async/await"
  void ngOnDestroy() async {
    await doSomethingAsync();
  }
}

/cc @srawlins

[lrhn]

Note that async is an implemenation detail. You can write asynchronous code without it, using Future methods, so preventing async won't actually stop all incorrect uses. It's just easier to write asynchonrous code with async and await.

(Not saying this won't catch most problems, just that it won't catch all).

[bwilkerson]

Do we need an annotation for this? Are there cases where angular users really want to have an async body on a method that is marked as void? Perhaps a lint to disallow combining async and void would be sufficient (and not require a library author to remember to mark all void methods with the annotation).

[matanlurey]

@lrhn:

Note that async is an implemenation detail. You can write asynchronous code without it

True. We find that the majority of the time the misuse of these lifecycle methods is due to async/await though, not other asynchronous methods (like Future, Timer, scheduleMicrotask, etc). There is no way to prevent every bug with a lint, but we think this would prevent many.

@bwilkerson:

Do we need an annotation for this?

ACX does, because it is valid in the language to do any of the following:

abstract class HasVoidA {
  void a();
}

class A1 implements HasVoidA {
  @override
  void async a() {}
}

class A2 implements HasVoidA {
  @override
  async a() {}
}

class A3 implements HasVoidA {
  @override
  Future<void> async a() {}
}

Any of these (A1, A2, or A3) would/should be flagged if HasVoidA#a was annotated.

and not require a library author to remember to mark all void methods with the annotation.

In this case, it's just 5-6 methods, so it's really easy for us to remember and annotate correctly.

[natebosch]

Here's another place it might be useful for this annotation to apply:

void doStuff(@mustBeSynchronous void Function()) {
  ...
}

I suspect that explicitly marking these places is going to give us the best result. Without the annotation the tradeoff between false positive and false negatives is entirely up to how we identify potential issues, with the annotation it's a tool for framework authors who better understand each problem space.

[matanlurey]

I'm wondering what the appetite is on this hint, I'd be happy to implement it.

@zanderso and I ran into a place where it would have been preferable over a runtime failure: https://github.com/flutter/engine/pull/52769#discussion_r1599170519.

[bwilkerson]

It sounds like there's enough value to accept the addition of the annotation and the corresponding warnings.

[lrhn]

Could we somehow use avoid_void_async?

I guess that if one wants to use a void ... async function in other places, the lint won't be enabled (too many false positives for that), and if we want to enable the lint locally ... well, that would just be an annotation, on a void returning function position, which is basically what this is a request for anyway. So at most it could reuse code to check for the void/async combination.

[bwilkerson]

The advantage of an annotation is that users will receive a warning if they're misusing the API. It doesn't depend on the user of the API to have a lint defined in order to help prevent problems.