consider an annotation to tag async members as not needing await

[pq]

Follow-up from conversation in https://github.com/dart-lang/lints/issues/25.

The idea is to add an annotation that allows API authors to mark declarations as not requiring an await at call sites. This could greatly reduce the number of uses of functions like unawaited to silence the unawaited_futures lint.

[bwilkerson]

Do we want something fairly specific, like awaitNotRequired, or something more general, like doesNotThrow? (My understanding is that we care about un-awaited futures because of exception handling.) I'd guess the former, but thought it worth asking the question.

[pq]

Excellent question. My gut says we want the specificity of something like awaitNotRequired but would be very curious to get feedback from folks more familiar with APIs they'd like to see annotated.

/cc @davidmorgan @jefflim-google @goderbauer

[goderbauer]

In another thread where this was discussed (https://github.com/dart-lang/linter/issues/2513#issuecomment-801413249) Hixie suggested @cannotThrow to emphasize that it's only ok to not await Futures if they don't throw. I am also leaning a little more in this direction.

[bwilkerson]

Which leads to the next question: are there any static checks associated with cannotThrow (other than it can only be applied to functions, methods, getters, and fields)?

For example, I could imagine checking some or all of the following:

• Functions marked with cannotThrow can't contain either a throw or rethrow.
• Functions marked with cannotThrow can only invoke functions that are also annotated with it.
• Overrides of such methods are required to conform to the same contract (that is, it's inherited).

[davidmorgan]

I think order of execution is more of a concern with missing 'await' than exception handling? At least, in web and web test code that was the concern. I don't remember anything specifically about exception handling.

So what I was expecting is:

• Methods that have a side effect you likely want to wait for, should be either awaited or unawaited
• Async methods with a side effect you likely don't want to wait for, e.g. logging, could be annotated to silence the lint
• Async methods you never need to wait for can return void

and for that awaitNotRequired sounds good.

I'm not sure about the requirement to not throw; it seems to me like it's a valid design to have unawaited futures that throw, and rely on the zone exception handler to catch them?

[davidmorgan]

Also, it's quite possible that you have a method that can never throw that you always want to await, e.g.

@cannotThrow
Future<void> sleepABit() => await Future.delayed(Duration(seconds: 2));

So I don't think that works by itself.

[eernstg]

Just noted this issue, and I can't help mentioning a thing: A fire-and-forget async function can be expressed by using the return type void. Wouldn't that do the job?

void fireAndForget() async { ... }

void main() async {
  fireAndForget(); // No complaints about unawaited futures.
}

[pq]

Thanks @eernstg! I've wondered this too and it's exactly why I think some specific API examples would be handy.

[pq]

In https://github.com/dart-lang/lints/issues/25#issuecomment-853308392, @goderbauer mentioned

https://api.flutter.dev/flutter/animation/AnimationController/forward.html

In this and related cases a void return doesn't allow for the optional use of a returned TickerFuture.

[goderbauer]

These methods return Futures because sometimes you do want to await them (although that's rare), but in the most common cases you should be able to just drop them on the floor without having to litter your code with // ignores or unawaited.

[pq]

@goderbauer: I'm curious how you'd feel about adopting an @awaitNotRequired annotation in Flutter APIs. This doesn't preclude @cannotThrow but given @davidmorgan's comments it seems like it alone would only cover some of the bases.

[goderbauer]

@awaitNotRequired sounds good to me. Would be interesting to apply that to the framework methods we know and see how many more unawaited_future warnings remain.

[jacob314]

I would like to see @awaitNotRequred adopted by some dart:sdk apis as well.

For example: Map.remove called on a Map<String,Future> does not need to be awaited as the return value is rarely useful. Marking this method as @awaitNotRequired does seem odd as the Map class doesn't have anything specifically to do with Futures. Another option is to only trigger the unawaited_futures lint if the return value of a member is Future ignoring the generic type arguments.

[pq]

See also: https://github.com/dart-lang/linter/issues/2513.

[pq]

I would like to see @awaitNotRequred adopted by some dart:sdk apis as well.

/fyi @lrhn @leafpetersen @natebosch @jakemac53

If this is something we agree we want to do, it will suggest the annotation living in core (not package:meta).

EDIT: pragmatically, if there are only a few core library declarations we want to tag, we could bake that awareness into the lint and sidestep the challenge of landing a new annotation in the SDK. (Not tidy but there's certainly precedent. 😬 )

[pq]

As for @jacob314's example, I do wonder if that might not be better addressed in the lint. That one feels like a false positive to me.

[dnfield]

Another request that came up for this would be that if a Future returning function only contains @awaitNotRequired calls in it, it is also transitively @awaitNotRequired, e.g.

@awaitNotRequired
Future<void> doNotAwaitMe() async {
  print('ok');
}

Future<void> awaitMe() async {
  print('I sure hope you awaited this');
}

// Should be transitively @awaitNotRequired
Future<void> someFunction() async {
  await doNotAwaitMe(); // or don't await it, shouldn't matter.
  print('hi');
}

// Lint error to not await this function
Future<void> someOtherFunction() async {
  await awaitMe(); 
  print('hi');
}

[goderbauer]

@dnfield Do we have an actual use case for that in the framework?

[bwilkerson]

Another request that came up for this would be that if a Future returning function only contains @awaitNotRequired calls in it, it is also transitively @awaitNotRequired

I think that request means that such a function is treated as if it had been explicitly annotated without requiring an explicit annotation. If so, I'm not positive, but I suspect that that's something we can't efficiently implement.

(The only implementation I'm currently thinking of would be to create and incrementally maintain the call graph for all the code being analyzed, or at least for Future returning functions, in order to propagate requiredness through it. I think that doing so would be too inefficient to be practical.)

What we could do is add a lint to flag any such functions that aren't explicitly annotated as needing to be annotated, but such a lint might produce a lot of noise.

[dnfield]

@Hixie was suggesting this. My impression is that we'd want to avoid needing to go up the chain of every function in the framework that calls annotated functions and avoiding awaiting them.

As I think about it, I wonder why that wouldn't be a good thing - we'd explicitly make choices about whether we really meant for a specific function to be awaited or not. But maybe @Hixie has some example I'm not thinking of.

[natebosch]

Adding inference for this behavior would make it too fragile - local edits could have unpredictable consequences.

[lrhn]

A @cannotThrow function should not depend on only calling other @cannotThrow functions. Sometimes you just know that it won't throw, even though it can, like calling int.parse with something you already know contains only digits.

Likewise, a function only calling @awaitNotRequired functions is not itself necessarily one. It could do something else between the calls that you should wait for, or decide that the not-required-wait future should actually be awaited.

I don't particularly want lint-related annotations in the platform libraries. A @pragma("analyzer:ignore_await_futures") annotation isn't out-of-scope. I just don't want to make it part of a public API.

Ignoring methods which happen to return a future due to generics, but which aren't inherently asynchronous seems somewhat reasonable, but also risky. It is a future, and it's impossible to know whether someone waited for it or not. (Take: T id<T>(T value) => value; id(asyncOperation());. That future needs awaiting just as much as asyncOperation() itself. It's impossible to know whether a function propagates the future, or stores it somewhere - which is fine, an assignment isn't linted), Heuristics can easily be wrong, general rules are at least predictable.

[fzyzcjy]

Hi, is there any updates :)

[eernstg]

It was noted that even though we have support for fire-and-forget async functions (by giving them the return type void), we could also need support for "fire-and-forget-with-a-nontrivial-type", cf. this comment.

One possible approach for this could be to use a nullable return type:

// The return type `Future<...>?` could be used to indicate that "`await` is not required".
// We _can_ do this even in the case where the returned object will never actually be null,
// e.g., because the function body has the modifier `async`. It may seem odd to do so, but
// we would do it exactly because it's a signal to the lint, and it wouldn't be much of a problem
// that the call site has an artificial null-check if the return value is almost always ignored.
Future<int>? foo() async {...}

// This kind of return type is already usable in the case where we may or may not receive
// a `Future` at run time.
Future<int>? foo2() {
  return someCondition ? Future<int>.value(someExpression) : null;
}

An invocation of foo() or foo2() with no await in an async function is currently linted. This is a bit inconsistent, considering the treatment of FutureOr<...>, so we may or may not want to do that. I'm proposing that we treat them the same, and allow them to signal "await not required".

Note that foo2()?.then(...) allows for a fast path when foo2 returns null (we might then run more code synchronously). This means that the return type of foo2, and actually returning null in some cases, is useful in practice.

A very similar approach is to use the other union type, FutureOr. In this case it is already treated in such a way that await is not required:

// Use the return type `FutureOr<...>` to indicate that "`await` is not required".
FutureOr<int> foo3() {
  return someCondition ? Future<int>.value(someExpression) : 24;
}

// With `FutureOr`, it is almost always confusing to use it as the return type
// of an `async` function.
FutureOr<int> foo4() async {
  // The returned object is a `Future<int>` in any case.
  return someCondition ? Future<int>.value(someExpression) : 24;
}

Today, an invocation of foo3() with no await in an async function is not linted, that is, we're potentially ignoring a Future, and the lint doesn't flag the situation. This does mean that we can use the return type FutureOr<T> for an async function (or any function) that would otherwise return a Future<T>, and this will disable the lint unawaited_futures for each call site. It also has the nice property that the type of await foo3() has the type int, which means that the expression await foo3() has the best possible type (a plain int) even though the type of foo3() itself has (deliberately) been made less precise.

(So I might actually recommend use FutureOr, rather than 'use a nullable return type'. It just seems really misleading to say that foo4 might return an int. Well, maybe it isn't worse than claiming that foo might return null. ;-)

That said, I think we should lint the situation where a function has an async body and the return type is not of the form Future<T> for any T and not dynamic. This means that we would lint the declaration of a fire-and-forget function (with return type void) and functions like foo and foo4.

This means that the declaration of a fire-and-forget function would be linted, and that's probably a useful approach because they are expected to be rare anyway, and it is probably fine to ask for a special exception in order to declare one. Similarly, "fire-and-forget-with-nontrivial-type" functions like foo and foo4 would be linted because they are expected to be rare, and they need a comment. With that in place, all the call sites are silently allowed to ignore the returned future.

[jakemac53]

It is much better imo to handle this via an explicit annotation compared to lying about the return type as a workaround. Using a nullable type requires users to deal with the null that will never appear, and FutureOr similarly requires users to do a type check that is not necessary.

[bwilkerson]

I agree that an explicit annotation would be better. My expectation is that when a users sees an async method that returns void, they are unlikely to connect that to the concept that the method is fire-and-forget. I suspect that Future<T>? is even less likely to evoke that semantic in a user's mind. I think it would be much better to have an explicit annotation than to introduce an unfamiliar idiom. I also expect that the number of fire-and-forget functions is fairly small in practice, so I don't think that requiring an explicit annotation would be a significant burden on developers.

[Hixie]

FWIW the place where this comes up a lot in Flutter framework APIs is around animations. There are lots of APIs that return Future<void>s (and in some cases special Flutter-specific classes implementing Future<void>) that will never (by design) throw an exception, they will always either terminate or not terminate but that's it. Using await on these is almost always not what you want, but it sometimes is. So returning void or whatnot wouldn't work.

[dnfield]

Why wouldn't returning void work?

[eernstg]

@jakemac53 wrote:

It is much better imo to handle this via an explicit annotation compared to lying about the return type as a workaround

@bwilkerson wrote:

I suspect that Future<T>? is even less likely to evoke that semantic in a user's mind.

I mentioned here that all these functions (both fire-and-forget and fire-and-forget-with-nontrivial-type) should be linted at the declaration (because fire-and-forget-of-any-variant is expected to be rare). That could serve as a reminder to developers (of any such function) that this is an exceptional declaration, and its special nature should be documented.

It may still be safer to use an approach where each of the fire-and-forget-with-nontrivial-type functions has an @awaitNotRequired annotation, but the difference isn't huge.

At call sites we have nearly the opposite situation: If the property is signalled by the type of an expression then it will propagate in expressions, which is not the case if we rely on metadata.

import 'dart:async';

@awaitNotRequired
Future<int> fooMeta() async => 0;

// Fire-and-forget, should be treated as ignorable.
void bar() async {}

// Use the type to cancel the "should `await`" and "don't discard" lints. Ugly as it is, we use
// `FutureOr`, because it already works.
FutureOr<int> fooType() async => 0;

var someCondition = true;

X id<X>(X x) => x;

void main() async {
  fooMeta(); // No lint, as desired.
  (fooMeta()); // Lint.
  someCondition ? fooMeta() : fooMeta(); // Lint.
  id(fooMeta()); // Lint.

  bar(); // No lint, which is ok: fire-and-forget.
  id(bar()); // Still no lint, as desired.

  fooType(); // No lint, as desired.
  (fooType()); // Still no lint, in various expressions.
  someCondition ? fooType() : fooType();
  id(fooType());
}

[eernstg]

@Hixie wrote

the place where this comes up a lot in Flutter framework APIs ...

@dnfield wrote:

Why wouldn't returning void work?

Agreed, those examples sound very much like they could be declared as fire-and-forget functions, that is, with return type void (not Future<void>).

[Hixie]

If the return type is void, how do you get a hold of the Future if you do want to await it?

[dnfield]

You can still await a void returning function, but you won't be able to get the future object I guess

[eernstg]

Future<void> fut = myFireAndForget(...) as dynamic; is one way to override the protection of void results.

[Hixie]

Ok but that's ten times worse than just not enabling the lint that requires awaits, so, people just don't bother enabling the lint instead of doing that.

[eernstg]

Is this also true in the case where almost all invocations of myFireAndForget would actually ignore the returned future? I'd expect the forced access to the future to be very rare, if that isn't true then it's of course inconvenient.

[Hixie]

If the API returns a Future we should not document it as returning void, IMHO. It's surprising and unintuitive, and fights all our tools that are based on types.

[lrhn]

that's ten times worse

That's lowballing it! Please, for the love of applicable deity, do not await void return values, or cast them to anything. Ever. It's void for a reason!

I very much want to change the language to automatically throw away values assigned to the static type void, and prevent you from casting from void (or using a statically typed void value in any way), so that compilers can efficiently treat void functions as not returning anything. As it is now, they have to carefully thread the actual return value through any number of assignments to void. (It's a hard fight, because we have a lot of old crufty code which casts between dynamic and void with abandon, but it's something I do want!)

If you have something that someone sometimes want to await, but mostly don't, I think returning a future is fine, and annotating the function with something to make it not-a-warning to ignore the future is also fine. (Having two functions is also an option, with one just being unawaited of the other: void foo(...) => unawaited(fooWithFuture(...));. Or you can optimize the void function to not waste time on allocating a Future at all.)

Heck, we could allow annotation identifiers to refer to method declarations, not just const variable declarations, then you could do @unawaited.

[jacob314]

Implementing this annotation would have a large impact. One code base has over 10K calls to unawaited most of which could be eliminated by properly tagging framework APIs with awaitNotRequired. Even though the unawaited_futures lint has so many false positives, it is one of the most loved lints we have as people really appreciate the surprising bugs it catches. We get a lot of feature requests to have it handle more cases then it does today such as warning in methods that are not themselves async.

[pq]

Implementing this annotation would have a large impact.

💯

In addition to the existing uses of unawaited there are a host of future ones that could be avoided when folks start experimenting with discarded_futures.

[pq]

(Forward pointer / note to self: if/when we do land this, we should consider a diagnostic to help us identify unnecessary unawaiteds to help with cleanup. 🤔 )

[bsutton]

As per https://github.com/dart-lang/linter/issues/3651 my use case is in the package dcli which is a heavy user of waitfor wrapped in it's own method waitforex.

Users of the dcli library use waitforex which calls waitfor. Waitforex performs a stack repair operation so that when async exceptions are thrown they are presented to the caller as if the call stack occurred in a fully synchronous call tree.

As such waitforex needs to be marked as not needing to be unawaited whilst being allowed to throw.

So the explicit awaitNotRequired would be required in my use case.

Here is how waitForEx is used:

void main() {
   waitForEx(someAsyncFunction());
}

Here is the code:

T waitForEx<T>(Future<T> future) {
  final stackTrace = StackTraceImpl();
  late T value;
  try {
    value = cli.waitFor<T>(future);
  }
  // ignore: avoid_catching_errors
  on AsyncError catch (e) {
    Error.throwWithStackTrace(e.error, stackTrace.merge(e.stackTrace));
    // ignore: avoid_catches_without_on_clauses
  } catch (e, st) {
    Error.throwWithStackTrace(e, stackTrace.merge(st));
  }

  return value;
}

Edit: added code example.