Analysis of `discarded_futures` and `unawaited_futures` does not work for tearoffs used as arguments and, in closures, is dependent on whether enclosing function is `async`

[limwa]

trafficstars

Issue description

As the title says the analysis of the discarded_futures and unawaited_futures does not work for tearoffs used as arguments. Futhermore, in closures, the reporting of issues is dependent on whether the enclosing function is async or not (instead of evaluating that for the closure itself). Finally, unawaited completely disables issue reporting for all futures used inside it, even if they are discarded in closures, which can lead to errors.

The following examples were analyzed using dart analyze, with the rules discarded_futures and unawaited_futures enabled.

I have created a gist here, which contains the execution logs of these examples, along with some print statements, to show that this incomplete analysis can lead to errors in real-world applications.

Example 1 (analysis does not work for tearoffs used as arguments + in closures, is dependent on whether enclosing function is async)

In this example, loadFromRemote is a function that returns a Future<void>. execute accepts a callback which is a function that returns a void. Therefore, by calling callback, the future returned by loadFromRemote is discarded, without the use of unawaited or ignore [unexpected].

import 'dart:async';

Future<void> loadFromRemote() async {
  await Future.delayed(const Duration(seconds: 1));
}

void execute(void Function() callback) {
  callback();
}

void load() {
  execute(loadFromRemote);
}

Notes:

1. If loadFromRemote is called inside a closure, with () => loadFromRemote(), the discarded_futures lint rule creates an issue [expected]. If, after this change, load is modified to be async, the issue disappears [unexpected].

Example 2 (in closures, is dependent on whether enclosing function is async)

In this example, there are two functions, tryLoadFromRemote and tryLoadFromCache that both return Future<void>. Then, there is a function, load, that also returns Future<void> and is async. tryLoadFromCache is used in load and is awaited, but tryLoadFromRemote, which is used inside a sync closure in then, is not awaited and, because of that, is discarded. No issue is reported on the tryLoadFromCache call [expected], but the tryLoadFromRemote call does not report any issues [unexpected].

import 'dart:async';

Future<void> tryLoadFromRemote() async {
  await Future.delayed(const Duration(seconds: 1));
}

Future<void> tryLoadFromCache() async {}

Future<void> load() async {
  await tryLoadFromCache().then((_) {
    tryLoadFromRemote();
  });
}

Notes:

1. If the signature of load is changed to void load() and the await keyword is removed, both calls report issues [expected]. If the code is wrapped in unawaited (see below), no issues are reported on the tryLoadFromCache call [expected] and on the tryLoadFromRemote call [unexpected].

   void load() {
     unawaited(tryLoadFromCache().then((_) {
       tryLoadFromRemote();
     }));
   }
   

General info

• Dart 3.5.3 (stable) (Wed Sep 11 16:22:47 2024 +0000) on "linux_x64"
• on linux / Linux 6.11.3-200.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Oct 10 22:31:19 UTC 2024
• locale is en_US.UTF-8

Process info

Memory	CPU	Elapsed time	Command line
331 MB	133.0%	00:02	dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.98.1
79 MB	22.0%	00:02	dart tooling-daemon --machine

[dart-github-bot]

Summary: The discarded_futures and unawaited_futures analysis rules do not work correctly for tearoffs used as arguments and in closures. The analysis is dependent on whether the enclosing function is async, rather than evaluating the closure itself. Additionally, unawaited disables issue reporting for all futures within it, even if they are discarded in closures, leading to potential errors.

[lrhn]

Should it work?

The discarded_futures lint says that it warns if an asynchronous function is called within a non-async function. Likely just if anything produces a Future inside a non-async function. There is no value of type Future in the load function of the first example. There is a function of type Future Function() which is cast to void Function(), which is a sign that a Future will likely be lost somewhere, but that's not something that making the load function async and adding an await can fix, so it's not really within the parameters of discarded_futures.

If load is made async, the same argument applies to unawaited_futures. This is not a problem that can be fixed by adding an await.

In the second example, the tryLoadFromRemote() call should trigger discarded_futures. It might be confused by being "inside an await". That does look like a bug.

[limwa]

There is a function of type Future Function() which is cast to void Function(), which is a sign that a Future will likely be lost somewhere, but that's not something that making the load function async and adding an await can fix, so it's not really within the parameters of discarded_futures.

If load is made async, the same argument applies to unawaited_futures. This is not a problem that can be fixed by adding an await.

Hey, thanks for your input! To me, it makes sense that there should be a lint issue. The correct fix I thought of for that example would be

void load() {
  execute(() => unawaited(loadFromRemote()));
}

It is a lot more verbose, but, this way, the future is explicitly discarded, instead of implicitly, which is what the discarded_futures lint rule more or less tries to avoid.

[limwa]

In the second example, the tryLoadFromRemote() call should trigger discarded_futures. It might be confused by being "inside an await". That does look like a bug.

From what I tested, it's more like it's dependent on the async keyword in the load method. I think it's because discarded_futures skips analysis for async functions.

For instance, if the keyword gets removed and the future is returned instead of awaited, the code is still valid and discarded_futures reports an issue in the tryLoadFromRemote call.