From ee43de0476ce69964a2df8e8064b55b908c12dd5 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 13 Apr 2020 16:00:03 -0700 Subject: [PATCH] [flutter_tools] support enable-experiment in flutter analyze (#54613) --- .../flutter_tools/lib/src/base/terminal.dart | 2 +- .../lib/src/commands/analyze.dart | 3 + .../lib/src/commands/analyze_base.dart | 3 + .../src/commands/analyze_continuously.dart | 3 + .../lib/src/commands/analyze_once.dart | 3 + .../flutter_tools/lib/src/dart/analysis.dart | 26 ++++-- .../lib/src/runner/flutter_command.dart | 11 +++ .../hermetic/analyze_continuously_test.dart | 39 +++++++++ .../commands.shard/hermetic/analyze_test.dart | 80 ++++++++----------- .../permeable/analyze_once_test.dart | 26 ++++++ .../test/src/fake_process_manager.dart | 7 +- 11 files changed, 149 insertions(+), 54 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/terminal.dart b/packages/flutter_tools/lib/src/base/terminal.dart index 55bc9c7525..3332fe1bc9 100644 --- a/packages/flutter_tools/lib/src/base/terminal.dart +++ b/packages/flutter_tools/lib/src/base/terminal.dart @@ -168,7 +168,7 @@ class AnsiTerminal implements Terminal { static String colorCode(TerminalColor color) => _colorMap[color]; @override - bool get supportsColor => _platform.stdoutSupportsAnsi ?? false; + bool get supportsColor => _platform?.stdoutSupportsAnsi ?? false; // Assume unicode emojis are supported when not on Windows. // If we are on Windows, unicode emojis are supported in Windows Terminal, diff --git a/packages/flutter_tools/lib/src/commands/analyze.dart b/packages/flutter_tools/lib/src/commands/analyze.dart index d2690a5f54..286a620dce 100644 --- a/packages/flutter_tools/lib/src/commands/analyze.dart +++ b/packages/flutter_tools/lib/src/commands/analyze.dart @@ -30,6 +30,7 @@ class AnalyzeCommand extends FlutterCommand { _logger = logger, _terminal = terminal, _platform = platform { + addEnableExperimentation(verbose: verboseHelp); argParser.addFlag('flutter-repo', negatable: false, help: 'Include all the examples and tests from the Flutter repository.', @@ -118,6 +119,7 @@ class AnalyzeCommand extends FlutterCommand { platform: _platform, processManager: _processManager, terminal: _terminal, + experiments: stringsArg('enable-experiment'), ).analyze(); } else { await AnalyzeOnce( @@ -132,6 +134,7 @@ class AnalyzeCommand extends FlutterCommand { platform: _platform, processManager: _processManager, terminal: _terminal, + experiments: stringsArg('enable-experiment'), ).analyze(); } return FlutterCommandResult.success(); diff --git a/packages/flutter_tools/lib/src/commands/analyze_base.dart b/packages/flutter_tools/lib/src/commands/analyze_base.dart index 5f5249ae7a..52d333469d 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_base.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_base.dart @@ -28,6 +28,7 @@ abstract class AnalyzeBase { @required this.platform, @required this.processManager, @required this.terminal, + @required this.experiments, }); /// The parsed argument results for execution. @@ -46,6 +47,8 @@ abstract class AnalyzeBase { final Platform platform; @protected final AnsiTerminal terminal; + @protected + final List experiments; /// Called by [AnalyzeCommand] to start the analysis process. Future analyze(); diff --git a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart index 506b44c6cc..0b7d38720d 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart @@ -27,6 +27,7 @@ class AnalyzeContinuously extends AnalyzeBase { @required AnsiTerminal terminal, @required Platform platform, @required ProcessManager processManager, + @required List experiments, }) : super( argResults, repoPackages: repoPackages, @@ -36,6 +37,7 @@ class AnalyzeContinuously extends AnalyzeBase { platform: platform, terminal: terminal, processManager: processManager, + experiments: experiments, ); String analysisTarget; @@ -74,6 +76,7 @@ class AnalyzeContinuously extends AnalyzeBase { platform: platform, processManager: processManager, terminal: terminal, + experiments: experiments, ); server.onAnalyzing.listen((bool isAnalyzing) => _handleAnalysisStatus(server, isAnalyzing)); server.onErrors.listen(_handleAnalysisErrors); diff --git a/packages/flutter_tools/lib/src/commands/analyze_once.dart b/packages/flutter_tools/lib/src/commands/analyze_once.dart index a186edf103..8248013450 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -31,6 +31,7 @@ class AnalyzeOnce extends AnalyzeBase { @required Platform platform, @required ProcessManager processManager, @required AnsiTerminal terminal, + @required List experiments, this.workingDirectory, }) : super( argResults, @@ -41,6 +42,7 @@ class AnalyzeOnce extends AnalyzeBase { platform: platform, processManager: processManager, terminal: terminal, + experiments: experiments, ); /// The working directory for testing analysis using dartanalyzer. @@ -98,6 +100,7 @@ class AnalyzeOnce extends AnalyzeBase { logger: logger, processManager: processManager, terminal: terminal, + experiments: experiments, ); StreamSubscription subscription; diff --git a/packages/flutter_tools/lib/src/dart/analysis.dart b/packages/flutter_tools/lib/src/dart/analysis.dart index 8714c50ce7..83773e26d2 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -24,12 +24,14 @@ class AnalysisServer { @required ProcessManager processManager, @required Logger logger, @required Platform platform, - @required AnsiTerminal terminal, + @required Terminal terminal, + @required List experiments, }) : _fileSystem = fileSystem, _processManager = processManager, _logger = logger, _platform = platform, - _terminal = terminal; + _terminal = terminal, + _experiments = experiments; final String sdkPath; final List directories; @@ -37,7 +39,8 @@ class AnalysisServer { final ProcessManager _processManager; final Logger _logger; final Platform _platform; - final AnsiTerminal _terminal; + final Terminal _terminal; + final List _experiments; Process _process; final StreamController _analyzingController = @@ -49,11 +52,20 @@ class AnalysisServer { int _id = 0; Future start() async { - final String snapshot = - _fileSystem.path.join(sdkPath, 'bin/snapshots/analysis_server.dart.snapshot'); + final String snapshot = _fileSystem.path.join( + sdkPath, + 'bin', + 'snapshots', + 'analysis_server.dart.snapshot', + ); final List command = [ _fileSystem.path.join(sdkPath, 'bin', 'dart'), snapshot, + for (String experiment in _experiments) + ...[ + '--enable-experiment', + experiment, + ], '--disable-server-feature-completion', '--disable-server-feature-search', '--sdk', @@ -181,14 +193,14 @@ enum _AnalysisSeverity { class AnalysisError implements Comparable { AnalysisError(this.json, { @required Platform platform, - @required AnsiTerminal terminal, + @required Terminal terminal, @required FileSystem fileSystem, }) : _platform = platform, _terminal = terminal, _fileSystem = fileSystem; final Platform _platform; - final AnsiTerminal _terminal; + final Terminal _terminal; final FileSystem _fileSystem; static final Map _severityMap = { diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index f90d8efbfb..07eb6a4bcf 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -462,6 +462,17 @@ abstract class FlutterCommand extends Command { ); } + void addEnableExperimentation({ bool verbose }) { + argParser.addMultiOption( + FlutterOptions.kEnableExperiment, + help: + 'The name of an experimental Dart feature to enable. For more info ' + 'see: https://github.com/dart-lang/sdk/blob/master/docs/process/' + 'experimental-flags.md', + hide: !verbose, + ); + } + set defaultBuildMode(BuildMode value) { _defaultBuildMode = value; } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/analyze_continuously_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/analyze_continuously_test.dart index d8ee436c4f..bdb9f22cdc 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/analyze_continuously_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/analyze_continuously_test.dart @@ -4,6 +4,7 @@ import 'dart:async'; +import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; @@ -72,6 +73,7 @@ void main() { processManager: processManager, logger: logger, terminal: terminal, + experiments: [], ); int errorCount = 0; @@ -96,6 +98,7 @@ void main() { processManager: processManager, logger: logger, terminal: terminal, + experiments: [], ); int errorCount = 0; @@ -119,6 +122,7 @@ void main() { processManager: processManager, logger: logger, terminal: terminal, + experiments: [], ); int errorCount = 0; @@ -130,4 +134,39 @@ void main() { await onDone; expect(errorCount, 0); }); + + testWithoutContext('Can forward null-safety experiments to the AnalysisServer', () async { + final Completer completer = Completer(); + final StreamController> stdin = StreamController>(); + const String fakeSdkPath = 'dart-sdk'; + final FakeCommand fakeCommand = FakeCommand( + command: const [ + 'dart-sdk/bin/dart', + 'dart-sdk/bin/snapshots/analysis_server.dart.snapshot', + '--enable-experiment', + 'non-nullable', + '--disable-server-feature-completion', + '--disable-server-feature-search', + '--sdk', + 'dart-sdk', + ], + completer: completer, + stdin: IOSink(stdin.sink), + ); + + server = AnalysisServer(fakeSdkPath, [''], + fileSystem: MemoryFileSystem.test(), + platform: FakePlatform(), + processManager: FakeProcessManager.list([ + fakeCommand, + ]), + logger: BufferLogger.test(), + terminal: Terminal.test(), + experiments: [ + 'non-nullable' + ], + ); + + await server.start(); + }); } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart index 053a616e0e..e5b34174a1 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart @@ -11,54 +11,44 @@ import '../../src/common.dart'; const String _kFlutterRoot = '/data/flutter'; void main() { - FileSystem fs; - Directory tempDir; - - setUp(() { - fs = MemoryFileSystem(); - fs.directory(_kFlutterRoot).createSync(recursive: true); + testWithoutContext('analyze inRepo', () { + final FileSystem fileSystem = MemoryFileSystem.test(); + fileSystem.directory(_kFlutterRoot).createSync(recursive: true); + final Directory tempDir = fileSystem.systemTempDirectory + .createTempSync('flutter_analysis_test.'); Cache.flutterRoot = _kFlutterRoot; - tempDir = fs.systemTempDirectory.createTempSync('flutter_analysis_test.'); - }); - tearDown(() { - tryToDelete(tempDir); - }); + // Absolute paths + expect(inRepo([tempDir.path], fileSystem), isFalse); + expect(inRepo([fileSystem.path.join(tempDir.path, 'foo')], fileSystem), isFalse); + expect(inRepo([Cache.flutterRoot], fileSystem), isTrue); + expect(inRepo([fileSystem.path.join(Cache.flutterRoot, 'foo')], fileSystem), isTrue); - group('analyze', () { - testWithoutContext('inRepo', () { - bool inRepo(List fileList) { - if (fileList == null || fileList.isEmpty) { - fileList = [fs.path.current]; - } - final String root = fs.path.normalize(fs.path.absolute(Cache.flutterRoot)); - final String prefix = root + fs.path.separator; - for (String file in fileList) { - file = fs.path.normalize(fs.path.absolute(file)); - if (file == root || file.startsWith(prefix)) { - return true; - } - } - return false; - } + // Relative paths + fileSystem.currentDirectory = Cache.flutterRoot; + expect(inRepo(['.'], fileSystem), isTrue); + expect(inRepo(['foo'], fileSystem), isTrue); + fileSystem.currentDirectory = tempDir.path; + expect(inRepo(['.'], fileSystem), isFalse); + expect(inRepo(['foo'], fileSystem), isFalse); - // Absolute paths - expect(inRepo([tempDir.path]), isFalse); - expect(inRepo([fs.path.join(tempDir.path, 'foo')]), isFalse); - expect(inRepo([Cache.flutterRoot]), isTrue); - expect(inRepo([fs.path.join(Cache.flutterRoot, 'foo')]), isTrue); - - // Relative paths - fs.currentDirectory = Cache.flutterRoot; - expect(inRepo(['.']), isTrue); - expect(inRepo(['foo']), isTrue); - fs.currentDirectory = tempDir.path; - expect(inRepo(['.']), isFalse); - expect(inRepo(['foo']), isFalse); - - // Ensure no exceptions - inRepo(null); - inRepo([]); - }); + // Ensure no exceptions + inRepo(null, fileSystem); + inRepo([], fileSystem); }); } + +bool inRepo(List fileList, FileSystem fileSystem) { + if (fileList == null || fileList.isEmpty) { + fileList = [fileSystem.path.current]; + } + final String root = fileSystem.path.normalize(fileSystem.path.absolute(Cache.flutterRoot)); + final String prefix = root + fileSystem.path.separator; + for (String file in fileList) { + file = fileSystem.path.normalize(fileSystem.path.absolute(file)); + if (file == root || file.startsWith(prefix)) { + return true; + } + } + return false; +} diff --git a/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart b/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart index e596d7b0a1..376a8c2554 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/analyze_once_test.dart @@ -304,6 +304,32 @@ StringBuffer bar = StringBuffer('baz'); } }); + testUsingContext('analyze once supports analyzing null-safe code', () async { + const String contents = ''' +int? bar; +'''; + final Directory tempDir = fileSystem.systemTempDirectory.createTempSync('flutter_analyze_once_test_null_safety.'); + _createDotPackages(tempDir.path); + + tempDir.childFile('main.dart').writeAsStringSync(contents); + try { + await runCommand( + command: AnalyzeCommand( + workingDirectory: fileSystem.directory(tempDir), + platform: _kNoColorTerminalPlatform, + fileSystem: fileSystem, + logger: logger, + processManager: processManager, + terminal: terminal, + ), + arguments: ['analyze', '--no-pub', '--enable-experiment=non-nullable'], + statusTextContains: ['No issues found!'], + ); + } finally { + tryToDelete(tempDir); + } + }); + testUsingContext('analyze once returns no issues for todo comments', () async { const String contents = ''' // TODO(foobar): diff --git a/packages/flutter_tools/test/src/fake_process_manager.dart b/packages/flutter_tools/test/src/fake_process_manager.dart index a911c8ee68..1610eff231 100644 --- a/packages/flutter_tools/test/src/fake_process_manager.dart +++ b/packages/flutter_tools/test/src/fake_process_manager.dart @@ -28,6 +28,7 @@ class FakeCommand { this.stdout = '', this.stderr = '', this.completer, + this.stdin, }) : assert(command != null), assert(duration != null), assert(exitCode != null); @@ -82,6 +83,10 @@ class FakeCommand { /// resolves. final Completer completer; + /// An optional stdin sink that will be exposed through the resulting + /// [FakeProcess]. + final IOSink stdin; + void _matches(List command, String workingDirectory, Map environment) { expect(command, equals(this.command)); if (this.workingDirectory != null) { @@ -198,7 +203,7 @@ abstract class FakeProcessManager implements ProcessManager { fakeCommand.onRun, _pid, fakeCommand.stderr, - null, // stdin + fakeCommand.stdin, fakeCommand.stdout, fakeCommand.completer, );