diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 80d2cce2fc..eca9cc5811 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.yaml` file +For a one-off, use `flutter analyze --flutter-repo`. This uses the `analysis_options_repo.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 dd6a04c6ea..1bb8aac41e 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -9,18 +9,22 @@ # # 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 @@ -127,6 +131,7 @@ 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 new file mode 100644 index 0000000000..893a9af649 --- /dev/null +++ b/analysis_options_repo.yaml @@ -0,0 +1,152 @@ +# 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 344f4ef3ac..4b46108bf5 100644 --- a/dev/bots/analyze-sample-code.dart +++ b/dev/bots/analyze-sample-code.dart @@ -185,8 +185,6 @@ 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); @@ -205,7 +203,7 @@ dependencies: print('Found $sampleCodeSections sample code sections.'); final Process process = await Process.start( _flutter, - ['analyze', '--no-preamble', '--no-congratulate', mainDart.parent.path], + ['analyze', '--no-preamble', mainDart.path], workingDirectory: temp.path, ); stderr.addStream(process.stderr); @@ -214,6 +212,10 @@ 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 36ccf17ba7..a2d083303b 100644 --- a/dev/devicelab/bin/tasks/dartdocs.dart +++ b/dev/devicelab/bin/tasks/dartdocs.dart @@ -21,24 +21,21 @@ Future main() async { int publicMembers = 0; int otherErrors = 0; int otherLines = 0; - 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 •')) { + 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('info •') || entry.startsWith('warning •') || entry.startsWith('error •')) { + } else if (entry.startsWith('[')) { otherErrors += 1; - } else if (entry.contains(' (ran in ')) { + } else if (entry.startsWith('(Ran in ')) { // ignore this line - } else if (entry.isNotEmpty) { + } else { otherLines += 1; } } - await for (String entry in analysis.stderr.transform(utf8.decoder).transform(const LineSplitter())) { - print('analyzer stderr: $entry'); - if (entry.startsWith('[lint] ')) { + await for (String entry in analysis.stdout.transform(utf8.decoder).transform(const LineSplitter())) { + print('analyzer stdout: $entry'); + if (entry == 'Building flutter tool...') { // ignore this line } else { otherLines += 1; diff --git a/packages/analysis_options.yaml b/packages/analysis_options.yaml deleted file mode 100644 index 217831d18f..0000000000 --- a/packages/analysis_options.yaml +++ /dev/null @@ -1,8 +0,0 @@ -# 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 504456df84..41cb657c9b 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 four similar analysis options files in the flutter repos: +# There are three similar analysis options files in the flutter repo: # - 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 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 like Atom +# would use it when analyzing the flutter tool itself. # -# When editing, make sure you keep this and /analysis_options.yaml consistent. +# When editing, make sure you keep /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 deleted file mode 100644 index b8591ca6db..0000000000 --- a/packages/flutter_tools/analysis_options.yaml +++ /dev/null @@ -1,4 +0,0 @@ -# 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 b0ba4a4336..0f4969e9b5 100644 --- a/packages/flutter_tools/lib/src/commands/analyze.dart +++ b/packages/flutter_tools/lib/src/commands/analyze.dart @@ -9,47 +9,28 @@ 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', - 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.'); + 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); // 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', - defaultsTo: true, - help: 'When analyzing the flutter repository, display the number of ' - 'files that will be analyzed.'); + 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); } /// The working directory for testing analysis using dartanalyzer. @@ -59,19 +40,17 @@ 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; } @@ -80,15 +59,11 @@ class AnalyzeCommand extends FlutterCommand { Future runCommand() { if (argResults['watch']) { return new AnalyzeContinuously( - argResults, - runner.getRepoRoots(), - runner.getRepoPackages(), - previewDart2: argResults['preview-dart-2'], + argResults, 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 f172763bf5..c2bd5f81b7 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_continuously.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_continuously.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'package:args/args.dart'; @@ -10,20 +11,17 @@ 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.repoRoots, this.repoPackages, { - this.previewDart2: false, - }) : super(argResults); + AnalyzeContinuously(ArgResults argResults, this.repoPackages, { this.previewDart2: false }) : super(argResults); - final List repoRoots; final List repoPackages; final bool previewDart2; @@ -45,14 +43,11 @@ class AnalyzeContinuously extends AnalyzeBase { if (argResults['flutter-repo']) { final PackageDependencyTracker dependencies = new PackageDependencyTracker(); dependencies.checkForConflictingDependencies(repoPackages, dependencies); - - directories = repoRoots; + directories = repoPackages.map((Directory dir) => dir.path).toList(); analysisTarget = 'Flutter repository'; - printTrace('Analyzing Flutter repository:'); - for (String projectPath in repoRoots) { + for (String projectPath in directories) printTrace(' ${fs.path.relative(projectPath)}'); - } } else { directories = [fs.currentDirectory.path]; analysisTarget = fs.currentDirectory.path; @@ -110,14 +105,10 @@ class AnalyzeContinuously extends AnalyzeBase { // Print an analysis summary. String errorsMessage; - int issueCount = errors.length; + final 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) @@ -131,13 +122,10 @@ 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 in $seconds seconds'); + printStatus('$errorsMessage • analyzed $files, $seconds seconds'); if (firstAnalysis && isBenchmarking) { - // We don't want to return a failing exit code based on missing documentation. - issueCount -= undocumentedCount; - - writeBenchmark(analysisTimer, issueCount, undocumentedCount); + writeBenchmark(analysisTimer, issueCount, -1); // TODO(ianh): track members missing dartdocs instead of saying -1 server.dispose().whenComplete(() { exit(issueCount > 0 ? 1 : 0); }); } @@ -145,10 +133,209 @@ 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((AnalysisError error) => error.type == 'TODO'); + fileErrors.errors.removeWhere(_filterError); 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 3614f6ccb5..0ad0646b34 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -3,182 +3,293 @@ // 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/logger.dart'; +import '../base/process.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.repoRoots, - this.repoPackages, { + AnalyzeOnce(ArgResults argResults, 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 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 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; } } } - if (argResults['flutter-repo']) { - // check for conflicting dependencies - final PackageDependencyTracker dependencies = new PackageDependencyTracker(); - dependencies.checkForConflictingDependencies(repoPackages, dependencies); + final bool currentPackage = argResults['current-package'] && (argResults.wasParsed('current-package') || dartFiles.isEmpty); + final bool flutterRepo = argResults['flutter-repo'] || (workingDirectory == null && inRepo(argResults.rest)); - directories.addAll(repoRoots); + // 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.'); - if (argResults.wasParsed('current-package') && argResults['current-package']) { - directories.add(currentDirectory); + 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); + } } - } else { - if (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); } + + if (previewDart2) { + arguments.add('--preview-dart-2'); + } else { + arguments.add('--no-preview-dart-2'); + } + + 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); - } - - // 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'); - } - }); + // determine what all the various .packages files depend on + final PackageDependencyTracker dependencies = new PackageDependencyTracker(); + dependencies.checkForConflictingDependencies(pubSpecDirectories, dependencies); + final Map packages = dependencies.asPackageMap(); Cache.releaseLockEarly(); - // 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'); + if (argResults['preamble']) { + if (dartFiles.length == 1) { + logger.printStatus('Analyzing ${fs.path.relative(dartFiles.first.path)}...'); } else { - throwToolExit(null); + logger.printStatus('Analyzing ${dartFiles.length} files...'); } } + 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 (undocumentedMembers > 0) { - printStatus('No issues found! (ran in ${seconds}s; ' - '$undocumentedMembers public ${ undocumentedMembers == - 1 ? "member lacks" : "members lack" } documentation)'); + if (membersMissingDocumentation > 0) { + printStatus('No analyzer warnings! (ran in ${elapsed}s; $membersMissingDocumentation public ${ membersMissingDocumentation == 1 ? "member lacks" : "members lack" } documentation)'); } else { - printStatus('No issues found! (ran in ${seconds}s)'); + printStatus('No analyzer warnings! (ran in ${elapsed}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 1166835c96..421e9cd918 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -2,215 +2,267 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; -import 'dart:convert'; +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 '../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'); - } - - 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': [] - }); +class AnalysisDriver { + AnalysisDriver(this.options) { + AnalysisEngine.instance.logger = + new _StdLogger(outSink: options.outSink, errorSink: options.errorSink); + _processPlugins(); } - Stream get onAnalyzing => _analyzingController.stream; - Stream get onErrors => _errorsController.stream; + final Set _analyzedSources = new HashSet(); - Future get onExit => _process.exitCode; + AnalysisOptionsProvider analysisOptionsProvider = + new AnalysisOptionsProvider(); - void _sendCommand(String method, Map params) { - final String message = json.encode( { - 'id': (++_id).toString(), - 'method': method, - 'params': params - }); - _process.stdin.writeln(message); - printTrace('==> $message'); - } + file_system.ResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE; - void _handleServerResponse(String line) { - printTrace('<== $line'); + AnalysisContext context; - final dynamic response = json.decode(line); + DriverOptions options; - if (response is Map) { - if (response['event'] != null) { - final String event = response['event']; - final dynamic params = response['params']; + String get sdkDir => options.dartSdkPath ?? cli_util.getSdkPath(); - 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']); + 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; } - 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); + 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; } - 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']); - } + List _getResolvers(InternalAnalysisContext context, + Map> packageMap) { - 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)); - } + // Create our list of resolvers. + final List resolvers = []; - Future dispose() async { - await _analyzingController.close(); - await _errorsController.close(); - return _process?.kill(); - } -} + // 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; -class AnalysisError implements Comparable { - AnalysisError(this.json); - - 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); + resolvers.add(new DartUriResolver(sdk)); } else { - return message; + // 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)); + } + + 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); + } + + 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 _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); } } - @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() { - return - '${severity.toLowerCase().padLeft(7)} $_separator ' - '$messageSentenceFragment $_separator ' - '${fs.path.relative(file)}:$startLine:$startColumn'; - } - - String toLegacyString() { - return '[${severity.toLowerCase()}] $messageSentenceFragment ($file:$startLine:$startColumn)'; + void _processPlugins() { + final List plugins = []; + plugins.addAll(AnalysisEngine.instance.requiredPlugins); + final ExtensionManager manager = new ExtensionManager(); + manager.processPlugins(plugins); + linter.registerLintRules(); } } -class FileAnalysisErrors { - FileAnalysisErrors(this.file, this.errors); +class AnalysisDriverException implements Exception { + AnalysisDriverException([this.message]); - final String file; - final List errors; + final String message; + + @override + String toString() => message == null ? 'Exception' : 'Exception: $message'; +} + +class AnalysisErrorDescription { + AnalysisErrorDescription(this.error, this.line); + + static Directory cwd = fs.currentDirectory.absolute; + + 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; + } + + LineInfo_Location 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); + + @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); + } } 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 4ae4304312..9a02aaf605 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command_runner.dart @@ -383,19 +383,12 @@ class FlutterCommandRunner extends CommandRunner { Cache.flutterRoot ??= _defaultFlutterRoot; } - /// 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'].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)) + 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))) .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 2a67faa7fc..3686de462a 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/dart/analysis.dart'; +import 'package:flutter_tools/src/commands/analyze_continuously.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 new file mode 100644 index 0000000000..c56ff8be38 --- /dev/null +++ b/packages/flutter_tools/test/commands/analyze_duplicate_names_test.dart @@ -0,0 +1,48 @@ +// 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 8bc7b5577d..7de75b8f4c 100644 --- a/packages/flutter_tools/test/commands/analyze_once_test.dart +++ b/packages/flutter_tools/test/commands/analyze_once_test.dart @@ -17,6 +17,7 @@ import '../src/common.dart'; import '../src/context.dart'; void main() { + final String analyzerSeparator = platform.isWindows ? '-' : '•'; group('analyze once', () { @@ -32,11 +33,7 @@ void main() { }); tearDownAll(() { - try { - tempDir?.deleteSync(recursive: true); - } catch (e) { - // ignore errors deleting the temporary directory - } + tempDir?.deleteSync(recursive: true); }); // Create a project to be analyzed @@ -53,7 +50,7 @@ void main() { }, timeout: allowForRemotePubInvocation); // Analyze in the current directory - no arguments - testUsingContext('working directory', () async { + testUsingContext('flutter analyze working directory', () async { await runCommand( command: new AnalyzeCommand(workingDirectory: fs.directory(projectPath)), arguments: ['analyze'], @@ -62,17 +59,17 @@ void main() { }); // Analyze a specific file outside the current directory - testUsingContext('passing one file throws', () async { + testUsingContext('flutter analyze one file', () async { await runCommand( command: new AnalyzeCommand(), arguments: ['analyze', libMain.path], - toolExit: true, - exitMessageContains: 'is not a directory', + statusTextContains: ['No issues found!'], ); }); // Analyze in the current directory - no arguments - testUsingContext('working directory with errors', () async { + testUsingContext('flutter analyze 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. @@ -96,7 +93,22 @@ void main() { statusTextContains: [ 'Analyzing', 'warning $analyzerSeparator The parameter \'onPressed\' is required', - 'info $analyzerSeparator The method \'_incrementCounter\' isn\'t used', + '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', '2 issues found.', ], toolExit: true, @@ -104,7 +116,8 @@ void main() { }); // Analyze in the current directory - no arguments - testUsingContext('working directory with local options', () async { + testUsingContext('flutter analyze 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')); @@ -122,15 +135,15 @@ void main() { statusTextContains: [ 'Analyzing', 'warning $analyzerSeparator The parameter \'onPressed\' is required', - 'info $analyzerSeparator The method \'_incrementCounter\' isn\'t used', - 'info $analyzerSeparator Only throw instances of classes extending either Exception or Error', + '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('no duplicate issues', () async { + testUsingContext('flutter analyze no duplicate issues', () async { final Directory tempDir = fs.systemTempDirectory.createTempSync('analyze_once_test_').absolute; try { @@ -164,6 +177,22 @@ 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'); @@ -221,23 +250,18 @@ 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 catch (e) { + } on ToolExit { 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 8fc417dfdd..5c1e2d4931 100644 --- a/packages/flutter_tools/test/commands/create_test.dart +++ b/packages/flutter_tools/test/commands/create_test.dart @@ -431,13 +431,14 @@ Future _createAndAnalyzeProject( { List unexpectedPaths = const [], bool plugin = false }) async { await _createProject(dir, createArgs, expectedPaths, unexpectedPaths: unexpectedPaths, plugin: plugin); if (plugin) { - await _analyzeProject(dir.path); + await _analyzeProject(dir.path, target: fs.path.join(dir.path, 'lib', 'flutter_project.dart')); + await _analyzeProject(fs.path.join(dir.path, 'example')); } else { await _analyzeProject(dir.path); } } -Future _analyzeProject(String workingDir) async { +Future _analyzeProject(String workingDir, {String target}) async { final String flutterToolsPath = fs.path.absolute(fs.path.join( 'bin', 'flutter_tools.dart', @@ -447,6 +448,8 @@ Future _analyzeProject(String workingDir) async { ..addAll(dartVmFlags) ..add(flutterToolsPath) ..add('analyze'); + if (target != null) + args.add(target); final ProcessResult exec = await Process.run( '$dartSdkPath/bin/dart',