From 2f41ea546f98a2bacabcd2444ca7ce4127a4360a Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Wed, 11 Apr 2018 14:50:36 -0700 Subject: [PATCH] re-write flutter analyze (the single-shot and --flutter-repo) to use the analysis server (#16281) re-write flutter analyze (the single-shot and --flutter-repo) to use the analysis server --- CONTRIBUTING.md | 2 +- analysis_options.yaml | 7 +- analysis_options_repo.yaml | 152 ------- dev/bots/analyze-sample-code.dart | 8 +- dev/devicelab/bin/tasks/dartdocs.dart | 21 +- packages/analysis_options.yaml | 8 + .../flutter/lib/analysis_options_user.yaml | 16 +- packages/flutter_tools/analysis_options.yaml | 4 + .../lib/src/commands/analyze.dart | 63 ++- .../src/commands/analyze_continuously.dart | 229 +---------- .../lib/src/commands/analyze_once.dart | 353 ++++++---------- .../flutter_tools/lib/src/dart/analysis.dart | 382 ++++++++---------- .../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 | 66 +-- .../test/commands/create_test.dart | 7 +- 17 files changed, 423 insertions(+), 960 deletions(-) delete mode 100644 analysis_options_repo.yaml create mode 100644 packages/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 eca9cc5811d23..80d2cce2fcb23 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 1bb8aac41e492..dd6a04c6ea539 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/analysis_options_repo.yaml b/analysis_options_repo.yaml deleted file mode 100644 index 893a9af649110..0000000000000 --- a/analysis_options_repo.yaml +++ /dev/null @@ -1,152 +0,0 @@ -# Specify analysis options. -# -# Until there are meta linter rules, each desired lint must be explicitly enabled. -# See: https://github.com/dart-lang/linter/issues/288 -# -# For a list of lints, see: http://dart-lang.github.io/linter/lints/ -# 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: -# - analysis_options.yaml -# - analysis_options_repo.yaml (this file) -# - packages/flutter/lib/analysis_options_user.yaml -# -# This file contains the analysis options used by 'flutter analyze' when analyzing -# the flutter repository. It is very similar to analysis_options.yaml; -# the only difference (currently) is the public_member_api_docs option, -# which is turned on and programmatically reduced to a single output line -# indicating the # of violations for that rule. - -analyzer: - language: - enableStrictCallChecks: true - enableSuperMixins: true - strong-mode: - implicit-dynamic: false - errors: - # treat missing required parameters as a warning (not a hint) - missing_required_param: warning - # treat missing returns as a warning (not a hint) - missing_return: warning - # allow having TODOs in the code - todo: ignore - # `flutter analyze` (without `--watch`) just ignores directories - # that contain a .dartignore file, and this file does not have any - # effect on what files are actually analyzed. - -linter: - rules: - # these rules are documented on and in the same order as - # the Dart Lint rules page to make maintenance easier - # https://github.com/dart-lang/linter/blob/master/example/all.yaml - - always_declare_return_types - - always_put_control_body_on_new_line - # - always_put_required_named_parameters_first # we prefer having parameters in the same order as fields https://github.com/flutter/flutter/issues/10219 - - always_require_non_null_named_parameters - - always_specify_types - - annotate_overrides - # - avoid_annotating_with_dynamic # conflicts with always_specify_types - - avoid_as - # - avoid_bool_literals_in_conditional_expressions # not yet tested - # - avoid_catches_without_on_clauses # we do this commonly - # - avoid_catching_errors # we do this commonly - - avoid_classes_with_only_static_members - - avoid_empty_else - - avoid_function_literals_in_foreach_calls - - avoid_init_to_null - - avoid_null_checks_in_equality_operators - # - avoid_positional_boolean_parameters # not yet tested - # - avoid_private_typedef_functions # not yet tested - - avoid_relative_lib_imports - - avoid_renaming_method_parameters - - avoid_return_types_on_setters - # - avoid_returning_null # we do this commonly - # - avoid_returning_this # https://github.com/dart-lang/linter/issues/842 - # - avoid_setters_without_getters # not yet tested - # - avoid_single_cascade_in_expression_statements # not yet tested - - avoid_slow_async_io - # - avoid_types_as_parameter_names # https://github.com/dart-lang/linter/pull/954/files - # - avoid_types_on_closure_parameters # conflicts with always_specify_types - # - avoid_unused_constructor_parameters # https://github.com/dart-lang/linter/pull/847 - - await_only_futures - - camel_case_types - - cancel_subscriptions - # - cascade_invocations # not yet tested - # - close_sinks # https://github.com/flutter/flutter/issues/5789 - # - comment_references # blocked on https://github.com/dart-lang/dartdoc/issues/1153 - # - constant_identifier_names # https://github.com/dart-lang/linter/issues/204 - - control_flow_in_finally - - directives_ordering - - empty_catches - - empty_constructor_bodies - - empty_statements - - hash_and_equals - - implementation_imports - # - invariant_booleans # https://github.com/flutter/flutter/issues/5790 - - iterable_contains_unrelated_type - # - join_return_with_assignment # not yet tested - - library_names - - library_prefixes - - list_remove_unrelated_type - # - literal_only_boolean_expressions # https://github.com/flutter/flutter/issues/5791 - - no_adjacent_strings_in_list - - no_duplicate_case_values - - non_constant_identifier_names - # - omit_local_variable_types # opposite of always_specify_types - # - one_member_abstracts # too many false positives - # - only_throw_errors # https://github.com/flutter/flutter/issues/5792 - - overridden_fields - - package_api_docs - - package_names - - package_prefixed_library_names - # - parameter_assignments # we do this commonly - - prefer_adjacent_string_concatenation - - prefer_asserts_in_initializer_lists - - prefer_bool_in_asserts - - prefer_collection_literals - - prefer_conditional_assignment - - prefer_const_constructors - - prefer_const_constructors_in_immutables - - prefer_const_declarations - - prefer_const_literals_to_create_immutables - # - prefer_constructors_over_static_methods # not yet tested - - prefer_contains - # - prefer_equal_for_default_values # not yet tested - # - prefer_expression_function_bodies # conflicts with https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#consider-using--for-short-functions-and-methods - - prefer_final_fields - - prefer_final_locals - - prefer_foreach - # - prefer_function_declarations_over_variables # not yet tested - - prefer_initializing_formals - # - prefer_interpolation_to_compose_strings # not yet tested - - prefer_is_empty - - prefer_is_not_empty - - prefer_single_quotes - - prefer_typing_uninitialized_variables - - public_member_api_docs # this is the only difference from analysis_options.yaml - - recursive_getters - - slash_for_doc_comments - - sort_constructors_first - - sort_unnamed_constructors_first - - super_goes_last - - test_types_in_equals - - throw_in_finally - # - type_annotate_public_apis # subset of always_specify_types - - type_init_formals - # - unawaited_futures # https://github.com/flutter/flutter/issues/5793 - - unnecessary_brace_in_string_interps - - unnecessary_getters_setters - # - unnecessary_lambdas # https://github.com/dart-lang/linter/issues/498 - - unnecessary_null_aware_assignments - - unnecessary_null_in_if_null_operators - - unnecessary_overrides - - unnecessary_parenthesis - # - unnecessary_statements # not yet tested - - unnecessary_this - - unrelated_type_equality_checks - - use_rethrow_when_possible - # - use_setters_to_change_properties # not yet tested - # - use_string_buffers # https://github.com/dart-lang/linter/pull/664 - # - use_to_and_as_if_applicable # has false positives, so we prefer to catch this by code-review - - valid_regexps diff --git a/dev/bots/analyze-sample-code.dart b/dev/bots/analyze-sample-code.dart index 4b46108bf53a8..344f4ef3ac76e 100644 --- a/dev/bots/analyze-sample-code.dart +++ b/dev/bots/analyze-sample-code.dart @@ -185,6 +185,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); @@ -203,7 +205,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); @@ -212,10 +214,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 a2d083303bdbd..36ccf17ba70e0 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')) { + 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 if (entry.startsWith('info • Document all public members •')) { publicMembers += 1; - } else if (entry.startsWith('[')) { + } else if (entry.startsWith('info •') || entry.startsWith('warning •') || entry.startsWith('error •')) { otherErrors += 1; - } else if (entry.startsWith('(Ran in ')) { + } else if (entry.contains(' (ran in ')) { // ignore this line - } else { + } 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 0000000000000..217831d18fc5f --- /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 41cb657c9bb2d..504456df844d4 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_tools/analysis_options.yaml b/packages/flutter_tools/analysis_options.yaml new file mode 100644 index 0000000000000..b8591ca6dba1a --- /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 0f4969e9b5e3a..b0ba4a43365ce 100644 --- a/packages/flutter_tools/lib/src/commands/analyze.dart +++ b/packages/flutter_tools/lib/src/commands/analyze.dart @@ -9,28 +9,47 @@ 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.'); // 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 +59,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 +80,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 c2bd5f81b759f..f172763bf5d4e 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'; 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; @@ -105,10 +110,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) @@ -122,10 +131,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); }); } @@ -133,209 +145,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.sdk, this.directories, { this.previewDart2: false }); - - final String sdk; - 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(sdk, 'bin/snapshots/analysis_server.dart.snapshot'); - final List command = [ - fs.path.join(dartSdkPath, 'bin', 'dart'), - snapshot, - '--sdk', - sdk, - ]; - - 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 0ad0646b341e0..3614f6ccb563c 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -3,293 +3,182 @@ // 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'; +import '../dart/sdk.dart'; 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.NOT_FOUND) { + 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)); - - // 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.'); - - 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['flutter-repo']) { + // check for conflicting dependencies + final PackageDependencyTracker dependencies = new PackageDependencyTracker(); + dependencies.checkForConflictingDependencies(repoPackages, dependencies); - // 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); - } + directories.addAll(repoRoots); - if (previewDart2) { - arguments.add('--preview-dart-2'); - } else { - arguments.add('--no-preview-dart-2'); + if (argResults.wasParsed('current-package') && argResults['current-package']) { + directories.add(currentDirectory); + } + } else { + if (argResults['current-package']) { + directories.add(currentDirectory); } + } - final String dartanalyzer = fs.path.join(Cache.flutterRoot, 'bin', 'cache', 'dart-sdk', '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; + if (argResults['dartdocs'] && !argResults['flutter-repo']) { + throwToolExit('The --dartdocs option is currently only supported with --flutter-repo.'); } - for (Directory dir in repoPackages) { - _collectDartFiles(dir, dartFiles); - pubSpecDirectories.add(dir); + if (directories.isEmpty) { + throwToolExit('Nothing to analyze.', exitCode: 0); } - // determine what all the various .packages files depend on - final PackageDependencyTracker dependencies = new PackageDependencyTracker(); - dependencies.checkForConflictingDependencies(pubSpecDirectories, dependencies); - final Map packages = dependencies.asPackageMap(); + // analyze all + final Completer analysisCompleter = new Completer(); + final List errors = []; + + final AnalysisServer server = new AnalysisServer( + dartSdkPath, + 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)}...'); - } else { - logger.printStatus('Analyzing ${dartFiles.length} files...'); + // 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'); } } - 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); + // emit benchmarks + if (isBenchmarking) { + writeBenchmark(timer, errors.length, undocumentedMembers); + } - if (isBenchmarking) - writeBenchmark(stopwatch, errorCount, membersMissingDocumentation); + // report results + dumpErrors(errors.map((AnalysisError error) => error.toLegacyString())); - 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 (errors.isNotEmpty && argResults['preamble']) { + printStatus(''); } - if (argResults['congratulate']) { - if (membersMissingDocumentation > 0) { - printStatus('No analyzer warnings! (ran in ${elapsed}s; $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation)'); - } else { - printStatus('No analyzer warnings! (ran in ${elapsed}s)'); - } + errors.sort(); + for (AnalysisError error in errors) { + printStatus(error.toString()); } - } - /// 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'); - } - } + final String seconds = + (timer.elapsedMilliseconds / 1000.0).toStringAsFixed(1); - if (projectPath == null) - return null; - final File packagesFile = fs.file(fs.path.join(projectPath, '.packages')); - return await packagesFile.exists() ? packagesFile : null; - } + // We consider any level of error to be an error exit (we don't report different levels). + if (errors.isNotEmpty) { + printStatus(''); - 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; - } + printStatus('${errors.length} ${pluralize('issue', errors.length)} found. (ran in ${seconds}s)'); - 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 (undocumentedMembers > 0) { + throwToolExit('[lint] $undocumentedMembers public ' + '${ undocumentedMembers == 1 + ? "member lacks" + : "members lack" } documentation'); + } else { + throwToolExit(null); + } } - 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); + if (argResults['congratulate']) { + if (undocumentedMembers > 0) { + printStatus('No issues found! (ran in ${seconds}s; ' + '$undocumentedMembers public ${ undocumentedMembers == + 1 ? "member lacks" : "members lack" } documentation)'); + } else { + printStatus('No issues found! (ran in ${seconds}s)'); } } - - return collected; } } diff --git a/packages/flutter_tools/lib/src/dart/analysis.dart b/packages/flutter_tools/lib/src/dart/analysis.dart index 421e9cd9186a3..1166835c96e09 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -2,267 +2,215 @@ // 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'; -import 'package:analyzer/source/analysis_options_provider.dart'; -import 'package:analyzer/source/error_processor.dart'; -import 'package:analyzer/source/package_map_resolver.dart'; -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'; +import 'sdk.dart'; + +class AnalysisServer { + AnalysisServer(this.sdk, this.directories, { this.previewDart2: false }); + + final String sdk; + 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(sdk, 'bin/snapshots/analysis_server.dart.snapshot'); + final List command = [ + fs.path.join(dartSdkPath, 'bin', 'dart'), + snapshot, + '--sdk', + sdk, + ]; + + if (previewDart2) { + command.add('--preview-dart-2'); + } else { + command.add('--no-preview-dart-2'); + } -class AnalysisDriver { - AnalysisDriver(this.options) { - AnalysisEngine.instance.logger = - new _StdLogger(outSink: options.outSink, errorSink: options.errorSink); - _processPlugins(); - } - - final Set _analyzedSources = new HashSet(); - - AnalysisOptionsProvider analysisOptionsProvider = - new AnalysisOptionsProvider(); - - file_system.ResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE; - - AnalysisContext context; + 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 - DriverOptions options; + final Stream errorStream = _process.stderr.transform(utf8.decoder).transform(const LineSplitter()); + errorStream.listen(printError); - String get sdkDir => options.dartSdkPath ?? cli_util.getSdkPath(); + final Stream inStream = _process.stdout.transform(utf8.decoder).transform(const LineSplitter()); + inStream.listen(_handleServerResponse); - 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)); + // Available options (many of these are obsolete): + // enableAsync, enableDeferredLoading, enableEnums, enableNullAwareOperators, + // enableSuperMixins, generateDart2jsHints, generateHints, generateLints + _sendCommand('analysis.updateOptions', { + 'options': { + 'enableSuperMixins': true } - } - 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); + _sendCommand('server.setSubscriptions', { + 'subscriptions': ['STATUS'] + }); - final List infos = []; - for (Source source in sources) { - context.computeErrors(source); - infos.add(context.getErrors(source)); - _analyzedSources.add(source); - } - - return infos; + _sendCommand('analysis.setAnalysisRoots', { + 'included': directories, + 'excluded': [] + }); } - List _getResolvers(InternalAnalysisContext context, - Map> packageMap) { - - // Create our list of resolvers. - final List resolvers = []; + Stream get onAnalyzing => _analyzingController.stream; + Stream get onErrors => _errorsController.stream; - // 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; + Future get onExit => _process.exitCode; - resolvers.add(new DartUriResolver(sdk)); - } 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; + void _sendCommand(String method, Map params) { + final String message = json.encode( { + 'id': (++_id).toString(), + 'method': method, + 'params': params + }); + _process.stdin.writeln(message); + printTrace('==> $message'); + } - resolvers.add(new DartUriResolver(sdk)); + 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']); + } } + } - 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(''))); - - resolvers.add(packageUriResolver); + 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); } - - resolvers.add(new file_system.ResourceUriResolver(resourceProvider)); - return resolvers; } - 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; + 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 _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 _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)); } - void _processPlugins() { - final List plugins = []; - plugins.addAll(AnalysisEngine.instance.requiredPlugins); - final ExtensionManager manager = new ExtensionManager(); - manager.processPlugins(plugins); - linter.registerLintRules(); + 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; - - @override - String toString() => message == null ? 'Exception' : 'Exception: $message'; -} + static final Map _severityMap = { + 'ERROR': 3, + 'WARNING': 2, + 'INFO': 1 + }; -class AnalysisErrorDescription { - AnalysisErrorDescription(this.error, this.line); + static final String _separator = platform.isWindows ? '-' : '•'; - static Directory cwd = fs.currentDirectory.absolute; + // "severity":"INFO","type":"TODO","location":{ + // "file":"/Users/.../lib/test.dart","offset":362,"length":72,"startLine":15,"startColumn":4 + // },"message":"...","hasFix":false} + Map json; - final AnalysisError error; - final LineInfo line; + 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']; - ErrorCode get errorCode => error.errorCode; + 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 errorType { - final ErrorSeverity severity = errorCode.errorSeverity; - if (severity == ErrorSeverity.INFO) { - if (errorCode.type == ErrorType.HINT || errorCode.type == ErrorType.LINT) - return errorCode.type.displayName; + String get messageSentenceFragment { + if (message.endsWith('.')) { + return message.substring(0, message.length - 1); + } else { + return message; } - return severity.displayName; } - LineInfo_Location get location => line.getLocation(error.offset); - - String get path => _shorten(cwd.path, error.source.fullName); - - Source get source => error.source; + @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); - String asString() => '[$errorType] ${error.message} ($path, ' - 'line ${location.lineNumber}, col ${location.columnNumber})'; + if (offset != other.offset) + return offset - other.offset; - static String _shorten(String root, String path) => - path.startsWith(root) ? path.substring(root.length + 1) : path; -} + final int diff = other.severityLevel - severityLevel; + if (diff != 0) + return diff; -class DriverOptions extends AnalysisOptionsImpl { - DriverOptions() { - // Set defaults. - lint = true; - generateSdkErrors = false; - trackCacheDependencies = false; + return message.compareTo(other.message); } - /// 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); + @override + String toString() { + return + '${severity.toLowerCase().padLeft(7)} $_separator ' + '$messageSentenceFragment $_separator ' + '${fs.path.relative(file)}:$startLine:$startColumn'; } - Packages _packages; - - Map> asMap() => _map; - final HashMap> _map = - new HashMap>(); - - Packages asPackages() => _packages; + String toLegacyString() { + return '[${severity.toLowerCase()}] $messageSentenceFragment ($file:$startLine:$startColumn)'; + } } -class _StdLogger extends Logger { - _StdLogger({this.outSink, this.errorSink}); +class FileAnalysisErrors { + FileAnalysisErrors(this.file, this.errors); - final IOSink outSink; - final IOSink errorSink; - - @override - void logError(String message, [Exception exception]) => - errorSink.writeln(message); - - @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); - } + 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 9a02aaf6058f3..4ae4304312fbb 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 3686de462a588..2a67faa7fcd8a 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 c56ff8be388b3..0000000000000 --- 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 7de75b8f4cdc1..8bc7b5577d03f 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', () { @@ -33,7 +32,11 @@ void main() { }); tearDownAll(() { - tempDir?.deleteSync(recursive: true); + try { + tempDir?.deleteSync(recursive: true); + } catch (e) { + // ignore errors deleting the temporary directory + } }); // Create a project to be analyzed @@ -50,7 +53,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'], @@ -59,17 +62,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. @@ -93,22 +96,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, @@ -116,8 +104,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')); @@ -135,15 +122,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 { @@ -177,22 +164,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'); @@ -250,18 +221,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 5c1e2d49318be..8fc417dfdda8b 100644 --- a/packages/flutter_tools/test/commands/create_test.dart +++ b/packages/flutter_tools/test/commands/create_test.dart @@ -431,14 +431,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', @@ -448,8 +447,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',