From 09dec7f508d741cc11b69f30d128b4ddc82a018f Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 10 May 2018 09:48:40 -0700 Subject: [PATCH] re-write flutter analyze to use the analysis server (#16979) re-write flutter analyze (the single-shot and --flutter-repo) to use the analysis server --- CONTRIBUTING.md | 2 +- analysis_options.yaml | 7 +- dev/bots/analyze-sample-code.dart | 8 +- dev/devicelab/bin/tasks/dartdocs.dart | 25 +- packages/analysis_options.yaml | 8 + .../flutter/lib/analysis_options_user.yaml | 16 +- .../flutter_goldens/analysis_options.yaml | 4 + packages/flutter_tools/analysis_options.yaml | 4 + .../lib/src/commands/analyze.dart | 67 ++- .../src/commands/analyze_continuously.dart | 229 +--------- .../lib/src/commands/analyze_once.dart | 378 ++++++---------- .../flutter_tools/lib/src/dart/analysis.dart | 411 ++++++++---------- .../src/runner/flutter_command_runner.dart | 15 +- .../commands/analyze_continuously_test.dart | 2 +- .../analyze_duplicate_names_test.dart | 48 -- .../test/commands/analyze_once_test.dart | 60 +-- .../test/commands/create_test.dart | 7 +- 17 files changed, 460 insertions(+), 831 deletions(-) create mode 100644 packages/analysis_options.yaml create mode 100644 packages/flutter_goldens/analysis_options.yaml create mode 100644 packages/flutter_tools/analysis_options.yaml delete mode 100644 packages/flutter_tools/test/commands/analyze_duplicate_names_test.dart diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eca9cc5811d..80d2cce2fcb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,7 +79,7 @@ analyzer. There are two main ways to run it. In either case you will want to run `flutter update-packages` first, or you will get bogus error messages about core classes like Offset from `dart:ui`. -For a one-off, use `flutter analyze --flutter-repo`. This uses the `analysis_options_repo.yaml` file +For a one-off, use `flutter analyze --flutter-repo`. This uses the `analysis_options.yaml` file at the root of the repository for its configuration. For continuous analysis, use `flutter analyze --flutter-repo --watch`. This uses normal diff --git a/analysis_options.yaml b/analysis_options.yaml index 95dc52e86f6..14cebb5a69c 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -9,22 +9,18 @@ # # There are four similar analysis options files in the flutter repos: # - analysis_options.yaml (this file) -# - analysis_options_repo.yaml # - packages/flutter/lib/analysis_options_user.yaml # - https://github.com/flutter/plugins/blob/master/analysis_options.yaml +# - https://github.com/flutter/engine/blob/master/analysis_options.yaml # # This file contains the analysis options used by Flutter tools, such as IntelliJ, # Android Studio, and the `flutter analyze` command. -# It is very similar to the analysis_options_repo.yaml file in this same directory; -# the only difference (currently) is the public_member_api_docs option, -# which triggers too many messages to be used in editors. # # The flutter/plugins repo contains a copy of this file, which should be kept # in sync with this file. analyzer: language: - enableStrictCallChecks: true enableSuperMixins: true strong-mode: implicit-dynamic: false @@ -131,7 +127,6 @@ linter: - prefer_is_not_empty - prefer_single_quotes - prefer_typing_uninitialized_variables - # - public_member_api_docs # this is the only difference from analysis_options_repo.yaml - recursive_getters - slash_for_doc_comments - sort_constructors_first diff --git a/dev/bots/analyze-sample-code.dart b/dev/bots/analyze-sample-code.dart index a3dc4c1a9a4..5bf2bf5e121 100644 --- a/dev/bots/analyze-sample-code.dart +++ b/dev/bots/analyze-sample-code.dart @@ -189,6 +189,8 @@ Future main() async { } } buffer.add(''); + buffer.add('// ignore_for_file: unused_element'); + buffer.add(''); final List lines = new List.filled(buffer.length, null, growable: true); for (Section section in sections) { buffer.addAll(section.strings); @@ -207,7 +209,7 @@ dependencies: print('Found $sampleCodeSections sample code sections.'); final Process process = await Process.start( _flutter, - ['analyze', '--no-preamble', mainDart.path], + ['analyze', '--no-preamble', '--no-congratulate', mainDart.parent.path], workingDirectory: temp.path, ); stderr.addStream(process.stderr); @@ -216,10 +218,6 @@ dependencies: errors.removeAt(0); if (errors.first.startsWith('Running "flutter packages get" in ')) errors.removeAt(0); - if (errors.first.startsWith('Analyzing ')) - errors.removeAt(0); - if (errors.last.endsWith(' issues found.') || errors.last.endsWith(' issue found.')) - errors.removeLast(); int errorCount = 0; for (String error in errors) { final String kBullet = Platform.isWindows ? ' - ' : ' • '; diff --git a/dev/devicelab/bin/tasks/dartdocs.dart b/dev/devicelab/bin/tasks/dartdocs.dart index a2d083303bd..36ccf17ba70 100644 --- a/dev/devicelab/bin/tasks/dartdocs.dart +++ b/dev/devicelab/bin/tasks/dartdocs.dart @@ -21,21 +21,24 @@ Future main() async { int publicMembers = 0; int otherErrors = 0; int otherLines = 0; - await for (String entry in analysis.stderr.transform(utf8.decoder).transform(const LineSplitter())) { - print('analyzer stderr: $entry'); - if (entry.startsWith('[lint] Document all public members')) { - publicMembers += 1; - } else if (entry.startsWith('[')) { - otherErrors += 1; - } else if (entry.startsWith('(Ran in ')) { + await for (String entry in analysis.stdout.transform(utf8.decoder).transform(const LineSplitter())) { + entry = entry.trim(); + print('analyzer stdout: $entry'); + if (entry == 'Building flutter tool...') { // ignore this line - } else { + } else if (entry.startsWith('info • Document all public members •')) { + publicMembers += 1; + } else if (entry.startsWith('info •') || entry.startsWith('warning •') || entry.startsWith('error •')) { + otherErrors += 1; + } else if (entry.contains(' (ran in ')) { + // ignore this line + } else if (entry.isNotEmpty) { otherLines += 1; } } - await for (String entry in analysis.stdout.transform(utf8.decoder).transform(const LineSplitter())) { - print('analyzer stdout: $entry'); - if (entry == 'Building flutter tool...') { + await for (String entry in analysis.stderr.transform(utf8.decoder).transform(const LineSplitter())) { + print('analyzer stderr: $entry'); + if (entry.startsWith('[lint] ')) { // ignore this line } else { otherLines += 1; diff --git a/packages/analysis_options.yaml b/packages/analysis_options.yaml new file mode 100644 index 00000000000..217831d18fc --- /dev/null +++ b/packages/analysis_options.yaml @@ -0,0 +1,8 @@ +# Take our settings from the repo's main analysis_options.yaml file, but include +# an additional rule to validate that public members are documented. + +include: ../analysis_options.yaml + +linter: + rules: + - public_member_api_docs diff --git a/packages/flutter/lib/analysis_options_user.yaml b/packages/flutter/lib/analysis_options_user.yaml index 41cb657c9bb..504456df844 100644 --- a/packages/flutter/lib/analysis_options_user.yaml +++ b/packages/flutter/lib/analysis_options_user.yaml @@ -7,21 +7,21 @@ # See the configuration guide for more # https://github.com/dart-lang/sdk/tree/master/pkg/analyzer#configuring-the-analyzer # -# There are three similar analysis options files in the flutter repo: +# There are four similar analysis options files in the flutter repos: # - analysis_options.yaml -# - analysis_options_repo.yaml # - packages/flutter/lib/analysis_options_user.yaml (this file) +# - https://github.com/flutter/plugins/blob/master/analysis_options.yaml +# - https://github.com/flutter/engine/blob/master/analysis_options.yaml # -# This file contains the analysis options used by "flutter analyze" -# and the dartanalyzer when analyzing code outside the flutter repository. -# It isn't named 'analysis_options.yaml' because otherwise editors like Atom -# would use it when analyzing the flutter tool itself. +# This file contains the analysis options used by "flutter analyze" and the +# dartanalyzer when analyzing code outside the flutter repository. It isn't named +# 'analysis_options.yaml' because otherwise editors would use it when analyzing +# the flutter tool itself. # -# When editing, make sure you keep /analysis_options.yaml consistent. +# When editing, make sure you keep this and /analysis_options.yaml consistent. analyzer: language: - enableStrictCallChecks: true enableSuperMixins: true strong-mode: true errors: diff --git a/packages/flutter_goldens/analysis_options.yaml b/packages/flutter_goldens/analysis_options.yaml new file mode 100644 index 00000000000..b8591ca6dba --- /dev/null +++ b/packages/flutter_goldens/analysis_options.yaml @@ -0,0 +1,4 @@ +# Use the analysis options settings from the top level of the repo (not +# the ones from above, which include the `public_member_api_docs` rule). + +include: ../../analysis_options.yaml diff --git a/packages/flutter_tools/analysis_options.yaml b/packages/flutter_tools/analysis_options.yaml new file mode 100644 index 00000000000..b8591ca6dba --- /dev/null +++ b/packages/flutter_tools/analysis_options.yaml @@ -0,0 +1,4 @@ +# Use the analysis options settings from the top level of the repo (not +# the ones from above, which include the `public_member_api_docs` rule). + +include: ../../analysis_options.yaml diff --git a/packages/flutter_tools/lib/src/commands/analyze.dart b/packages/flutter_tools/lib/src/commands/analyze.dart index 0f4969e9b5e..4968cbf402a 100644 --- a/packages/flutter_tools/lib/src/commands/analyze.dart +++ b/packages/flutter_tools/lib/src/commands/analyze.dart @@ -9,28 +9,51 @@ import '../runner/flutter_command.dart'; import 'analyze_continuously.dart'; import 'analyze_once.dart'; -bool isDartFile(FileSystemEntity entry) => entry is File && entry.path.endsWith('.dart'); - -typedef bool FileFilter(FileSystemEntity entity); - class AnalyzeCommand extends FlutterCommand { - AnalyzeCommand({ bool verboseHelp: false, this.workingDirectory }) { - argParser.addFlag('flutter-repo', help: 'Include all the examples and tests from the Flutter repository.', defaultsTo: false); - argParser.addFlag('current-package', help: 'Include the lib/main.dart file from the current directory, if any.', defaultsTo: true); - argParser.addFlag('dartdocs', help: 'List every public member that is lacking documentation (only works with --flutter-repo and without --watch).', defaultsTo: false, hide: !verboseHelp); - argParser.addFlag('watch', help: 'Run analysis continuously, watching the filesystem for changes.', negatable: false); - argParser.addFlag('preview-dart-2', defaultsTo: true, help: 'Preview Dart 2.0 functionality.'); - argParser.addOption('write', valueHelp: 'file', help: 'Also output the results to a file. This is useful with --watch if you want a file to always contain the latest results.'); - argParser.addOption('dart-sdk', valueHelp: 'path-to-sdk', help: 'The path to the Dart SDK.', hide: !verboseHelp); + AnalyzeCommand({bool verboseHelp: false, this.workingDirectory}) { + argParser.addFlag('flutter-repo', + negatable: false, + help: 'Include all the examples and tests from the Flutter repository.', + defaultsTo: false, + hide: !verboseHelp); + argParser.addFlag('current-package', + help: 'Analyze the current project, if applicable.', defaultsTo: true); + argParser.addFlag('dartdocs', + negatable: false, + help: 'List every public member that is lacking documentation ' + '(only works with --flutter-repo).', + hide: !verboseHelp); + argParser.addFlag('watch', + help: 'Run analysis continuously, watching the filesystem for changes.', + negatable: false); + argParser.addFlag('preview-dart-2', + defaultsTo: true, help: 'Preview Dart 2.0 functionality.'); + argParser.addOption('write', + valueHelp: 'file', + help: 'Also output the results to a file. This is useful with --watch ' + 'if you want a file to always contain the latest results.'); + argParser.addOption('dart-sdk', + valueHelp: 'path-to-sdk', + help: 'The path to the Dart SDK.', + hide: !verboseHelp); // Hidden option to enable a benchmarking mode. - argParser.addFlag('benchmark', negatable: false, hide: !verboseHelp, help: 'Also output the analysis time.'); + argParser.addFlag('benchmark', + negatable: false, + hide: !verboseHelp, + help: 'Also output the analysis time.'); usesPubOption(); // Not used by analyze --watch - argParser.addFlag('congratulate', help: 'When analyzing the flutter repository, show output even when there are no errors, warnings, hints, or lints.', defaultsTo: true); - argParser.addFlag('preamble', help: 'When analyzing the flutter repository, display the number of files that will be analyzed.', defaultsTo: true); + argParser.addFlag('congratulate', + help: 'When analyzing the flutter repository, show output even when ' + 'there are no errors, warnings, hints, or lints.', + defaultsTo: true); + argParser.addFlag('preamble', + defaultsTo: true, + help: 'When analyzing the flutter repository, display the number of ' + 'files that will be analyzed.'); } /// The working directory for testing analysis using dartanalyzer. @@ -40,17 +63,19 @@ class AnalyzeCommand extends FlutterCommand { String get name => 'analyze'; @override - String get description => 'Analyze the project\'s Dart code.'; + String get description => "Analyze the project's Dart code."; @override bool get shouldRunPub { // If they're not analyzing the current project. - if (!argResults['current-package']) + if (!argResults['current-package']) { return false; + } // Or we're not in a project directory. - if (!fs.file('pubspec.yaml').existsSync()) + if (!fs.file('pubspec.yaml').existsSync()) { return false; + } return super.shouldRunPub; } @@ -59,11 +84,15 @@ class AnalyzeCommand extends FlutterCommand { Future runCommand() { if (argResults['watch']) { return new AnalyzeContinuously( - argResults, runner.getRepoPackages(), previewDart2: argResults['preview-dart-2'] + argResults, + runner.getRepoRoots(), + runner.getRepoPackages(), + previewDart2: argResults['preview-dart-2'], ).analyze(); } else { return new AnalyzeOnce( argResults, + runner.getRepoRoots(), runner.getRepoPackages(), workingDirectory: workingDirectory, previewDart2: argResults['preview-dart-2'], diff --git a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart index a9bd2dcb656..9d8376525fe 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:convert'; import 'package:args/args.dart'; @@ -11,17 +10,20 @@ import '../base/common.dart'; import '../base/file_system.dart'; import '../base/io.dart'; import '../base/logger.dart'; -import '../base/process_manager.dart'; import '../base/terminal.dart'; import '../base/utils.dart'; import '../cache.dart'; +import '../dart/analysis.dart'; import '../dart/sdk.dart' as sdk; import '../globals.dart'; import 'analyze_base.dart'; class AnalyzeContinuously extends AnalyzeBase { - AnalyzeContinuously(ArgResults argResults, this.repoPackages, { this.previewDart2: false }) : super(argResults); + AnalyzeContinuously(ArgResults argResults, this.repoRoots, this.repoPackages, { + this.previewDart2: false, + }) : super(argResults); + final List repoRoots; final List repoPackages; final bool previewDart2; @@ -43,11 +45,14 @@ class AnalyzeContinuously extends AnalyzeBase { if (argResults['flutter-repo']) { final PackageDependencyTracker dependencies = new PackageDependencyTracker(); dependencies.checkForConflictingDependencies(repoPackages, dependencies); - directories = repoPackages.map((Directory dir) => dir.path).toList(); + + directories = repoRoots; analysisTarget = 'Flutter repository'; + printTrace('Analyzing Flutter repository:'); - for (String projectPath in directories) + for (String projectPath in repoRoots) { printTrace(' ${fs.path.relative(projectPath)}'); + } } else { directories = [fs.currentDirectory.path]; analysisTarget = fs.currentDirectory.path; @@ -107,10 +112,14 @@ class AnalyzeContinuously extends AnalyzeBase { // Print an analysis summary. String errorsMessage; - final int issueCount = errors.length; + int issueCount = errors.length; final int issueDiff = issueCount - lastErrorCount; lastErrorCount = issueCount; + final int undocumentedCount = errors.where((AnalysisError issue) { + return issue.code == 'public_member_api_docs'; + }).length; + if (firstAnalysis) errorsMessage = '$issueCount ${pluralize('issue', issueCount)} found'; else if (issueDiff > 0) @@ -124,10 +133,13 @@ class AnalyzeContinuously extends AnalyzeBase { final String files = '${analyzedPaths.length} ${pluralize('file', analyzedPaths.length)}'; final String seconds = (analysisTimer.elapsedMilliseconds / 1000.0).toStringAsFixed(2); - printStatus('$errorsMessage • analyzed $files, $seconds seconds'); + printStatus('$errorsMessage • analyzed $files in $seconds seconds'); if (firstAnalysis && isBenchmarking) { - writeBenchmark(analysisTimer, issueCount, -1); // TODO(ianh): track members missing dartdocs instead of saying -1 + // We don't want to return a failing exit code based on missing documentation. + issueCount -= undocumentedCount; + + writeBenchmark(analysisTimer, issueCount, undocumentedCount); server.dispose().whenComplete(() { exit(issueCount > 0 ? 1 : 0); }); } @@ -135,209 +147,10 @@ class AnalyzeContinuously extends AnalyzeBase { } } - bool _filterError(AnalysisError error) { - // TODO(devoncarew): Also filter the regex items from `analyzeOnce()`. - - if (error.type == 'TODO') - return true; - - return false; - } - void _handleAnalysisErrors(FileAnalysisErrors fileErrors) { - fileErrors.errors.removeWhere(_filterError); + fileErrors.errors.removeWhere((AnalysisError error) => error.type == 'TODO'); analyzedPaths.add(fileErrors.file); analysisErrors[fileErrors.file] = fileErrors.errors; } } - -class AnalysisServer { - AnalysisServer(this.sdkPath, this.directories, { this.previewDart2: false }); - - final String sdkPath; - final List directories; - final bool previewDart2; - - Process _process; - final StreamController _analyzingController = new StreamController.broadcast(); - final StreamController _errorsController = new StreamController.broadcast(); - - int _id = 0; - - Future start() async { - final String snapshot = fs.path.join(sdkPath, 'bin/snapshots/analysis_server.dart.snapshot'); - final List command = [ - fs.path.join(sdkPath, 'bin', 'dart'), - snapshot, - '--sdk', - sdkPath, - ]; - - if (previewDart2) { - command.add('--preview-dart-2'); - } else { - command.add('--no-preview-dart-2'); - } - - 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 - - final Stream errorStream = _process.stderr.transform(utf8.decoder).transform(const LineSplitter()); - errorStream.listen(printError); - - final Stream inStream = _process.stdout.transform(utf8.decoder).transform(const LineSplitter()); - inStream.listen(_handleServerResponse); - - // Available options (many of these are obsolete): - // enableAsync, enableDeferredLoading, enableEnums, enableNullAwareOperators, - // enableSuperMixins, generateDart2jsHints, generateHints, generateLints - _sendCommand('analysis.updateOptions', { - 'options': { - 'enableSuperMixins': true - } - }); - - _sendCommand('server.setSubscriptions', { - 'subscriptions': ['STATUS'] - }); - - _sendCommand('analysis.setAnalysisRoots', { - 'included': directories, - 'excluded': [] - }); - } - - Stream get onAnalyzing => _analyzingController.stream; - Stream get onErrors => _errorsController.stream; - - Future get onExit => _process.exitCode; - - void _sendCommand(String method, Map params) { - final String message = json.encode( { - 'id': (++_id).toString(), - 'method': method, - 'params': params - }); - _process.stdin.writeln(message); - printTrace('==> $message'); - } - - void _handleServerResponse(String line) { - printTrace('<== $line'); - - final dynamic response = json.decode(line); - - if (response is Map) { - if (response['event'] != null) { - final String event = response['event']; - final dynamic params = response['params']; - - if (params is Map) { - if (event == 'server.status') - _handleStatus(response['params']); - else if (event == 'analysis.errors') - _handleAnalysisIssues(response['params']); - else if (event == 'server.error') - _handleServerError(response['params']); - } - } else if (response['error'] != null) { - // Fields are 'code', 'message', and 'stackTrace'. - final Map error = response['error']; - printError('Error response from the server: ${error['code']} ${error['message']}'); - if (error['stackTrace'] != null) - printError(error['stackTrace']); - } - } - } - - void _handleStatus(Map statusInfo) { - // {"event":"server.status","params":{"analysis":{"isAnalyzing":true}}} - if (statusInfo['analysis'] != null && !_analyzingController.isClosed) { - final bool isAnalyzing = statusInfo['analysis']['isAnalyzing']; - _analyzingController.add(isAnalyzing); - } - } - - void _handleServerError(Map error) { - // Fields are 'isFatal', 'message', and 'stackTrace'. - printError('Error from the analysis server: ${error['message']}'); - if (error['stackTrace'] != null) - printError(error['stackTrace']); - } - - void _handleAnalysisIssues(Map issueInfo) { - // {"event":"analysis.errors","params":{"file":"/Users/.../lib/main.dart","errors":[]}} - final String file = issueInfo['file']; - final List errors = issueInfo['errors'].map((Map json) => new AnalysisError(json)).toList(); - if (!_errorsController.isClosed) - _errorsController.add(new FileAnalysisErrors(file, errors)); - } - - Future dispose() async { - await _analyzingController.close(); - await _errorsController.close(); - return _process?.kill(); - } -} - -class AnalysisError implements Comparable { - AnalysisError(this.json); - - static final Map _severityMap = { - 'ERROR': 3, - 'WARNING': 2, - 'INFO': 1 - }; - - // "severity":"INFO","type":"TODO","location":{ - // "file":"/Users/.../lib/test.dart","offset":362,"length":72,"startLine":15,"startColumn":4 - // },"message":"...","hasFix":false} - Map json; - - String get severity => json['severity']; - int get severityLevel => _severityMap[severity] ?? 0; - String get type => json['type']; - String get message => json['message']; - String get code => json['code']; - - String get file => json['location']['file']; - int get startLine => json['location']['startLine']; - int get startColumn => json['location']['startColumn']; - int get offset => json['location']['offset']; - - @override - int compareTo(AnalysisError other) { - // Sort in order of file path, error location, severity, and message. - if (file != other.file) - return file.compareTo(other.file); - - if (offset != other.offset) - return offset - other.offset; - - final int diff = other.severityLevel - severityLevel; - if (diff != 0) - return diff; - - return message.compareTo(other.message); - } - - @override - String toString() { - final String relativePath = fs.path.relative(file); - return '${severity.toLowerCase().padLeft(7)} • $message • $relativePath:$startLine:$startColumn'; - } - - String toLegacyString() { - return '[${severity.toLowerCase()}] $message ($file:$startLine:$startColumn)'; - } -} - -class FileAnalysisErrors { - FileAnalysisErrors(this.file, this.errors); - - final String file; - final List errors; -} diff --git a/packages/flutter_tools/lib/src/commands/analyze_once.dart b/packages/flutter_tools/lib/src/commands/analyze_once.dart index fa58a59903a..82b71eef90a 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -3,13 +3,12 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:collection'; import 'package:args/args.dart'; import '../base/common.dart'; import '../base/file_system.dart'; -import '../base/process.dart'; +import '../base/logger.dart'; import '../base/utils.dart'; import '../cache.dart'; import '../dart/analysis.dart'; @@ -18,281 +17,178 @@ import '../globals.dart'; import 'analyze.dart'; import 'analyze_base.dart'; -bool isDartFile(FileSystemEntity entry) => entry is File && entry.path.endsWith('.dart'); - -typedef bool FileFilter(FileSystemEntity entity); - /// An aspect of the [AnalyzeCommand] to perform once time analysis. class AnalyzeOnce extends AnalyzeBase { - AnalyzeOnce(ArgResults argResults, this.repoPackages, { + AnalyzeOnce( + ArgResults argResults, + this.repoRoots, + this.repoPackages, { this.workingDirectory, this.previewDart2: false, }) : super(argResults); + final List repoRoots; final List repoPackages; - /// The working directory for testing analysis using dartanalyzer + /// The working directory for testing analysis using dartanalyzer. final Directory workingDirectory; final bool previewDart2; @override Future analyze() async { - final Stopwatch stopwatch = new Stopwatch()..start(); - final Set pubSpecDirectories = new HashSet(); - final List dartFiles = []; - for (String file in argResults.rest.toList()) { - file = fs.path.normalize(fs.path.absolute(file)); - final String root = fs.path.rootPrefix(file); - dartFiles.add(fs.file(file)); - while (file != root) { - file = fs.path.dirname(file); - if (fs.isFileSync(fs.path.join(file, 'pubspec.yaml'))) { - pubSpecDirectories.add(fs.directory(file)); - break; + final String currentDirectory = + (workingDirectory ?? fs.currentDirectory).path; + + // find directories from argResults.rest + final Set directories = new Set.from(argResults.rest + .map((String path) => fs.path.canonicalize(path))); + if (directories.isNotEmpty) { + for (String directory in directories) { + final FileSystemEntityType type = fs.typeSync(directory); + + if (type == FileSystemEntityType.notFound) { + throwToolExit("'$directory' does not exist"); + } else if (type != FileSystemEntityType.directory) { + throwToolExit("'$directory' is not a directory"); } } } - final bool currentPackage = argResults['current-package'] && (argResults.wasParsed('current-package') || dartFiles.isEmpty); - final bool flutterRepo = argResults['flutter-repo'] || (workingDirectory == null && inRepo(argResults.rest)); + if (argResults['flutter-repo']) { + // check for conflicting dependencies + final PackageDependencyTracker dependencies = + new PackageDependencyTracker(); + dependencies.checkForConflictingDependencies(repoPackages, dependencies); - // Use dartanalyzer directly except when analyzing the Flutter repository. - // Analyzing the repository requires a more complex report than dartanalyzer - // currently supports (e.g. missing member dartdoc summary). - // TODO(danrubel): enhance dartanalyzer to provide this type of summary - if (!flutterRepo) { - if (argResults['dartdocs']) - throwToolExit('The --dartdocs option is currently only supported with --flutter-repo.'); + directories.addAll(repoRoots); - final List arguments = []; - arguments.addAll(dartFiles.map((FileSystemEntity f) => f.path)); - - if (arguments.isEmpty || currentPackage) { - // workingDirectory is non-null only when testing flutter analyze - final Directory currentDirectory = workingDirectory ?? fs.currentDirectory.absolute; - final Directory projectDirectory = await projectDirectoryContaining(currentDirectory); - if (projectDirectory != null) { - arguments.add(projectDirectory.path); - } else if (arguments.isEmpty) { - arguments.add(currentDirectory.path); - } + if (argResults.wasParsed('current-package') && + argResults['current-package']) { + directories.add(currentDirectory); } - - // If the files being analyzed are outside of the current directory hierarchy - // then dartanalyzer does not yet know how to find the ".packages" file. - // TODO(danrubel): fix dartanalyzer to find the .packages file - final File packagesFile = await packagesFileFor(arguments); - if (packagesFile != null) { - arguments.insert(0, '--packages'); - arguments.insert(1, packagesFile.path); + } else { + if (argResults['current-package']) { + directories.add(currentDirectory); } - - if (previewDart2) { - arguments.add('--preview-dart-2'); - } else { - arguments.add('--no-preview-dart-2'); - } - - final String sdkPath = argResults['dart-sdk'] ?? sdk.dartSdkPath; - - final String dartanalyzer = fs.path.join(sdkPath, 'bin', 'dartanalyzer'); - arguments.insert(0, dartanalyzer); - bool noErrors = false; - final Set issues = new Set(); - int exitCode = await runCommandAndStreamOutput( - arguments, - workingDirectory: workingDirectory?.path, - mapFunction: (String line) { - // De-duplicate the dartanalyzer command output (https://github.com/dart-lang/sdk/issues/25697). - if (line.startsWith(' ')) { - if (!issues.add(line.trim())) - return null; - } - - // Workaround for the fact that dartanalyzer does not exit with a non-zero exit code - // when errors are found. - // TODO(danrubel): Fix dartanalyzer to return non-zero exit code - if (line == 'No issues found!') - noErrors = true; - - // Remove text about the issue count ('2 hints found.'); with the duplicates - // above, the printed count would be incorrect. - if (line.endsWith(' found.')) - return null; - - return line; - }, - ); - stopwatch.stop(); - if (issues.isNotEmpty) - printStatus('${issues.length} ${pluralize('issue', issues.length)} found.'); - final String elapsed = (stopwatch.elapsedMilliseconds / 1000.0).toStringAsFixed(1); - // Workaround for the fact that dartanalyzer does not exit with a non-zero exit code - // when errors are found. - // TODO(danrubel): Fix dartanalyzer to return non-zero exit code - if (exitCode == 0 && !noErrors) - exitCode = 1; - if (exitCode != 0) - throwToolExit('(Ran in ${elapsed}s)', exitCode: exitCode); - printStatus('Ran in ${elapsed}s'); - return; } - for (Directory dir in repoPackages) { - _collectDartFiles(dir, dartFiles); - pubSpecDirectories.add(dir); + if (argResults['dartdocs'] && !argResults['flutter-repo']) { + throwToolExit( + 'The --dartdocs option is currently only supported with --flutter-repo.'); } - // determine what all the various .packages files depend on - final PackageDependencyTracker dependencies = new PackageDependencyTracker(); - dependencies.checkForConflictingDependencies(pubSpecDirectories, dependencies); - final Map packages = dependencies.asPackageMap(); + if (directories.isEmpty) { + throwToolExit('Nothing to analyze.', exitCode: 0); + } + + // analyze all + final Completer analysisCompleter = new Completer(); + final List errors = []; + + final String sdkPath = argResults['dart-sdk'] ?? sdk.dartSdkPath; + + final AnalysisServer server = new AnalysisServer( + sdkPath, + directories.toList(), + previewDart2: previewDart2, + ); + + StreamSubscription subscription; + subscription = server.onAnalyzing.listen((bool isAnalyzing) { + if (!isAnalyzing) { + analysisCompleter.complete(); + subscription?.cancel(); + subscription = null; + } + }); + server.onErrors.listen((FileAnalysisErrors fileErrors) { + fileErrors.errors + .removeWhere((AnalysisError error) => error.type == 'TODO'); + errors.addAll(fileErrors.errors); + }); + + await server.start(); + server.onExit.then((int exitCode) { + if (!analysisCompleter.isCompleted) { + analysisCompleter.completeError('analysis server exited: $exitCode'); + } + }); Cache.releaseLockEarly(); - if (argResults['preamble']) { - if (dartFiles.length == 1) { - logger.printStatus('Analyzing ${fs.path.relative(dartFiles.first.path)}...'); + // collect results + final Stopwatch timer = new Stopwatch()..start(); + final String message = directories.length > 1 + ? '${directories.length} ${directories.length == 1 ? 'directory' : 'directories'}' + : fs.path.basename(directories.first); + final Status progress = argResults['preamble'] + ? logger.startProgress('Analyzing $message...') + : null; + + await analysisCompleter.future; + progress?.cancel(); + timer.stop(); + + // report dartdocs + int undocumentedMembers = 0; + + if (argResults['flutter-repo']) { + undocumentedMembers = errors.where((AnalysisError error) { + return error.code == 'public_member_api_docs'; + }).length; + + if (!argResults['dartdocs']) { + errors.removeWhere( + (AnalysisError error) => error.code == 'public_member_api_docs'); + } + } + + // emit benchmarks + if (isBenchmarking) { + writeBenchmark(timer, errors.length, undocumentedMembers); + } + + // report results + dumpErrors( + errors.map((AnalysisError error) => error.toLegacyString())); + + if (errors.isNotEmpty && argResults['preamble']) { + printStatus(''); + } + errors.sort(); + for (AnalysisError error in errors) { + printStatus(error.toString()); + } + + final String seconds = + (timer.elapsedMilliseconds / 1000.0).toStringAsFixed(1); + + // We consider any level of error to be an error exit (we don't report different levels). + if (errors.isNotEmpty) { + printStatus(''); + + printStatus( + '${errors.length} ${pluralize('issue', errors.length)} found. (ran in ${seconds}s)'); + + if (undocumentedMembers > 0) { + throwToolExit('[lint] $undocumentedMembers public ' + '${ undocumentedMembers == 1 + ? "member lacks" + : "members lack" } documentation'); } else { - logger.printStatus('Analyzing ${dartFiles.length} files...'); + throwToolExit(null); } } - final DriverOptions options = new DriverOptions(); - options.dartSdkPath = argResults['dart-sdk']; - options.packageMap = packages; - options.analysisOptionsFile = fs.path.join(Cache.flutterRoot, 'analysis_options_repo.yaml'); - final AnalysisDriver analyzer = new AnalysisDriver(options); - // TODO(pq): consider error handling - final List errors = analyzer.analyze(dartFiles); - - int errorCount = 0; - int membersMissingDocumentation = 0; - for (AnalysisErrorDescription error in errors) { - bool shouldIgnore = false; - if (error.errorCode.name == 'public_member_api_docs') { - // https://github.com/dart-lang/linter/issues/208 - if (isFlutterLibrary(error.source.fullName)) { - if (!argResults['dartdocs']) { - membersMissingDocumentation += 1; - shouldIgnore = true; - } - } else { - shouldIgnore = true; - } - } - if (shouldIgnore) - continue; - printError(error.asString()); - errorCount += 1; - } - dumpErrors(errors.map((AnalysisErrorDescription error) => error.asString())); - - stopwatch.stop(); - final String elapsed = (stopwatch.elapsedMilliseconds / 1000.0).toStringAsFixed(1); - - if (isBenchmarking) - writeBenchmark(stopwatch, errorCount, membersMissingDocumentation); - - if (errorCount > 0) { - // we consider any level of error to be an error exit (we don't report different levels) - if (membersMissingDocumentation > 0) - throwToolExit('[lint] $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation (ran in ${elapsed}s)'); - else - throwToolExit('(Ran in ${elapsed}s)'); - } if (argResults['congratulate']) { - if (membersMissingDocumentation > 0) { - printStatus('No analyzer warnings! (ran in ${elapsed}s; $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation)'); + if (undocumentedMembers > 0) { + printStatus('No issues found! (ran in ${seconds}s; ' + '$undocumentedMembers public ${ undocumentedMembers == + 1 ? "member lacks" : "members lack" } documentation)'); } else { - printStatus('No analyzer warnings! (ran in ${elapsed}s)'); + printStatus('No issues found! (ran in ${seconds}s)'); } } } - - /// Return a path to the ".packages" file for use by dartanalyzer when analyzing the specified files. - /// Report an error if there are file paths that belong to different projects. - Future packagesFileFor(List filePaths) async { - String projectPath = await projectPathContaining(filePaths.first); - if (projectPath != null) { - if (projectPath.endsWith(fs.path.separator)) - projectPath = projectPath.substring(0, projectPath.length - 1); - final String projectPrefix = projectPath + fs.path.separator; - // Assert that all file paths are contained in the same project directory - for (String filePath in filePaths) { - if (!filePath.startsWith(projectPrefix) && filePath != projectPath) - throwToolExit('Files in different projects cannot be analyzed at the same time.\n' - ' Project: $projectPath\n File outside project: $filePath'); - } - } else { - // Assert that all file paths are not contained in any project - for (String filePath in filePaths) { - final String otherProjectPath = await projectPathContaining(filePath); - if (otherProjectPath != null) - throwToolExit('Files inside a project cannot be analyzed at the same time as files not in any project.\n' - ' File inside a project: $filePath'); - } - } - - if (projectPath == null) - return null; - final File packagesFile = fs.file(fs.path.join(projectPath, '.packages')); - return await packagesFile.exists() ? packagesFile : null; - } - - Future projectPathContaining(String targetPath) async { - final FileSystemEntity target = await fs.isDirectory(targetPath) ? fs.directory(targetPath) : fs.file(targetPath); - final Directory projectDirectory = await projectDirectoryContaining(target); - return projectDirectory?.path; - } - - Future projectDirectoryContaining(FileSystemEntity entity) async { - Directory dir = entity is Directory ? entity : entity.parent; - dir = dir.absolute; - while (!await dir.childFile('pubspec.yaml').exists()) { - final Directory parent = dir.parent; - if (parent == null || parent.path == dir.path) - return null; - dir = parent; - } - return dir; - } - - List flutterRootComponents; - bool isFlutterLibrary(String filename) { - flutterRootComponents ??= fs.path.normalize(fs.path.absolute(Cache.flutterRoot)).split(fs.path.separator); - final List filenameComponents = fs.path.normalize(fs.path.absolute(filename)).split(fs.path.separator); - if (filenameComponents.length < flutterRootComponents.length + 4) // the 4: 'packages', package_name, 'lib', file_name - return false; - for (int index = 0; index < flutterRootComponents.length; index += 1) { - if (flutterRootComponents[index] != filenameComponents[index]) - return false; - } - if (filenameComponents[flutterRootComponents.length] != 'packages') - return false; - if (filenameComponents[flutterRootComponents.length + 1] == 'flutter_tools') - return false; - if (filenameComponents[flutterRootComponents.length + 2] != 'lib') - return false; - return true; - } - - List _collectDartFiles(Directory dir, List collected) { - // Bail out in case of a .dartignore. - if (fs.isFileSync(fs.path.join(dir.path, '.dartignore'))) - return collected; - - for (FileSystemEntity entity in dir.listSync(recursive: false, followLinks: false)) { - if (isDartFile(entity)) - collected.add(entity); - if (entity is Directory) { - final String name = fs.path.basename(entity.path); - if (!name.startsWith('.') && name != 'packages') - _collectDartFiles(entity, collected); - } - } - - return collected; - } } diff --git a/packages/flutter_tools/lib/src/dart/analysis.dart b/packages/flutter_tools/lib/src/dart/analysis.dart index c0d9822af56..09d2425848d 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -2,269 +2,220 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:collection'; - -import 'package:analyzer/error/error.dart'; -import 'package:analyzer/file_system/file_system.dart' as file_system; -import 'package:analyzer/file_system/physical_file_system.dart'; -// TODO(goderbauer): update import path when deprecation has landed on stable -import 'package:analyzer/source/analysis_options_provider.dart'; // ignore: deprecated_member_use -import 'package:analyzer/source/error_processor.dart'; -import 'package:analyzer/source/line_info.dart'; -import 'package:analyzer/source/package_map_resolver.dart'; // ignore: deprecated_member_use -import 'package:analyzer/src/context/builder.dart'; // ignore: implementation_imports -import 'package:analyzer/src/dart/sdk/sdk.dart'; // ignore: implementation_imports -import 'package:analyzer/src/generated/engine.dart'; // ignore: implementation_imports -import 'package:analyzer/src/generated/java_io.dart'; // ignore: implementation_imports -import 'package:analyzer/src/generated/source.dart'; // ignore: implementation_imports -import 'package:analyzer/src/generated/source_io.dart'; // ignore: implementation_imports -import 'package:analyzer/src/task/options.dart'; // ignore: implementation_imports -import 'package:linter/src/rules.dart' as linter; // ignore: implementation_imports -import 'package:cli_util/cli_util.dart' as cli_util; -import 'package:package_config/packages.dart' show Packages; -import 'package:package_config/src/packages_impl.dart' show MapPackages; // ignore: implementation_imports -import 'package:plugin/manager.dart'; -import 'package:plugin/plugin.dart'; +import 'dart:async'; +import 'dart:convert'; import '../base/file_system.dart' hide IOSink; +import '../base/file_system.dart'; import '../base/io.dart'; +import '../base/platform.dart'; +import '../base/process_manager.dart'; +import '../globals.dart'; -class AnalysisDriver { - AnalysisDriver(this.options) { - AnalysisEngine.instance.logger = - new _StdLogger(outSink: options.outSink, errorSink: options.errorSink); - _processPlugins(); - } +class AnalysisServer { + AnalysisServer(this.sdkPath, this.directories, {this.previewDart2: false}); - final Set _analyzedSources = new HashSet(); + final String sdkPath; + final List directories; + final bool previewDart2; - AnalysisOptionsProvider analysisOptionsProvider = - new AnalysisOptionsProvider(); + Process _process; + final StreamController _analyzingController = + new StreamController.broadcast(); + final StreamController _errorsController = + new StreamController.broadcast(); - file_system.ResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE; + int _id = 0; - AnalysisContext context; + Future start() async { + final String snapshot = + fs.path.join(sdkPath, 'bin/snapshots/analysis_server.dart.snapshot'); + final List command = [ + fs.path.join(sdkPath, 'bin', 'dart'), + snapshot, + '--sdk', + sdkPath, + ]; - DriverOptions options; - - String get sdkDir => options.dartSdkPath ?? cli_util.getSdkPath(); - - List analyze(Iterable files) { - final List infos = _analyze(files); - final List errors = []; - for (AnalysisErrorInfo info in infos) { - for (AnalysisError error in info.errors) { - if (!_isFiltered(error)) - errors.add(new AnalysisErrorDescription(error, info.lineInfo)); - } - } - return errors; - } - - List _analyze(Iterable files) { - context = AnalysisEngine.instance.createAnalysisContext(); - _processAnalysisOptions(); - context.analysisOptions = options; - final PackageInfo packageInfo = new PackageInfo(options.packageMap); - final List resolvers = _getResolvers(context, packageInfo.asMap()); - context.sourceFactory = - new SourceFactory(resolvers, packageInfo.asPackages()); - - final List sources = []; - final ChangeSet changeSet = new ChangeSet(); - for (File file in files) { - final JavaFile sourceFile = new JavaFile(fs.path.normalize(file.absolute.path)); - Source source = new FileBasedSource(sourceFile, sourceFile.toURI()); - final Uri uri = context.sourceFactory.restoreUri(source); - if (uri != null) { - source = new FileBasedSource(sourceFile, uri); - } - sources.add(source); - changeSet.addedSource(source); - } - context.applyChanges(changeSet); - - final List infos = []; - for (Source source in sources) { - context.computeErrors(source); - infos.add(context.getErrors(source)); - _analyzedSources.add(source); - } - - return infos; - } - - List _getResolvers(InternalAnalysisContext context, - Map> packageMap) { - - // Create our list of resolvers. - final List resolvers = []; - - // Look for an embedder. - final EmbedderYamlLocator locator = new EmbedderYamlLocator(packageMap); - if (locator.embedderYamls.isNotEmpty) { - // Create and configure an embedded SDK. - final EmbedderSdk sdk = new EmbedderSdk(PhysicalResourceProvider.INSTANCE, locator.embedderYamls); - // Fail fast if no URI mappings are found. - assert(sdk.libraryMap.size() > 0); - sdk.analysisOptions = context.analysisOptions; - - resolvers.add(new DartUriResolver(sdk)); + if (previewDart2) { + command.add('--preview-dart-2'); } else { - // Fall back to a standard SDK if no embedder is found. - final FolderBasedDartSdk sdk = new FolderBasedDartSdk(resourceProvider, - PhysicalResourceProvider.INSTANCE.getFolder(sdkDir)); - sdk.analysisOptions = context.analysisOptions; - - resolvers.add(new DartUriResolver(sdk)); + command.add('--no-preview-dart-2'); } - if (options.packageRootPath != null) { - final ContextBuilderOptions builderOptions = new ContextBuilderOptions(); - builderOptions.defaultPackagesDirectoryPath = options.packageRootPath; - final ContextBuilder builder = new ContextBuilder(resourceProvider, null, null, - options: builderOptions); - final PackageMapUriResolver packageUriResolver = new PackageMapUriResolver(resourceProvider, - builder.convertPackagesToMap(builder.createPackageMap(''))); + 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 - resolvers.add(packageUriResolver); - } + final Stream errorStream = + _process.stderr.transform(utf8.decoder).transform(const LineSplitter()); + errorStream.listen(printError); - resolvers.add(new file_system.ResourceUriResolver(resourceProvider)); - return resolvers; + final Stream inStream = + _process.stdout.transform(utf8.decoder).transform(const LineSplitter()); + inStream.listen(_handleServerResponse); + + // Available options (many of these are obsolete): + // enableAsync, enableDeferredLoading, enableEnums, enableNullAwareOperators, + // enableSuperMixins, generateDart2jsHints, generateHints, generateLints + _sendCommand('analysis.updateOptions', { + 'options': {'enableSuperMixins': true} + }); + + _sendCommand('server.setSubscriptions', { + 'subscriptions': ['STATUS'] + }); + + _sendCommand('analysis.setAnalysisRoots', + {'included': directories, 'excluded': []}); } - bool _isFiltered(AnalysisError error) { - final ErrorProcessor processor = ErrorProcessor.getProcessor(context.analysisOptions, error); - // Filtered errors are processed to a severity of null. - return processor != null && processor.severity == null; + Stream get onAnalyzing => _analyzingController.stream; + Stream get onErrors => _errorsController.stream; + + Future get onExit => _process.exitCode; + + void _sendCommand(String method, Map params) { + final String message = json.encode({ + 'id': (++_id).toString(), + 'method': method, + 'params': params + }); + _process.stdin.writeln(message); + printTrace('==> $message'); } - void _processAnalysisOptions() { - final String optionsPath = options.analysisOptionsFile; - if (optionsPath != null) { - final file_system.File file = - PhysicalResourceProvider.INSTANCE.getFile(optionsPath); - final Map optionMap = - analysisOptionsProvider.getOptionsFromFile(file); - if (optionMap != null) - applyToAnalysisOptions(options, optionMap); + void _handleServerResponse(String line) { + printTrace('<== $line'); + + final dynamic response = json.decode(line); + + if (response is Map) { + if (response['event'] != null) { + final String event = response['event']; + final dynamic params = response['params']; + + if (params is Map) { + if (event == 'server.status') + _handleStatus(response['params']); + else if (event == 'analysis.errors') + _handleAnalysisIssues(response['params']); + else if (event == 'server.error') + _handleServerError(response['params']); + } + } else if (response['error'] != null) { + // Fields are 'code', 'message', and 'stackTrace'. + final Map error = response['error']; + printError( + 'Error response from the server: ${error['code']} ${error['message']}'); + if (error['stackTrace'] != null) { + printError(error['stackTrace']); + } + } } } - void _processPlugins() { - final List plugins = []; - plugins.addAll(AnalysisEngine.instance.requiredPlugins); - final ExtensionManager manager = new ExtensionManager(); - manager.processPlugins(plugins); - linter.registerLintRules(); + void _handleStatus(Map statusInfo) { + // {"event":"server.status","params":{"analysis":{"isAnalyzing":true}}} + if (statusInfo['analysis'] != null && !_analyzingController.isClosed) { + final bool isAnalyzing = statusInfo['analysis']['isAnalyzing']; + _analyzingController.add(isAnalyzing); + } + } + + void _handleServerError(Map error) { + // Fields are 'isFatal', 'message', and 'stackTrace'. + printError('Error from the analysis server: ${error['message']}'); + if (error['stackTrace'] != null) { + printError(error['stackTrace']); + } + } + + void _handleAnalysisIssues(Map issueInfo) { + // {"event":"analysis.errors","params":{"file":"/Users/.../lib/main.dart","errors":[]}} + final String file = issueInfo['file']; + final List errors = issueInfo['errors'] + .map((Map json) => new AnalysisError(json)) + .toList(); + if (!_errorsController.isClosed) + _errorsController.add(new FileAnalysisErrors(file, errors)); + } + + Future dispose() async { + await _analyzingController.close(); + await _errorsController.close(); + return _process?.kill(); } } -class AnalysisDriverException implements Exception { - AnalysisDriverException([this.message]); +class AnalysisError implements Comparable { + AnalysisError(this.json); - final String message; + static final Map _severityMap = { + 'ERROR': 3, + 'WARNING': 2, + 'INFO': 1 + }; + + static final String _separator = platform.isWindows ? '-' : '•'; + + // "severity":"INFO","type":"TODO","location":{ + // "file":"/Users/.../lib/test.dart","offset":362,"length":72,"startLine":15,"startColumn":4 + // },"message":"...","hasFix":false} + Map json; + + String get severity => json['severity']; + int get severityLevel => _severityMap[severity] ?? 0; + String get type => json['type']; + String get message => json['message']; + String get code => json['code']; + + String get file => json['location']['file']; + int get startLine => json['location']['startLine']; + int get startColumn => json['location']['startColumn']; + int get offset => json['location']['offset']; + + String get messageSentenceFragment { + if (message.endsWith('.')) { + return message.substring(0, message.length - 1); + } else { + return message; + } + } @override - String toString() => message == null ? 'Exception' : 'Exception: $message'; -} + int compareTo(AnalysisError other) { + // Sort in order of file path, error location, severity, and message. + if (file != other.file) + return file.compareTo(other.file); -class AnalysisErrorDescription { - AnalysisErrorDescription(this.error, this.line); + if (offset != other.offset) + return offset - other.offset; - static Directory cwd = fs.currentDirectory.absolute; + final int diff = other.severityLevel - severityLevel; + if (diff != 0) + return diff; - final AnalysisError error; - final LineInfo line; - - ErrorCode get errorCode => error.errorCode; - - String get errorType { - final ErrorSeverity severity = errorCode.errorSeverity; - if (severity == ErrorSeverity.INFO) { - if (errorCode.type == ErrorType.HINT || errorCode.type == ErrorType.LINT) - return errorCode.type.displayName; - } - return severity.displayName; + return message.compareTo(other.message); } - CharacterLocation get location => line.getLocation(error.offset); - - String get path => _shorten(cwd.path, error.source.fullName); - - Source get source => error.source; - - String asString() => '[$errorType] ${error.message} ($path, ' - 'line ${location.lineNumber}, col ${location.columnNumber})'; - - static String _shorten(String root, String path) => - path.startsWith(root) ? path.substring(root.length + 1) : path; -} - -class DriverOptions extends AnalysisOptionsImpl { - DriverOptions() { - // Set defaults. - lint = true; - generateSdkErrors = false; - trackCacheDependencies = false; - } - - /// The path to the dart SDK. - String dartSdkPath; - - /// Map of packages to folder paths. - Map packageMap; - - /// The path to the package root. - String packageRootPath; - - /// The path to analysis options. - String analysisOptionsFile; - - /// Out sink for logging. - IOSink outSink = stdout; - - /// Error sink for logging. - IOSink errorSink = stderr; -} - -class PackageInfo { - PackageInfo(Map packageMap) { - final Map packages = new HashMap(); - for (String package in packageMap.keys) { - final String path = packageMap[package]; - packages[package] = new Uri.directory(path); - _map[package] = [ - PhysicalResourceProvider.INSTANCE.getFolder(path) - ]; - } - _packages = new MapPackages(packages); - } - - Packages _packages; - - Map> asMap() => _map; - final HashMap> _map = - new HashMap>(); - - Packages asPackages() => _packages; -} - -class _StdLogger extends Logger { - _StdLogger({this.outSink, this.errorSink}); - - final IOSink outSink; - final IOSink errorSink; - @override - void logError(String message, [Exception exception]) => - errorSink.writeln(message); + String toString() { + return '${severity.toLowerCase().padLeft(7)} $_separator ' + '$messageSentenceFragment $_separator ' + '${fs.path.relative(file)}:$startLine:$startColumn'; + } - @override - void logInformation(String message, [Exception exception]) { - // TODO(pq): remove once addressed in analyzer (http://dartbug.com/28285) - if (message != 'No definition of type FutureOr') - outSink.writeln(message); + String toLegacyString() { + return '[${severity.toLowerCase()}] $messageSentenceFragment ($file:$startLine:$startColumn)'; } } + +class FileAnalysisErrors { + FileAnalysisErrors(this.file, this.errors); + + final String file; + final List errors; +} diff --git a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart index 9a02aaf6058..4ae4304312f 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart @@ -383,12 +383,19 @@ class FlutterCommandRunner extends CommandRunner { Cache.flutterRoot ??= _defaultFlutterRoot; } - /// Get all pub packages in the Flutter repo. - List getRepoPackages() { + /// Get the root directories of the repo - the directories containing Dart packages. + List getRepoRoots() { final String root = fs.path.absolute(Cache.flutterRoot); // not bin, and not the root - return ['dev', 'examples', 'packages'] - .expand((String path) => _gatherProjectPaths(fs.path.join(root, path))) + return ['dev', 'examples', 'packages'].map((String item) { + return fs.path.join(root, item); + }).toList(); + } + + /// Get all pub packages in the Flutter repo. + List getRepoPackages() { + return getRepoRoots() + .expand((String root) => _gatherProjectPaths(root)) .map((String dir) => fs.directory(dir)) .toList(); } diff --git a/packages/flutter_tools/test/commands/analyze_continuously_test.dart b/packages/flutter_tools/test/commands/analyze_continuously_test.dart index 3686de462a5..2a67faa7fcd 100644 --- a/packages/flutter_tools/test/commands/analyze_continuously_test.dart +++ b/packages/flutter_tools/test/commands/analyze_continuously_test.dart @@ -6,7 +6,7 @@ import 'dart:async'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/os.dart'; -import 'package:flutter_tools/src/commands/analyze_continuously.dart'; +import 'package:flutter_tools/src/dart/analysis.dart'; import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/dart/sdk.dart'; import 'package:flutter_tools/src/runner/flutter_command_runner.dart'; diff --git a/packages/flutter_tools/test/commands/analyze_duplicate_names_test.dart b/packages/flutter_tools/test/commands/analyze_duplicate_names_test.dart deleted file mode 100644 index c56ff8be388..00000000000 --- a/packages/flutter_tools/test/commands/analyze_duplicate_names_test.dart +++ /dev/null @@ -1,48 +0,0 @@ -// 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. - -import 'package:flutter_tools/src/cache.dart'; -import 'package:flutter_tools/src/base/file_system.dart'; -import 'package:flutter_tools/src/commands/analyze.dart'; -import 'package:test/test.dart'; - -import '../src/common.dart'; -import '../src/context.dart'; -import '../src/mocks.dart'; - -void main() { - Directory tempDir; - - setUpAll(() { - Cache.disableLocking(); - }); - - setUp(() { - tempDir = fs.systemTempDirectory.createTempSync('analysis_duplicate_names_test'); - }); - - tearDown(() { - tempDir?.deleteSync(recursive: true); - }); - - group('analyze', () { - testUsingContext('flutter analyze with two files with the same name', () async { - final File dartFileA = fs.file(fs.path.join(tempDir.path, 'a.dart')); - dartFileA.parent.createSync(); - dartFileA.writeAsStringSync('library test;'); - final File dartFileB = fs.file(fs.path.join(tempDir.path, 'b.dart')); - dartFileB.writeAsStringSync('library test;'); - - final AnalyzeCommand command = new AnalyzeCommand(); - applyMocksToCommand(command); - return createTestCommandRunner(command).run( - ['analyze', '--no-current-package', dartFileA.path, dartFileB.path] - ).then((Null value) { - expect(testLogger.statusText, contains('Analyzing')); - expect(testLogger.statusText, contains('No issues found!')); - }); - - }); - }); -} diff --git a/packages/flutter_tools/test/commands/analyze_once_test.dart b/packages/flutter_tools/test/commands/analyze_once_test.dart index 8f409551ab9..c8c574ed02c 100644 --- a/packages/flutter_tools/test/commands/analyze_once_test.dart +++ b/packages/flutter_tools/test/commands/analyze_once_test.dart @@ -17,7 +17,6 @@ import '../src/common.dart'; import '../src/context.dart'; void main() { - final String analyzerSeparator = platform.isWindows ? '-' : '•'; group('analyze once', () { @@ -55,7 +54,7 @@ void main() { }, timeout: allowForRemotePubInvocation); // Analyze in the current directory - no arguments - testUsingContext('flutter analyze working directory', () async { + testUsingContext('working directory', () async { await runCommand( command: new AnalyzeCommand(workingDirectory: fs.directory(projectPath)), arguments: ['analyze'], @@ -64,17 +63,17 @@ void main() { }); // Analyze a specific file outside the current directory - testUsingContext('flutter analyze one file', () async { + testUsingContext('passing one file throws', () async { await runCommand( command: new AnalyzeCommand(), arguments: ['analyze', libMain.path], - statusTextContains: ['No issues found!'], + toolExit: true, + exitMessageContains: 'is not a directory', ); }); // Analyze in the current directory - no arguments - testUsingContext('flutter analyze working directory with errors', () async { - + testUsingContext('working directory with errors', () async { // Break the code to produce the "The parameter 'onPressed' is required" hint // that is upgraded to a warning in package:flutter/analysis_options_user.yaml // to assert that we are using the default Flutter analysis options. @@ -98,22 +97,7 @@ void main() { statusTextContains: [ 'Analyzing', 'warning $analyzerSeparator The parameter \'onPressed\' is required', - 'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used', - '2 issues found.', - ], - toolExit: true, - ); - }); - - // Analyze a specific file outside the current directory - testUsingContext('flutter analyze one file with errors', () async { - await runCommand( - command: new AnalyzeCommand(), - arguments: ['analyze', libMain.path], - statusTextContains: [ - 'Analyzing', - 'warning $analyzerSeparator The parameter \'onPressed\' is required', - 'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used', + 'info $analyzerSeparator The method \'_incrementCounter\' isn\'t used', '2 issues found.', ], toolExit: true, @@ -121,8 +105,7 @@ void main() { }); // Analyze in the current directory - no arguments - testUsingContext('flutter analyze working directory with local options', () async { - + testUsingContext('working directory with local options', () async { // Insert an analysis_options.yaml file in the project // which will trigger a lint for broken code that was inserted earlier final File optionsFile = fs.file(fs.path.join(projectPath, 'analysis_options.yaml')); @@ -140,15 +123,15 @@ void main() { statusTextContains: [ 'Analyzing', 'warning $analyzerSeparator The parameter \'onPressed\' is required', - 'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used', - 'lint $analyzerSeparator Only throw instances of classes extending either Exception or Error', + 'info $analyzerSeparator The method \'_incrementCounter\' isn\'t used', + 'info $analyzerSeparator Only throw instances of classes extending either Exception or Error', '3 issues found.', ], toolExit: true, ); }); - testUsingContext('flutter analyze no duplicate issues', () async { + testUsingContext('no duplicate issues', () async { final Directory tempDir = fs.systemTempDirectory.createTempSync('analyze_once_test_').absolute; try { @@ -182,22 +165,6 @@ void bar() { } }); - // Analyze a specific file outside the current directory - testUsingContext('flutter analyze one file with local options', () async { - await runCommand( - command: new AnalyzeCommand(), - arguments: ['analyze', libMain.path], - statusTextContains: [ - 'Analyzing', - 'warning $analyzerSeparator The parameter \'onPressed\' is required', - 'hint $analyzerSeparator The method \'_incrementCounter\' isn\'t used', - 'lint $analyzerSeparator Only throw instances of classes extending either Exception or Error', - '3 issues found.', - ], - toolExit: true, - ); - }); - testUsingContext('--preview-dart-2', () async { const String contents = ''' StringBuffer bar = StringBuffer('baz'); @@ -255,18 +222,23 @@ Future runCommand({ List statusTextContains, List errorTextContains, bool toolExit: false, + String exitMessageContains, }) async { try { arguments.insert(0, '--flutter-root=${Cache.flutterRoot}'); await createTestCommandRunner(command).run(arguments); expect(toolExit, isFalse, reason: 'Expected ToolExit exception'); - } on ToolExit { + } on ToolExit catch (e) { if (!toolExit) { testLogger.clear(); rethrow; } + if (exitMessageContains != null) { + expect(e.message, contains(exitMessageContains)); + } } assertContains(testLogger.statusText, statusTextContains); assertContains(testLogger.errorText, errorTextContains); + testLogger.clear(); } diff --git a/packages/flutter_tools/test/commands/create_test.dart b/packages/flutter_tools/test/commands/create_test.dart index b1550502643..a893d957105 100644 --- a/packages/flutter_tools/test/commands/create_test.dart +++ b/packages/flutter_tools/test/commands/create_test.dart @@ -436,14 +436,13 @@ Future _createAndAnalyzeProject( { List unexpectedPaths = const [], bool plugin = false }) async { await _createProject(dir, createArgs, expectedPaths, unexpectedPaths: unexpectedPaths, plugin: plugin); if (plugin) { - await _analyzeProject(dir.path, target: fs.path.join(dir.path, 'lib', 'flutter_project.dart')); - await _analyzeProject(fs.path.join(dir.path, 'example')); + await _analyzeProject(dir.path); } else { await _analyzeProject(dir.path); } } -Future _analyzeProject(String workingDir, {String target}) async { +Future _analyzeProject(String workingDir) async { final String flutterToolsPath = fs.path.absolute(fs.path.join( 'bin', 'flutter_tools.dart', @@ -453,8 +452,6 @@ Future _analyzeProject(String workingDir, {String target}) async { ..addAll(dartVmFlags) ..add(flutterToolsPath) ..add('analyze'); - if (target != null) - args.add(target); final ProcessResult exec = await Process.run( '$dartSdkPath/bin/dart',