Sometimes lint `discarded_futures` does not recognize `unawaited`.

[polina-c]

Dor example, the file
devtools_app/lib/src/screens/performance/performance_controller.dart
in the PR: flutter/devtools#4659

[lrhn]

The lint, as described by itself, does not make exceptions for unawaited. It asks you to make the surrounding function async because you are calling an async function. If you do that, then you can use unawaited to avoid the unawaited_futures lint, which only applies to async functions.

Maybe discarded_futures lint should make an exception for futures which are not discarded. That is, it could use the same rules as unawaited_futures, only in a non-async function, and only complain if the future is actually discarded. If the future is used in a way that does not create a new future, then forcing the surrounding function to be async won't actually do anything useful. It will just force callers of that function to do the same thing.

(It's not a lint I'd ever consider enabling myself, because there are plenty of reasons to make non-async functions calling async functions when you are writing library code which works with futures.
For application code, it's more likely that you will only be consuming futures created by other code.)

[oprypin]

Disregarding high-level considerations, I think there can't be any argument made that this lint is better by still complaining even after one adds unawaited. This is a false positive and should be fixed.

[lrhn]

If the lint is intended to disregard future-returning invocations inside an unawaited(...) invocation, then not disregarding is definitely a bug.

I'm saying that the description of the lint does not suggest that to be the case.

Looking at the implementation CL, and the tests, they do suggest that unawaited should be recognized, so the description should probably be updated or expanded.

(So yes, false positive, and description needs to be improved.)

It's also still an overly broad lint, IMO. Passing a future as an argument to a function which expects a future as argument, should probably be considered as "not discarded", not matter which function it is. (Like #3739). Or calling a method on the future, like .ignore().

I'd prefer if it only warned in the same places where unawaited_future would warn inside an async function, where a future is actually about to be discarded, but the idea seems to be to force most uses of futures to be inside async functions, even those that don't actually need to be awaited immediately. (And that's also a valid goal for a lint, just not one I'd enable.)

(@pq : The implementation seems a little eager to bail out. It stops recursing subexpression when it sees an invocation of unawaited, so if you have unwaited(foo(voidParameter: asyncFunction())), it won't recognize the future being assigned to void as discarded.)

[oprypin]

Regarding the implementation, I'm guessing the check for unawaited is written just so the lint doesn't complain about unawaited itself, seeing as it is in fact a function that returns Future.

[oprypin]

Actually never mind, I'm not sure what actually happens with the interaction with RecursiveAstVisitor

[lrhn]

Yup, the unawaited function returns void, so that's not an issue. (https://api.dart.dev/stable/2.14.1/dart-async/unawaited.html).

There is nothing magical about the unawaited function, the unawaited_futures lint doesn't need to recognize it, because that lint just looks for futures which are not processed in some way. Calling unawaited function counts as processing the argument, and since it returns void, there is nothing more to check for.

That's another reason this lint worries me, it has to recognize unawaited explicitly, and therefore won't work with other, equally valid, ways to process a future (like Future.ignore()).

[oprypin]

That's another reason this lint worries me, it has to recognize unawaited explicitly, and therefore won't work with other, equally valid, ways to process a future (like Future.ignore()).

• That's just dart-lang/sdk#46218
  -a simple annotation really should be made available

[pq]

@oprypin: I agree 100% and would love to see @awaitNotRequired or similar implemented. Maybe time to revisit dart-lang/sdk#46218?

[lrhn]

Such an annotation would be introduced by the analyzer and likely be in package:meta.

If we want to get the functionality for platform declarations, we can use a @pragma("analyzer:await_not_required") annotation.
(Heck, the declaration of awaitNotRequired could be just const awaitNotRequired = pragma('analyzer:await_not_required');, then everyone can use the same value as annotation.)