Enable unawaited_futures in flutter/packages
#127323
Comments
stuartmorgan-g
added
c: contributor-productivity
This option had been disabled to match flutter/flutter, but the reason it was disabled there was "too many false positives", mostly around animation. That doesn't apply to most packages here, and we've had a number of production bugs�especially in plugins, that use async heavily in ways that are intended to be client-awaitable�that this would have caught. This PR: - Enables the option at the repo level. - Permanently (unless the owners decide to change it) opts out `animations` and `go_router`, both of which looked like mostly or entirely false positives. - Temporarily opted out a few plugins that have a lot of violations that should be handled in their own PRs later (`camera_android_camerax`, most of `webview_flutter`). - Fixes all remaining violations. In many cases this PR is behavior-changing, replacing implicitly unawaited futures that did not seem obviously intentional with `await`ed futures, so non-test code in particular should be reviewed carefully to make sure the changes are correct. All of the changes are manual, not `fix`-generated. Part of flutter/flutter#127323
|
This is now on for most packages. The following had more changes than I wanted to put in the PR, so were temporarily opted out, but we should go through them and remove the temporary exclusion (the package-local
/cc @bparrishMines for webview_flutter and @camsim99 for camerax. |
@camsim99 I looked at doing this one myself, and most of the unawaited futures seemed questionable (i.e., looked like they should probably actually be awaited), but I'm not familiar enough with the logic to feel comfortable changing them. Could you and/or @gmackall take a look sometime soon? It should be relatively quick for someone who knows the logic, and I'd like to not leave exceptions indefinitely. |
Removes the exception, and enables the lint, fixing violations. Although this does make the tests more verbose, explicitly marking everything as unawaited is preferable to turning it off for the whole file since we have had bugs with not correctly awaiting important parts of tests in the past (such as completers). Part of flutter/flutter#127323
Fixes `unawaited_futures` violations in `camera_android_camerax` plugin. The only `await`s that I did not add are those related to closing/disposing native objects that shouldn't require us to wait for completion. Part of flutter/flutter#127323.
…futures violations (#4354) For flutter/flutter#127323
Removes the exception, and enables the lint, fixing violations. Although this does make the tests more verbose, explicitly marking everything as unawaited is preferable to turning it off for the whole file since we have had bugs with not correctly awaiting important parts of tests in the past (such as completers). Part of flutter/flutter#127323
…is (#4438) flutter/flutter#127323 has been closed and the packages no longer have the custom analysis files.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Currently
unawaited_futuresis disabled in flutter/packages to match flutter/flutter, where the comment is "too many false positives, especially with the way AnimationController works". Based on some preliminary investigation, I don't think this applies to flutter/packages in general; in most packages it seems like there are very few violations. We may want some packages to have custom rules (notably,animation) to suppress it, but we should have it on by default; async operations that should usually be awaited are a core aspect of plugins, and we've had a number of subtle bugs that the option would have prevented.The text was updated successfully, but these errors were encountered: