Skip to content

Commit

Permalink
[various] Fixes unawaited_futures violations (flutter#4067)
Browse files Browse the repository at this point in the history
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#127323
  • Loading branch information
stuartmorgan authored May 24, 2023
1 parent f19282a commit 97e3ba6
Show file tree
Hide file tree
Showing 78 changed files with 266 additions and 143 deletions.
2 changes: 1 addition & 1 deletion analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ linter:
- tighten_type_of_initializing_formals
# - type_annotate_public_apis # subset of always_specify_types
- type_init_formals
# - unawaited_futures # too many false positives, especially with the way AnimationController works
- unawaited_futures # DIFFERENT FROM FLUTTER/FLUTTER: It's disabled there for "too many false positives"; that's not an issue here, and missing awaits have caused production issues in plugins.
- unnecessary_await_in_return
- unnecessary_brace_in_string_interps
- unnecessary_const
Expand Down
10 changes: 10 additions & 0 deletions packages/animations/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# This custom rule set only exists to allow opting out of the repository
# default of enabling unawaited_futures. Please do NOT add more changes
# here without consulting with #hackers-ecosystem on Discord.

include: ../../analysis_options.yaml

linter:
rules:
# Matches flutter/flutter, which disables this rule due to false positives.
unawaited_futures: false
4 changes: 4 additions & 0 deletions packages/camera/camera/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.10.5+2

* Fixes unawaited_futures violations.

## 0.10.5+1

* Removes obsolete null checks on non-nullable values.
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera/lib/src/camera_controller.dart
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ class CameraController extends ValueNotifier<CameraValue> {
}

if (value.isStreamingImages) {
stopImageStream();
await stopImageStream();
}

try {
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description: A Flutter plugin for controlling the camera. Supports previewing
Dart.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.10.5+1
version: 0.10.5+2

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down
4 changes: 2 additions & 2 deletions packages/camera/camera/test/camera_image_stream_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ void main() {

await cameraController.initialize();

cameraController.startVideoRecording(
await cameraController.startVideoRecording(
onAvailable: (CameraImage image) => null);

expect(
Expand All @@ -192,7 +192,7 @@ void main() {

await cameraController.initialize();

cameraController.startVideoRecording();
await cameraController.startVideoRecording();

expect(mockPlatform.streamCallLog.contains('startVideoCapturing'), isTrue);
});
Expand Down
4 changes: 4 additions & 0 deletions packages/camera/camera_android/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## NEXT

* Fixes unawaited_futures violations.

## 0.10.8+2

* Removes obsolete null checks on non-nullable values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,12 @@ class CameraController extends ValueNotifier<CameraValue> {
enableAudio: enableAudio,
);

CameraPlatform.instance
unawaited(CameraPlatform.instance
.onCameraInitialized(_cameraId)
.first
.then((CameraInitializedEvent event) {
initializeCompleter.complete(event);
});
}));

await CameraPlatform.instance.initializeCamera(
_cameraId,
Expand Down Expand Up @@ -444,8 +444,8 @@ class CameraController extends ValueNotifier<CameraValue> {
if (_isDisposed) {
return;
}
_deviceOrientationSubscription?.cancel();
_isDisposed = true;
await _deviceOrientationSubscription?.cancel();
super.dispose();
if (_initCalled != null) {
await _initCalled;
Expand Down
4 changes: 2 additions & 2 deletions packages/camera/camera_android/test/android_camera_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ void main() {
isMethodCall('startImageStream', arguments: null),
]);

subscription.cancel();
await subscription.cancel();
});

test('Should stop streaming', () async {
Expand All @@ -1136,7 +1136,7 @@ void main() {
final StreamSubscription<CameraImageData> subscription = camera
.onStreamedFrameAvailable(cameraId)
.listen((CameraImageData imageData) {});
subscription.cancel();
await subscription.cancel();

// Assert
expect(channel.log, <Matcher>[
Expand Down
8 changes: 8 additions & 0 deletions packages/camera/camera_android_camerax/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# TODO(stuartmorgan): Remove this file and fix all the unawaited_futures
# violations. See https://github.com/flutter/flutter/issues/127323

include: ../../../analysis_options.yaml

linter:
rules:
unawaited_futures: false
4 changes: 4 additions & 0 deletions packages/camera/camera_avfoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## NEXT

* Fixes unawaited_futures violations.

## 0.9.13+2

* Removes obsolete null checks on non-nullable values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,12 @@ class CameraController extends ValueNotifier<CameraValue> {
enableAudio: enableAudio,
);

CameraPlatform.instance
unawaited(CameraPlatform.instance
.onCameraInitialized(_cameraId)
.first
.then((CameraInitializedEvent event) {
initializeCompleter.complete(event);
});
}));

await CameraPlatform.instance.initializeCamera(
_cameraId,
Expand Down Expand Up @@ -443,8 +443,8 @@ class CameraController extends ValueNotifier<CameraValue> {
if (_isDisposed) {
return;
}
_deviceOrientationSubscription?.cancel();
_isDisposed = true;
await _deviceOrientationSubscription?.cancel();
super.dispose();
if (_initCalled != null) {
await _initCalled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ void main() {
isMethodCall('startImageStream', arguments: null),
]);

subscription.cancel();
await subscription.cancel();
});

test('Should stop streaming', () async {
Expand All @@ -1137,7 +1137,7 @@ void main() {
final StreamSubscription<CameraImageData> subscription = camera
.onStreamedFrameAvailable(cameraId)
.listen((CameraImageData imageData) {});
subscription.cancel();
await subscription.cancel();

// Assert
expect(channel.log, <Matcher>[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,7 @@ void main() {
isMethodCall('startImageStream', arguments: null),
]);

subscription.cancel();
await subscription.cancel();
});

test('Should stop streaming', () async {
Expand All @@ -1102,7 +1102,7 @@ void main() {
final StreamSubscription<CameraImageData> subscription = camera
.onStreamedFrameAvailable(cameraId)
.listen((CameraImageData imageData) {});
subscription.cancel();
await subscription.cancel();

// Assert
expect(channel.log, <Matcher>[
Expand Down
3 changes: 2 additions & 1 deletion packages/camera/camera_windows/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 0.2.1+7

* Fixes unawaited_futures violations.
* Updates minimum supported SDK version to Flutter 3.3/Dart 2.18.

## 0.2.1+6
Expand Down
8 changes: 4 additions & 4 deletions packages/camera/camera_windows/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ class _MyAppState extends State<MyApp> {
enableAudio: _recordAudio,
);

_errorStreamSubscription?.cancel();
unawaited(_errorStreamSubscription?.cancel());
_errorStreamSubscription = CameraPlatform.instance
.onCameraError(cameraId)
.listen(_onCameraError);

_cameraClosingStreamSubscription?.cancel();
unawaited(_cameraClosingStreamSubscription?.cancel());
_cameraClosingStreamSubscription = CameraPlatform.instance
.onCameraClosing(cameraId)
.listen(_onCameraClosing);
Expand Down Expand Up @@ -193,7 +193,7 @@ class _MyAppState extends State<MyApp> {

Future<void> _recordTimed(int seconds) async {
if (_initialized && _cameraId > 0 && !_recordingTimed) {
CameraPlatform.instance
unawaited(CameraPlatform.instance
.onVideoRecordedEvent(_cameraId)
.first
.then((VideoRecordedEvent event) async {
Expand All @@ -204,7 +204,7 @@ class _MyAppState extends State<MyApp> {

_showInSnackBar('Video captured to: ${event.file.path}');
}
});
}));

await CameraPlatform.instance.startVideoRecording(
_cameraId,
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_windows/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: camera_windows
description: A Flutter plugin for getting information about and controlling the camera on Windows.
repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_windows
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.2.1+6
version: 0.2.1+7

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down
5 changes: 3 additions & 2 deletions packages/cross_file/tool/run_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//
// usage: dart run tool/run_tests.dart
// (needs a `chrome` executable in $PATH, or a tweak to dart_test.yaml)
import 'dart:async';
import 'dart:io';
import 'package:path/path.dart' as p;

Expand All @@ -30,8 +31,8 @@ Future<void> main(List<String> args) async {

Future<Process> _streamOutput(Future<Process> processFuture) async {
final Process process = await processFuture;
stdout.addStream(process.stdout);
stderr.addStream(process.stderr);
unawaited(stdout.addStream(process.stdout));
unawaited(stderr.addStream(process.stderr));
return process;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/flutter_image/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 4.1.6

* Fixes unawaited_futures violations.
* Updates minimum supported SDK version to Flutter 3.3/Dart 2.18.
* Aligns Dart and Flutter SDK constraints.

Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_image/lib/network.dart
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ class NetworkImageWithRetry extends ImageProvider<NetworkImageWithRetry> {
scale: key.scale,
);
} catch (error) {
request?.close();
await request?.close();
lastFailure = error is FetchFailure
? error
: FetchFailure._(
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_image/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: >
Image utilities for Flutter: improved network providers, effects, etc.
repository: https://github.com/flutter/packages/tree/main/packages/flutter_image
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+flutter_image%22
version: 4.1.5
version: 4.1.6

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_markdown/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT

* Fixes unawaited_futures violations.
* Updates minimum Flutter version to 3.3.
* Aligns Dart and Flutter SDK constraints.
* Replace `describeEnum` with the `name` getter.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';

import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_markdown/flutter_markdown.dart';
Expand Down Expand Up @@ -125,11 +127,11 @@ class _BasicMarkdownDemoState extends State<BasicMarkdownDemo> {
String? href,
String title,
) async {
showDialog<Widget>(
unawaited(showDialog<Widget>(
context: context,
builder: (BuildContext context) =>
_createDialog(context, text, href, title),
);
));
}

Widget _createDialog(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class DemoNotesView extends StatelessWidget {
String? href,
String title,
) async {
showDialog<Widget>(
await showDialog<Widget>(
context: context,
builder: (BuildContext context) =>
_createDialog(context, text, href, title),
Expand Down
6 changes: 6 additions & 0 deletions packages/go_router/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# This custom rule set only exists to allow very targeted changes
# relative to the default repo settings, for specific use cases.
# Please do NOT add more changes here without consulting with
# #hackers-ecosystem on Discord.

include: ../../analysis_options.yaml

analyzer:
exclude:
# This directory deliberately has errors, to test `fix`.
- "test_fixes/**"
12 changes: 12 additions & 0 deletions packages/go_router/test/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# This custom rule set only exists to allow very targeted changes
# relative to the default repo settings, for specific use cases.
# Please do NOT add more changes here without consulting with
# #hackers-ecosystem on Discord.

include: ../../../analysis_options.yaml

linter:
rules:
# Matches flutter/flutter, which disables this rule due to false positives
# from navigator APIs.
unawaited_futures: false
2 changes: 1 addition & 1 deletion packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// ignore_for_file: cascade_invocations, diagnostic_describe_all_properties
// ignore_for_file: cascade_invocations, diagnostic_describe_all_properties, unawaited_futures

import 'dart:async';

Expand Down
5 changes: 3 additions & 2 deletions packages/go_router/tool/run_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

// ignore_for_file: avoid_print

import 'dart:async';
import 'dart:io';
import 'package:io/io.dart' as io;
import 'package:path/path.dart' as p;
Expand Down Expand Up @@ -99,8 +100,8 @@ dependencies:

Future<Process> _streamOutput(Future<Process> processFuture) async {
final Process process = await processFuture;
stdout.addStream(process.stdout);
stderr.addStream(process.stderr);
unawaited(stdout.addStream(process.stdout));
unawaited(stderr.addStream(process.stderr));
return process;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/go_router_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 2.0.2

* Fixes unawaited_futures violations.
* Updates minimum supported SDK version to Flutter 3.3/Dart 2.18.

## 2.0.1
Expand Down
Loading

0 comments on commit 97e3ba6

Please sign in to comment.