From e41d436fff30302af94236c7f0e1e827328983c7 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 23 Jun 2021 09:43:37 -0400 Subject: [PATCH 1/2] [flutter_plugin_tools] Migrate analyze to new base command Switches `analyze` to the new base command that handles the boilerplate of looping over target packages. This will change the output format slightly, but shoudn't have any functional change. Updates tests to use runCapturingPrint so that test run output isn't mixed with command output. Part of https://github.com/flutter/flutter/issues/83413 --- script/tool/lib/src/analyze_command.dart | 74 ++++++++++++---------- script/tool/test/analyze_command_test.dart | 19 +++--- 2 files changed, 52 insertions(+), 41 deletions(-) diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 003f0bcda82d..edc1cf23bc7f 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -8,11 +8,13 @@ import 'package:file/file.dart'; import 'package:path/path.dart' as p; import 'common/core.dart'; -import 'common/plugin_command.dart'; +import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; +const int _exitBadCustomAnalysisFile = 2; + /// A command to run Dart analysis on packages. -class AnalyzeCommand extends PluginCommand { +class AnalyzeCommand extends PackageLoopingCommand { /// Creates a analysis command instance. AnalyzeCommand( Directory packagesDir, { @@ -32,6 +34,8 @@ class AnalyzeCommand extends PluginCommand { static const String _analysisSdk = 'analysis-sdk'; + late String _dartBinaryPath; + @override final String name = 'analyze'; @@ -40,9 +44,10 @@ class AnalyzeCommand extends PluginCommand { 'This command requires "dart" and "flutter" to be in your path.'; @override - Future run() async { - print('Verifying analysis settings...'); + final bool hasLongOutput = false; + /// Checks that there are no unexpected analysis_options.yaml files. + void _validateAnalysisOptions() { final List files = packagesDir.listSync(recursive: true); for (final FileSystemEntity file in files) { if (file.basename != 'analysis_options.yaml' && @@ -59,18 +64,25 @@ class AnalyzeCommand extends PluginCommand { continue; } - print('Found an extra analysis_options.yaml in ${file.absolute.path}.'); - print( - 'If this was deliberate, pass the package to the analyze command with the --$_customAnalysisFlag flag and try again.'); - throw ToolExit(1); + printError( + 'Found an extra analysis_options.yaml in ${file.absolute.path}.'); + printError( + 'If this was deliberate, pass the package to the analyze command ' + 'with the --$_customAnalysisFlag flag and try again.'); + throw ToolExit(_exitBadCustomAnalysisFile); } + } + /// Ensures that the dependent packages have been fetched for all packages + /// that will be analyzed. + Future _fetchAllDependencies() async { final List packageDirectories = await getPackages().toList(); final Set packagePaths = packageDirectories.map((Directory dir) => dir.path).toSet(); packageDirectories.removeWhere((Directory directory) { - // We remove the 'example' subdirectories - 'flutter pub get' automatically - // runs 'pub get' there as part of handling the parent directory. + // Remove the 'example' subdirectories; 'flutter packages get' + // automatically runs 'pub get' there as part of handling the parent + // directory. return directory.basename == 'example' && packagePaths.contains(directory.parent.path); }); @@ -78,33 +90,29 @@ class AnalyzeCommand extends PluginCommand { await processRunner.runAndStream('flutter', ['packages', 'get'], workingDir: package, exitOnError: true); } + } + + @override + Future initializeRun() async { + print('Verifying analysis settings...'); + _validateAnalysisOptions(); + + print('Fetching dependencies...'); + await _fetchAllDependencies(); // Use the Dart SDK override if one was passed in. final String? dartSdk = argResults![_analysisSdk] as String?; - final String dartBinary = - dartSdk == null ? 'dart' : p.join(dartSdk, 'bin', 'dart'); - - final List failingPackages = []; - final List pluginDirectories = await getPlugins().toList(); - for (final Directory package in pluginDirectories) { - final int exitCode = await processRunner.runAndStream( - dartBinary, ['analyze', '--fatal-infos'], - workingDir: package); - if (exitCode != 0) { - failingPackages.add(p.basename(package.path)); - } - } - - print('\n\n'); + _dartBinaryPath = dartSdk == null ? 'dart' : p.join(dartSdk, 'bin', 'dart'); + } - if (failingPackages.isNotEmpty) { - print('The following packages have analyzer errors (see above):'); - for (final String package in failingPackages) { - print(' * $package'); - } - throw ToolExit(1); + @override + Future> runForPackage(Directory package) async { + final int exitCode = await processRunner.runAndStream( + _dartBinaryPath, ['analyze', '--fatal-infos'], + workingDir: package); + if (exitCode != 0) { + return PackageLoopingCommand.failure; } - - print('No analyzer errors found!'); + return PackageLoopingCommand.success; } } diff --git a/script/tool/test/analyze_command_test.dart b/script/tool/test/analyze_command_test.dart index 464aa1d91473..bdf9910f0b12 100644 --- a/script/tool/test/analyze_command_test.dart +++ b/script/tool/test/analyze_command_test.dart @@ -36,7 +36,7 @@ void main() { final MockProcess mockProcess = MockProcess(); mockProcess.exitCodeCompleter.complete(0); processRunner.processToReturn = mockProcess; - await runner.run(['analyze']); + await runCapturingPrint(runner, ['analyze']); expect( processRunner.recordedCalls, @@ -58,7 +58,7 @@ void main() { final MockProcess mockProcess = MockProcess(); mockProcess.exitCodeCompleter.complete(0); processRunner.processToReturn = mockProcess; - await runner.run(['analyze']); + await runCapturingPrint(runner, ['analyze']); expect( processRunner.recordedCalls, @@ -77,7 +77,7 @@ void main() { final MockProcess mockProcess = MockProcess(); mockProcess.exitCodeCompleter.complete(0); processRunner.processToReturn = mockProcess; - await runner.run(['analyze']); + await runCapturingPrint(runner, ['analyze']); expect( processRunner.recordedCalls, @@ -99,7 +99,8 @@ void main() { final MockProcess mockProcess = MockProcess(); mockProcess.exitCodeCompleter.complete(0); processRunner.processToReturn = mockProcess; - await runner.run(['analyze', '--analysis-sdk', 'foo/bar/baz']); + await runCapturingPrint( + runner, ['analyze', '--analysis-sdk', 'foo/bar/baz']); expect( processRunner.recordedCalls, @@ -123,7 +124,7 @@ void main() { createFakePlugin('foo', packagesDir, extraFiles: ['analysis_options.yaml']); - await expectLater(() => runner.run(['analyze']), + await expectLater(() => runCapturingPrint(runner, ['analyze']), throwsA(const TypeMatcher())); }); @@ -131,7 +132,7 @@ void main() { createFakePlugin('foo', packagesDir, extraFiles: ['.analysis_options']); - await expectLater(() => runner.run(['analyze']), + await expectLater(() => runCapturingPrint(runner, ['analyze']), throwsA(const TypeMatcher())); }); @@ -142,7 +143,8 @@ void main() { final MockProcess mockProcess = MockProcess(); mockProcess.exitCodeCompleter.complete(0); processRunner.processToReturn = mockProcess; - await runner.run(['analyze', '--custom-analysis', 'foo']); + await runCapturingPrint( + runner, ['analyze', '--custom-analysis', 'foo']); expect( processRunner.recordedCalls, @@ -164,7 +166,8 @@ void main() { processRunner.processToReturn = mockProcess; await expectLater( - () => runner.run(['analyze', '--custom-analysis', '']), + () => runCapturingPrint( + runner, ['analyze', '--custom-analysis', '']), throwsA(const TypeMatcher())); }); }); From d7321f7ca10f26d7b5066ebab3798bed0a340fdf Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Thu, 24 Jun 2021 14:25:53 -0400 Subject: [PATCH 2/2] Rename method --- script/tool/lib/src/analyze_command.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index edc1cf23bc7f..3f6a2444ad9b 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -74,8 +74,8 @@ class AnalyzeCommand extends PackageLoopingCommand { } /// Ensures that the dependent packages have been fetched for all packages - /// that will be analyzed. - Future _fetchAllDependencies() async { + /// (including their sub-packages) that will be analyzed. + Future _runPackagesGetOnTargetPackages() async { final List packageDirectories = await getPackages().toList(); final Set packagePaths = packageDirectories.map((Directory dir) => dir.path).toSet(); @@ -98,7 +98,7 @@ class AnalyzeCommand extends PackageLoopingCommand { _validateAnalysisOptions(); print('Fetching dependencies...'); - await _fetchAllDependencies(); + await _runPackagesGetOnTargetPackages(); // Use the Dart SDK override if one was passed in. final String? dartSdk = argResults![_analysisSdk] as String?;