From cda2c223f5378fd922a7ebb7bdc7d7bc68bc3dca Mon Sep 17 00:00:00 2001 From: xster Date: Thu, 30 Aug 2018 20:57:44 -0700 Subject: [PATCH] Turn on unawaited_futures in flutter_tools (#21048) --- packages/analysis_options.yaml | 2 +- packages/flutter_tools/analysis_options.yaml | 4 ++ .../lib/src/android/android_workflow.dart | 6 ++- .../flutter_tools/lib/src/base/process.dart | 4 +- .../lib/src/commands/analyze_once.dart | 3 +- .../lib/src/commands/attach.dart | 2 +- .../lib/src/commands/fuchsia_reload.dart | 3 +- .../flutter_tools/lib/src/dart/analysis.dart | 3 +- .../lib/src/ios/code_signing.dart | 4 +- .../lib/src/resident_runner.dart | 2 +- packages/flutter_tools/lib/src/run_hot.dart | 37 ++++++++------- .../lib/src/tester/flutter_tester.dart | 3 +- .../test/commands/daemon_test.dart | 45 +++++++++---------- packages/flutter_tools/test/compile_test.dart | 11 +++-- packages/flutter_tools/test/device_test.dart | 12 ++--- .../flutter_tools/test/emulator_test.dart | 12 ++--- .../test/integration/test_driver.dart | 4 +- packages/flutter_tools/test/project_test.dart | 2 +- .../test/protocol_discovery_test.dart | 9 ++-- .../flutter_tools/tool/daemon_client.dart | 3 +- 20 files changed, 97 insertions(+), 74 deletions(-) diff --git a/packages/analysis_options.yaml b/packages/analysis_options.yaml index 217831d18fc..001e88fa3af 100644 --- a/packages/analysis_options.yaml +++ b/packages/analysis_options.yaml @@ -5,4 +5,4 @@ include: ../analysis_options.yaml linter: rules: - - public_member_api_docs + - public_member_api_docs diff --git a/packages/flutter_tools/analysis_options.yaml b/packages/flutter_tools/analysis_options.yaml index b8591ca6dba..c02a5f643ad 100644 --- a/packages/flutter_tools/analysis_options.yaml +++ b/packages/flutter_tools/analysis_options.yaml @@ -2,3 +2,7 @@ # the ones from above, which include the `public_member_api_docs` rule). include: ../../analysis_options.yaml + +linter: + rules: + - unawaited_futures diff --git a/packages/flutter_tools/lib/src/android/android_workflow.dart b/packages/flutter_tools/lib/src/android/android_workflow.dart index f0ad58f8a95..524dbd85bfb 100644 --- a/packages/flutter_tools/lib/src/android/android_workflow.dart +++ b/packages/flutter_tools/lib/src/android/android_workflow.dart @@ -236,7 +236,11 @@ class AndroidWorkflow extends DoctorValidator implements Workflow { environment: androidSdk.sdkManagerEnv, ); - process.stdin.addStream(stdin); + // The real stdin will never finish streaming. Pipe until the child process + // finishes. + process.stdin.addStream(stdin); // ignore: unawaited_futures + // Wait for stdout and stderr to be fully processed, because process.exitCode + // may complete first. await waitGroup(>[ stdout.addStream(process.stdout), stderr.addStream(process.stderr), diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index 2105aa53da8..247e5cef13c 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -191,7 +191,9 @@ Future runInteractively(List command, { allowReentrantFlutter: allowReentrantFlutter, environment: environment, ); - process.stdin.addStream(stdin); + // The real stdin will never finish streaming. Pipe until the child process + // finishes. + process.stdin.addStream(stdin); // ignore: unawaited_futures // Wait for stdout and stderr to be fully processed, because process.exitCode // may complete first. await Future.wait(>[ diff --git a/packages/flutter_tools/lib/src/commands/analyze_once.dart b/packages/flutter_tools/lib/src/commands/analyze_once.dart index 610c319fd37..06f20952539 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -92,7 +92,8 @@ class AnalyzeOnce extends AnalyzeBase { }); await server.start(); - server.onExit.then((int exitCode) { + // Completing the future in the callback can't fail. + server.onExit.then((int exitCode) { // ignore: unawaited_futures if (!analysisCompleter.isCompleted) { analysisCompleter.completeError('analysis server exited: $exitCode'); } diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index 66ea962d308..9131f356fe0 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -82,7 +82,7 @@ class AttachCommand extends FlutterCommand { @override Future validateCommand() async { - super.validateCommand(); + await super.validateCommand(); if (await findTargetDevice() == null) throwToolExit(null); observatoryPort; diff --git a/packages/flutter_tools/lib/src/commands/fuchsia_reload.dart b/packages/flutter_tools/lib/src/commands/fuchsia_reload.dart index 4650790550f..46419aaa480 100644 --- a/packages/flutter_tools/lib/src/commands/fuchsia_reload.dart +++ b/packages/flutter_tools/lib/src/commands/fuchsia_reload.dart @@ -444,7 +444,8 @@ class _PortForwarder { .transform(utf8.decoder) .transform(const LineSplitter()) .listen((String data) { printTrace(data); }); - process.exitCode.then((int c) { + // Best effort to print the exit code. + process.exitCode.then((int c) { // ignore: unawaited_futures printTrace("'${command.join(' ')}' exited with exit code $c"); }); printTrace('Set up forwarding from $localPort to $address:$remotePort'); diff --git a/packages/flutter_tools/lib/src/dart/analysis.dart b/packages/flutter_tools/lib/src/dart/analysis.dart index 8c07f098965..d4eabd74eee 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -45,8 +45,7 @@ class AnalysisServer { printTrace('dart ${command.skip(1).join(' ')}'); _process = await processManager.start(command); // This callback hookup can't throw. - _process.exitCode - .whenComplete(() => _process = null); // ignore: unawaited_futures + _process.exitCode.whenComplete(() => _process = null); // ignore: unawaited_futures final Stream errorStream = _process.stderr.transform(utf8.decoder).transform(const LineSplitter()); diff --git a/packages/flutter_tools/lib/src/ios/code_signing.dart b/packages/flutter_tools/lib/src/ios/code_signing.dart index a337a755294..a060eb165da 100644 --- a/packages/flutter_tools/lib/src/ios/code_signing.dart +++ b/packages/flutter_tools/lib/src/ios/code_signing.dart @@ -153,9 +153,7 @@ Future> getCodeSigningIdentityDevelopmentTeam({ ); final Process opensslProcess = await runCommand(const ['openssl', 'x509', '-subject']); - opensslProcess.stdin - ..write(signingCertificate) - ..close(); + await (opensslProcess.stdin..write(signingCertificate)).close(); final String opensslOutput = await utf8.decodeStream(opensslProcess.stdout); // Fire and forget discard of the stderr stream so we don't hold onto resources. diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index b5f93b35771..9ba993b1bfe 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -762,7 +762,7 @@ abstract class ResidentRunner { await handleTerminalCommand(command); } catch (error, st) { printError('$error\n$st'); - _cleanUpAndExit(null); + await _cleanUpAndExit(null); } finally { _processingUserRequest = false; } diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index b051dafaf6b..01c0e9c9c9e 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -590,21 +590,28 @@ class HotRunner extends ResidentRunner { pause: pause ); countExpectedReports += reports.length; - Future.wait(reports).catchError((dynamic error) { - return >[error]; - }).then((List> list) { - // TODO(aam): Investigate why we are validating only first reload report, - // which seems to be current behavior - final Map firstReport = list.first; - // Don't print errors because they will be printed further down when - // `validateReloadReport` is called again. - device.updateReloadStatus(validateReloadReport(firstReport, - printErrors: false)); - if (!retrieveFirstReloadReport.isCompleted) - retrieveFirstReloadReport.complete(firstReport); - }, onError: (dynamic error, StackTrace stack) { - retrieveFirstReloadReport.completeError(error, stack); - }); + await Future + .wait(reports) + .catchError((dynamic error) { + return >[error]; + }) + .then( + (List> list) { + // TODO(aam): Investigate why we are validating only first reload report, + // which seems to be current behavior + final Map firstReport = list.first; + // Don't print errors because they will be printed further down when + // `validateReloadReport` is called again. + device.updateReloadStatus( + validateReloadReport(firstReport, printErrors: false) + ); + if (!retrieveFirstReloadReport.isCompleted) + retrieveFirstReloadReport.complete(firstReport); + }, + onError: (dynamic error, StackTrace stack) { + retrieveFirstReloadReport.completeError(error, stack); + }, + ); } if (countExpectedReports == 0) { diff --git a/packages/flutter_tools/lib/src/tester/flutter_tester.dart b/packages/flutter_tools/lib/src/tester/flutter_tester.dart index 7404b5756b2..e6c2b24aabe 100644 --- a/packages/flutter_tools/lib/src/tester/flutter_tester.dart +++ b/packages/flutter_tools/lib/src/tester/flutter_tester.dart @@ -157,7 +157,8 @@ class FlutterTesterDevice extends Device { 'FLUTTER_TEST': 'true', }, ); - _process.exitCode.then((_) => _isRunning = false); + // Setting a bool can't fail in the callback. + _process.exitCode.then((_) => _isRunning = false); // ignore: unawaited_futures _process.stdout .transform(utf8.decoder) .transform(const LineSplitter()) diff --git a/packages/flutter_tools/test/commands/daemon_test.dart b/packages/flutter_tools/test/commands/daemon_test.dart index 8139187f6ba..f880e593081 100644 --- a/packages/flutter_tools/test/commands/daemon_test.dart +++ b/packages/flutter_tools/test/commands/daemon_test.dart @@ -43,8 +43,8 @@ void main() { expect(response['id'], 0); expect(response['result'], isNotEmpty); expect(response['result'] is String, true); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); testUsingContext('printError should send daemon.logMessage event', () async { @@ -64,8 +64,8 @@ void main() { final Map logMessage = response['params'].cast(); expect(logMessage['level'], 'error'); expect(logMessage['message'], 'daemon.logMessage test'); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }, overrides: { Logger: () => notifyingLogger, }); @@ -103,9 +103,8 @@ void main() { notifyingLogger: notifyingLogger ); commands.add({'id': 0, 'method': 'daemon.shutdown'}); - return daemon.onExit.then((int code) { - responses.close(); - commands.close(); + return daemon.onExit.then((int code) async { + await commands.close(); expect(code, 0); }); }); @@ -127,8 +126,8 @@ void main() { final Map response = await responses.stream.firstWhere(_notEvent); expect(response['id'], 0); expect(response['error'], contains('appId is required')); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); testUsingContext('ext.flutter.debugPaint via service extension without an appId should report an error', () async { @@ -154,8 +153,8 @@ void main() { final Map response = await responses.stream.firstWhere(_notEvent); expect(response['id'], 0); expect(response['error'], contains('appId is required')); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); testUsingContext('app.stop without appId should report an error', () async { @@ -175,8 +174,8 @@ void main() { final Map response = await responses.stream.firstWhere(_notEvent); expect(response['id'], 0); expect(response['error'], contains('appId is required')); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); testUsingContext('daemon should send showMessage on startup if no Android devices are available', () async { @@ -212,11 +211,11 @@ void main() { final Map response = await responses.stream.firstWhere(_notEvent); expect(response['id'], 0); expect(response['result'], isList); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); - testUsingContext('should send device.added event when device is discovered', () { + testUsingContext('should send device.added event when device is discovered', () async { final StreamController> commands = new StreamController>(); final StreamController> responses = new StreamController>(); daemon = new Daemon( @@ -229,15 +228,15 @@ void main() { daemon.deviceDomain.addDeviceDiscoverer(discoverer); discoverer.addDevice(new MockAndroidDevice()); - return responses.stream.skipWhile(_isConnectedEvent).first.then((Map response) { + return await responses.stream.skipWhile(_isConnectedEvent).first.then((Map response) async { expect(response['event'], 'device.added'); expect(response['params'], isMap); final Map params = response['params']; expect(params['platform'], isNotEmpty); // the mock device has a platform of 'android-arm' - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); }, overrides: { AndroidWorkflow: () => new MockAndroidWorkflow(), @@ -261,8 +260,8 @@ void main() { final Map response = await responses.stream.firstWhere(_notEvent); expect(response['id'], 0); expect(response['error'], contains('emulatorId is required')); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); testUsingContext('emulator.getEmulators should respond with list', () async { @@ -277,8 +276,8 @@ void main() { final Map response = await responses.stream.firstWhere(_notEvent); expect(response['id'], 0); expect(response['result'], isList); - responses.close(); - commands.close(); + await responses.close(); + await commands.close(); }); }); diff --git a/packages/flutter_tools/test/compile_test.dart b/packages/flutter_tools/test/compile_test.dart index 096c797a3e5..3ddb5be980a 100644 --- a/packages/flutter_tools/test/compile_test.dart +++ b/packages/flutter_tools/test/compile_test.dart @@ -276,7 +276,7 @@ void main() { 'result abc\nline1\nline2\nabc /path/to/main.dart.dill 0\n' ))); - generator.recompile( + await generator.recompile( '/path/to/main.dart', null /* invalidatedFiles */ ).then((CompilerOutput output) { expect(mockFrontendServerStdIn.getAndClear(), @@ -320,7 +320,8 @@ void main() { compileExpressionResponseCompleter2.future, ])); - generator.recompile( + // The test manages timing via completers. + generator.recompile( // ignore: unawaited_futures '/path/to/main.dart', null /* invalidatedFiles */ ).then((CompilerOutput outputCompile) { expect(logger.errorText, @@ -332,8 +333,9 @@ void main() { ))); }); + // The test manages timing via completers. final Completer lastExpressionCompleted = new Completer(); - generator.compileExpression('0+1', null, null, null, null, false).then( + generator.compileExpression('0+1', null, null, null, null, false).then( // ignore: unawaited_futures (CompilerOutput outputExpression) { expect(outputExpression, isNotNull); expect(outputExpression.outputFilename, @@ -344,7 +346,8 @@ void main() { ))); }); - generator.compileExpression('1+1', null, null, null, null, false).then( + // The test manages timing via completers. + generator.compileExpression('1+1', null, null, null, null, false).then( // ignore: unawaited_futures (CompilerOutput outputExpression) { expect(outputExpression, isNotNull); expect(outputExpression.outputFilename, diff --git a/packages/flutter_tools/test/device_test.dart b/packages/flutter_tools/test/device_test.dart index 32a06a565cf..94cf23cc031 100644 --- a/packages/flutter_tools/test/device_test.dart +++ b/packages/flutter_tools/test/device_test.dart @@ -28,12 +28,12 @@ void main() { Future expectDevice(String id, List expected) async { expect(await deviceManager.getDevicesById(id).toList(), expected); } - expectDevice('01abfc49119c410e', [device2]); - expectDevice('Nexus 5X', [device2]); - expectDevice('0553790d0a4e726f', [device1]); - expectDevice('Nexus 5', [device1]); - expectDevice('0553790', [device1]); - expectDevice('Nexus', [device1, device2]); + await expectDevice('01abfc49119c410e', [device2]); + await expectDevice('Nexus 5X', [device2]); + await expectDevice('0553790d0a4e726f', [device1]); + await expectDevice('Nexus 5', [device1]); + await expectDevice('0553790', [device1]); + await expectDevice('Nexus', [device1, device2]); }); }); } diff --git a/packages/flutter_tools/test/emulator_test.dart b/packages/flutter_tools/test/emulator_test.dart index ffdc7a72dbe..64f04e26512 100644 --- a/packages/flutter_tools/test/emulator_test.dart +++ b/packages/flutter_tools/test/emulator_test.dart @@ -62,12 +62,12 @@ void main() { expect(await testEmulatorManager.getEmulatorsMatching(id), expected); } - expectEmulator('Nexus_5', [emulator1]); - expectEmulator('Nexus_5X', [emulator2]); - expectEmulator('Nexus_5X_API_27_x86', [emulator2]); - expectEmulator('Nexus', [emulator1, emulator2]); - expectEmulator('iOS Simulator', [emulator3]); - expectEmulator('ios', [emulator3]); + await expectEmulator('Nexus_5', [emulator1]); + await expectEmulator('Nexus_5X', [emulator2]); + await expectEmulator('Nexus_5X_API_27_x86', [emulator2]); + await expectEmulator('Nexus', [emulator1, emulator2]); + await expectEmulator('iOS Simulator', [emulator3]); + await expectEmulator('ios', [emulator3]); }); testUsingContext('create emulator with an empty name does not fail', diff --git a/packages/flutter_tools/test/integration/test_driver.dart b/packages/flutter_tools/test/integration/test_driver.dart index 091df6453d0..892c275b451 100644 --- a/packages/flutter_tools/test/integration/test_driver.dart +++ b/packages/flutter_tools/test/integration/test_driver.dart @@ -89,7 +89,9 @@ class FlutterTestDriver { workingDirectory: _projectFolder.path, environment: {'FLUTTER_TEST': 'true'}); - _proc.exitCode.then((int code) { + // This class doesn't use the result of the future. It's made available + // via a getter for external uses. + _proc.exitCode.then((int code) { // ignore: unawaited_futures _debugPrint('Process exited ($code)'); _hasExited = true; }); diff --git a/packages/flutter_tools/test/project_test.dart b/packages/flutter_tools/test/project_test.dart index 3e3ce864667..dc19f23332f 100644 --- a/packages/flutter_tools/test/project_test.dart +++ b/packages/flutter_tools/test/project_test.dart @@ -97,7 +97,7 @@ void main() { testInMemory('exits on already materialized module', () async { final FlutterProject project = await aModuleProject(); await project.android.materialize(); - expectToolExitLater(project.android.materialize(), contains('already materialized')); + return expectToolExitLater(project.android.materialize(), contains('already materialized')); }); testInMemory('creates android/app folder in place of .android/app', () async { final FlutterProject project = await aModuleProject(); diff --git a/packages/flutter_tools/test/protocol_discovery_test.dart b/packages/flutter_tools/test/protocol_discovery_test.dart index c89e19ba02b..af80d01940a 100644 --- a/packages/flutter_tools/test/protocol_discovery_test.dart +++ b/packages/flutter_tools/test/protocol_discovery_test.dart @@ -1,3 +1,4 @@ + // Copyright 2016 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -134,7 +135,7 @@ void main() { expect(uri.port, 99); expect('$uri', 'http://127.0.0.1:99/PTwjm8Ii8qg=/'); - discoverer.cancel(); + await discoverer.cancel(); logReader.dispose(); }); @@ -153,7 +154,7 @@ void main() { expect(uri.port, 1243); expect('$uri', 'http://127.0.0.1:1243/PTwjm8Ii8qg=/'); - discoverer.cancel(); + await discoverer.cancel(); logReader.dispose(); }); @@ -172,7 +173,7 @@ void main() { expect(uri.port, 99); expect('$uri', 'http://127.0.0.1:99/PTwjm8Ii8qg=/'); - discoverer.cancel(); + await discoverer.cancel(); logReader.dispose(); }); @@ -192,7 +193,7 @@ void main() { expect(uri.port, 54777); expect('$uri', 'http://[::1]:54777/PTwjm8Ii8qg=/'); - discoverer.cancel(); + await discoverer.cancel(); logReader.dispose(); }); }); diff --git a/packages/flutter_tools/tool/daemon_client.dart b/packages/flutter_tools/tool/daemon_client.dart index 4700f8b27f5..10f48959ef3 100644 --- a/packages/flutter_tools/tool/daemon_client.dart +++ b/packages/flutter_tools/tool/daemon_client.dart @@ -81,7 +81,8 @@ Future main() async { stdout.write('> '); }); - daemon.exitCode.then((int code) { + // Print in the callback can't fail. + daemon.exitCode.then((int code) { // ignore: unawaited_futures print('daemon exiting ($code)'); exit(code); });