From cbda208b4bfbdd9bd4d9d643950da4f624926beb Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Sun, 22 Jan 2017 16:43:24 -0800 Subject: [PATCH] Reduce the amount of spam from analyze watch. (#7582) See https://github.com/dart-lang/sdk/issues/28463, which I think is a regression. This also fixes the regression introduced by https://codereview.chromium.org/2559773002 whereby we were no longer checking any of the lints. --- .analysis_options | 6 ++- .analysis_options_repo | 7 ++- .../.analysis_options | 4 ++ dev/missing_dependency_tests/.dartignore | 0 examples/stocks/.analysis_options | 3 -- examples/stocks/lib/i18n/.dartignore | 0 .../lib/src/foundation/assertions.dart | 1 + packages/flutter/lib/src/http/.dartignore | 0 .../lib/src/commands/analyze_once.dart | 19 ++----- .../flutter_tools/lib/src/commands/drive.dart | 1 + .../flutter_tools/lib/src/dart/analysis.dart | 51 +++++++++---------- 11 files changed, 42 insertions(+), 50 deletions(-) create mode 100644 dev/missing_dependency_tests/.analysis_options create mode 100644 dev/missing_dependency_tests/.dartignore delete mode 100644 examples/stocks/.analysis_options create mode 100644 examples/stocks/lib/i18n/.dartignore create mode 100644 packages/flutter/lib/src/http/.dartignore diff --git a/.analysis_options b/.analysis_options index 7e257102f47..d32449ebd34 100644 --- a/.analysis_options +++ b/.analysis_options @@ -26,8 +26,10 @@ analyzer: todo: ignore exclude: - 'bin/cache/**' - - 'dev/missing_dependency_tests/**' - - 'packages/flutter_tools/test/data/dart_dependencies_test/**' + # the following two are relative to the stocks example and the flutter package respectively + # see https://github.com/dart-lang/sdk/issues/28463 + - 'lib/i18n/stock_messages_*.dart' + - 'lib/src/http/**' linter: rules: diff --git a/.analysis_options_repo b/.analysis_options_repo index 4cdf048576e..af84489eb3c 100644 --- a/.analysis_options_repo +++ b/.analysis_options_repo @@ -25,10 +25,9 @@ analyzer: strong_mode_down_cast_composite: ignore # allow having TODOs in the code todo: ignore - exclude: - - 'bin/cache/**' - - 'dev/missing_dependency_tests/**' - - 'packages/flutter_tools/test/data/dart_dependencies_test/**' + # `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: diff --git a/dev/missing_dependency_tests/.analysis_options b/dev/missing_dependency_tests/.analysis_options new file mode 100644 index 00000000000..ae6f18d359f --- /dev/null +++ b/dev/missing_dependency_tests/.analysis_options @@ -0,0 +1,4 @@ +analyzer: + exclude: + - '**' + diff --git a/dev/missing_dependency_tests/.dartignore b/dev/missing_dependency_tests/.dartignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/examples/stocks/.analysis_options b/examples/stocks/.analysis_options deleted file mode 100644 index f9e8f30d8fe..00000000000 --- a/examples/stocks/.analysis_options +++ /dev/null @@ -1,3 +0,0 @@ -analyzer: - exclude: - - 'lib/i18n/stock_messages_*.dart' diff --git a/examples/stocks/lib/i18n/.dartignore b/examples/stocks/lib/i18n/.dartignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/flutter/lib/src/foundation/assertions.dart b/packages/flutter/lib/src/foundation/assertions.dart index 7d70fb37fb4..904cb1bcb8a 100644 --- a/packages/flutter/lib/src/foundation/assertions.dart +++ b/packages/flutter/lib/src/foundation/assertions.dart @@ -132,6 +132,7 @@ class FlutterError extends AssertionError { /// /// All sentences in the error should be correctly punctuated (i.e., /// do end the error message with a period). + @override final String message; @override diff --git a/packages/flutter/lib/src/http/.dartignore b/packages/flutter/lib/src/http/.dartignore new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/flutter_tools/lib/src/commands/analyze_once.dart b/packages/flutter_tools/lib/src/commands/analyze_once.dart index a7d07303103..695ae2ddc13 100644 --- a/packages/flutter_tools/lib/src/commands/analyze_once.dart +++ b/packages/flutter_tools/lib/src/commands/analyze_once.dart @@ -73,14 +73,9 @@ class AnalyzeOnce extends AnalyzeBase { pubSpecDirectories.add(currentDirectory); } - // TODO(ianh): Fix the intl package resource generator - // TODO(pq): extract this regexp from the exclude in options - RegExp stockExampleFiles = new RegExp('examples/stocks/lib/i18n/.*\.dart\$'); - if (flutterRepo) { for (Directory dir in repoPackages) { - _collectDartFiles(dir, dartFiles, - exclude: (FileSystemEntity entity) => stockExampleFiles.hasMatch(entity.path)); + _collectDartFiles(dir, dartFiles); pubSpecDirectories.add(dir); } } @@ -164,7 +159,6 @@ class AnalyzeOnce extends AnalyzeBase { for (AnalysisErrorDescription error in errors) { bool shouldIgnore = false; if (error.errorCode.name == 'public_member_api_docs') { - // https://github.com/dart-lang/linter/issues/207 // https://github.com/dart-lang/linter/issues/208 if (isFlutterLibrary(error.source.fullName)) { if (!argResults['dartdocs']) { @@ -175,9 +169,6 @@ class AnalyzeOnce extends AnalyzeBase { shouldIgnore = true; } } - // TODO(ianh): Fix the Dart mojom compiler - if (error.source.fullName.endsWith('.mojom.dart')) - shouldIgnore = true; if (shouldIgnore) continue; printError(error.asString()); @@ -226,18 +217,18 @@ class AnalyzeOnce extends AnalyzeBase { return true; } - List _collectDartFiles(Directory dir, List collected, {FileFilter exclude}) { + List _collectDartFiles(Directory dir, List collected) { // Bail out in case of a .dartignore. - if (fs.isFileSync(path.join(path.dirname(dir.path), '.dartignore'))) + if (fs.isFileSync(path.join(dir.path, '.dartignore'))) return collected; for (FileSystemEntity entity in dir.listSync(recursive: false, followLinks: false)) { - if (isDartFile(entity) && (exclude == null || !exclude(entity))) + if (isDartFile(entity)) collected.add(entity); if (entity is Directory) { String name = path.basename(entity.path); if (!name.startsWith('.') && name != 'packages') - _collectDartFiles(entity, collected, exclude: exclude); + _collectDartFiles(entity, collected); } } diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 81d8d95c73d..b3803bcd2b5 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -84,6 +84,7 @@ class DriveCommand extends RunCommandBase { Device get device => _device; /// Subscription to log messages printed on the device or simulator. + // ignore: cancel_subscriptions StreamSubscription _deviceLogSubscription; int get debugPort => int.parse(argResults['debug-port']); diff --git a/packages/flutter_tools/lib/src/dart/analysis.dart b/packages/flutter_tools/lib/src/dart/analysis.dart index 38a6c7718f7..759d328e4f1 100644 --- a/packages/flutter_tools/lib/src/dart/analysis.dart +++ b/packages/flutter_tools/lib/src/dart/analysis.dart @@ -17,6 +17,7 @@ import 'package:analyzer/src/generated/java_io.dart'; // ignore: implementation_ 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 @@ -28,6 +29,12 @@ import '../base/file_system.dart' hide IOSink; import '../base/io.dart'; class AnalysisDriver { + AnalysisDriver(this.options) { + AnalysisEngine.instance.logger = + new _StdLogger(outSink: options.outSink, errorSink: options.errorSink); + _processPlugins(); + } + Set _analyzedSources = new HashSet(); AnalysisOptionsProvider analysisOptionsProvider = @@ -38,11 +45,6 @@ class AnalysisDriver { AnalysisContext context; DriverOptions options; - AnalysisDriver(this.options) { - AnalysisEngine.instance.logger = - new _StdLogger(outSink: options.outSink, errorSink: options.errorSink); - _processPlugins(); - } String get sdkDir => options.dartSdkPath ?? cli_util.getSdkDir().path; @@ -51,9 +53,8 @@ class AnalysisDriver { List errors = []; for (AnalysisErrorInfo info in infos) { for (AnalysisError error in info.errors) { - if (!_isFiltered(error)) { + if (!_isFiltered(error)) errors.add(new AnalysisErrorDescription(error, info.lineInfo)); - } } } return errors; @@ -61,7 +62,8 @@ class AnalysisDriver { List _analyze(Iterable files) { context = AnalysisEngine.instance.createAnalysisContext(); - _processAnalysisOptions(context, options); + _processAnalysisOptions(); + context.analysisOptions = options; PackageInfo packageInfo = new PackageInfo(options.packageMap); List resolvers = _getResolvers(context, packageInfo.asMap()); context.sourceFactory = @@ -94,7 +96,6 @@ class AnalysisDriver { List _getResolvers(InternalAnalysisContext context, Map> packageMap) { - // Create our list of resolvers. List resolvers = []; @@ -106,9 +107,6 @@ class AnalysisDriver { // Fail fast if no URI mappings are found. assert(sdk.libraryMap.size() > 0); sdk.analysisOptions = context.analysisOptions; - // TODO(pq): re-enable once we have a proper story for SDK summaries - // in the presence of embedders (https://github.com/dart-lang/sdk/issues/26467). - sdk.useSummary = false; resolvers.add(new DartUriResolver(sdk)); } else { @@ -141,17 +139,15 @@ class AnalysisDriver { return processor != null && processor.severity == null; } - void _processAnalysisOptions( - AnalysisContext context, AnalysisOptions analysisOptions) { + void _processAnalysisOptions() { String optionsPath = options.analysisOptionsFile; if (optionsPath != null) { file_system.File file = PhysicalResourceProvider.INSTANCE.getFile(optionsPath); Map optionMap = analysisOptionsProvider.getOptionsFromFile(file); - if (optionMap != null) { - applyToAnalysisOptions(context.analysisOptions, optionMap); - } + if (optionMap != null) + applyToAnalysisOptions(options, optionMap); } } @@ -160,23 +156,26 @@ class AnalysisDriver { plugins.addAll(AnalysisEngine.instance.requiredPlugins); ExtensionManager manager = new ExtensionManager(); manager.processPlugins(plugins); + linter.registerLintRules(); } } class AnalysisDriverException implements Exception { - final String message; AnalysisDriverException([this.message]); + 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; - AnalysisErrorDescription(this.error, this.line); ErrorCode get errorCode => error.errorCode; @@ -203,7 +202,6 @@ class AnalysisErrorDescription { } class DriverOptions extends AnalysisOptionsImpl { - DriverOptions() { // Set defaults. lint = true; @@ -223,9 +221,6 @@ class DriverOptions extends AnalysisOptionsImpl { /// The path to analysis options. String analysisOptionsFile; - /// Analysis options map. - Map analysisOptions; - /// Out sink for logging. IOSink outSink = stdout; @@ -247,18 +242,19 @@ class PackageInfo { } Packages _packages; - HashMap> _map = - new HashMap>(); Map> asMap() => _map; + HashMap> _map = + new HashMap>(); Packages asPackages() => _packages; } class _StdLogger extends Logger { + _StdLogger({this.outSink, this.errorSink}); + final IOSink outSink; final IOSink errorSink; - _StdLogger({this.outSink, this.errorSink}); @override void logError(String message, [Exception exception]) => @@ -267,6 +263,7 @@ class _StdLogger extends Logger { @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); + if (message != 'No definition of type FutureOr') + outSink.writeln(message); } }