From 8f4db8f51441fed445b15a656f0feea05cd8174a Mon Sep 17 00:00:00 2001 From: John Messerly Date: Thu, 11 Jun 2015 07:47:12 -0700 Subject: [PATCH] expose strong checker API, for use by analyzer_cli most of the changes here are around making options sane(r), which fixes #204 R=vsm@google.com Review URL: https://codereview.chromium.org/1174643003. --- pkg/dev_compiler/bin/devc.dart | 5 +- pkg/dev_compiler/bin/edit_files.dart | 5 +- pkg/dev_compiler/lib/config.dart | 26 -- pkg/dev_compiler/lib/devc.dart | 49 ++-- .../lib/src/analysis_context.dart | 44 ++-- pkg/dev_compiler/lib/src/checker/checker.dart | 6 +- .../lib/src/checker/resolver.dart | 8 +- pkg/dev_compiler/lib/src/checker/rules.dart | 4 +- .../lib/src/codegen/code_generator.dart | 8 +- .../lib/src/codegen/js_codegen.dart | 6 +- .../lib/src/dependency_graph.dart | 4 +- pkg/dev_compiler/lib/src/info.dart | 22 +- pkg/dev_compiler/lib/src/options.dart | 240 +++++------------- pkg/dev_compiler/lib/src/report.dart | 28 +- pkg/dev_compiler/lib/src/summary.dart | 2 +- pkg/dev_compiler/lib/src/testing.dart | 239 +++++++---------- pkg/dev_compiler/lib/src/utils.dart | 2 +- pkg/dev_compiler/lib/strong_mode.dart | 211 +++++++++++++++ .../test/checker/inferred_type_test.dart | 2 +- .../test/checker/self_host_test.dart | 5 +- pkg/dev_compiler/test/codegen_test.dart | 13 +- .../test/dependency_graph_test.dart | 23 +- pkg/dev_compiler/test/end_to_end_test.dart | 3 +- pkg/dev_compiler/test/report_test.dart | 20 +- 24 files changed, 507 insertions(+), 468 deletions(-) delete mode 100644 pkg/dev_compiler/lib/config.dart create mode 100644 pkg/dev_compiler/lib/strong_mode.dart diff --git a/pkg/dev_compiler/bin/devc.dart b/pkg/dev_compiler/bin/devc.dart index 255a9b936307..91dccb9a503f 100755 --- a/pkg/dev_compiler/bin/devc.dart +++ b/pkg/dev_compiler/bin/devc.dart @@ -23,13 +23,14 @@ void main(List args) { var options = parseOptions(args); if (options.help) _showUsageAndExit(); - if (!options.useMockSdk && options.dartSdkPath == null) { + var srcOpts = options.sourceOptions; + if (!srcOpts.useMockSdk && srcOpts.dartSdkPath == null) { print('Could not automatically find dart sdk path.'); print('Please pass in explicitly: --dart-sdk '); exit(1); } - if (options.entryPointFile == null) { + if (srcOpts.entryPointFile == null) { print('Expected filename.'); _showUsageAndExit(); } diff --git a/pkg/dev_compiler/bin/edit_files.dart b/pkg/dev_compiler/bin/edit_files.dart index 384a2e4d9cd3..1d95b4442353 100644 --- a/pkg/dev_compiler/bin/edit_files.dart +++ b/pkg/dev_compiler/bin/edit_files.dart @@ -135,7 +135,7 @@ void main(List argv) { } var filename = args.rest.first; - var options = new CompilerOptions( + var options = new SourceResolverOptions( dartSdkPath: sdkDir.path, useMultiPackage: args['use-multi-package'], packageRoot: args['package-root'], @@ -150,7 +150,8 @@ void main(List argv) { ? new RegExp(args['include-pattern']) : null; - var context = createAnalysisContext(options); + var context = + createAnalysisContextWithSources(new StrongModeOptions(), options); var visitor = new EditFileSummaryVisitor(context, args['level'], args['checkout-files-executable'], args['checkout-files-arg'], includePattern, excludePattern); diff --git a/pkg/dev_compiler/lib/config.dart b/pkg/dev_compiler/lib/config.dart deleted file mode 100644 index b7b18ec9f904..000000000000 --- a/pkg/dev_compiler/lib/config.dart +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -/// Configuration of DDC rule set. -library dev_compiler.config; - -// Options shared by the compiler and runtime. -class TypeOptions { - // A list of non-nullable type names (e.g., 'int') - static const List NONNULLABLE_TYPES = const []; - final List nonnullableTypes; - - TypeOptions({this.nonnullableTypes: NONNULLABLE_TYPES}); -} - -List optionsToList(String option, - {List defaultValue: const []}) { - if (option == null) { - return defaultValue; - } else if (option.isEmpty) { - return []; - } else { - return option.split(','); - } -} diff --git a/pkg/dev_compiler/lib/devc.dart b/pkg/dev_compiler/lib/devc.dart index 6972be3271ef..2e3687bb224a 100644 --- a/pkg/dev_compiler/lib/devc.dart +++ b/pkg/dev_compiler/lib/devc.dart @@ -53,7 +53,7 @@ abstract class AbstractCompiler { class Compiler implements AbstractCompiler { final CompilerOptions options; final AnalysisContext context; - final CheckerReporter _reporter; + final CompilerReporter _reporter; final TypeRules rules; final CodeChecker _checker; final SourceNode _entryNode; @@ -63,8 +63,12 @@ class Compiler implements AbstractCompiler { bool _failure = false; factory Compiler(CompilerOptions options, - {AnalysisContext context, CheckerReporter reporter}) { - if (context == null) context = createAnalysisContext(options); + {AnalysisContext context, CompilerReporter reporter}) { + var strongOpts = options.strongOptions; + var sourceOpts = options.sourceOptions; + if (context == null) { + context = createAnalysisContextWithSources(strongOpts, sourceOpts); + } if (reporter == null) { reporter = options.dumpInfo @@ -72,15 +76,16 @@ class Compiler implements AbstractCompiler { : new LogReporter(context, useColors: options.useColors); } var graph = new SourceGraph(context, reporter, options); - var rules = new RestrictedRules(context.typeProvider, options: options); - var checker = new CodeChecker(rules, reporter, options); + var rules = new RestrictedRules(context.typeProvider, + options: options.strongOptions); + var checker = new CodeChecker(rules, reporter, strongOpts); - var inputFile = options.entryPointFile; + var inputFile = sourceOpts.entryPointFile; var inputUri = inputFile.startsWith('dart:') || inputFile.startsWith('package:') ? Uri.parse(inputFile) - : new Uri.file(path.absolute(options.useImplicitHtml - ? ResolverOptions.implicitHtmlFile + : new Uri.file(path.absolute(sourceOpts.useImplicitHtml + ? SourceResolverOptions.implicitHtmlFile : inputFile)); var entryNode = graph.nodeFromUri(inputUri); @@ -90,7 +95,7 @@ class Compiler implements AbstractCompiler { Compiler._(this.options, this.context, this._reporter, this.rules, this._checker, this._entryNode) { - if (options.outputDir != null) { + if (outputDir != null) { _generators.add(new JSGenerator(this)); } // TODO(sigmund): refactor to support hashing of the dart output? @@ -98,6 +103,7 @@ class Compiler implements AbstractCompiler { } Uri get entryPointUri => _entryNode.uri; + String get outputDir => options.codegenOptions.outputDir; bool _buildSource(SourceNode node) { if (node is HtmlSourceNode) { @@ -116,7 +122,7 @@ class Compiler implements AbstractCompiler { } void _buildHtmlFile(HtmlSourceNode node) { - if (options.outputDir == null) return; + if (outputDir == null) return; var uri = node.source.uri; _reporter.enterHtml(uri); var output = generateEntryHtml(node, options); @@ -126,7 +132,7 @@ class Compiler implements AbstractCompiler { } _reporter.leaveHtml(); var filename = path.basename(node.uri.path); - String outputFile = path.join(options.outputDir, filename); + String outputFile = path.join(outputDir, filename); new File(outputFile).writeAsStringSync(output); } @@ -134,10 +140,10 @@ class Compiler implements AbstractCompiler { // ResourceSourceNodes files that just need to be copied over to the output // location. These can be external dependencies or pieces of the // dev_compiler runtime. - if (options.outputDir == null) return; + if (outputDir == null) return; var filepath = resourceOutputPath(node.uri, _entryNode.uri); assert(filepath != null); - filepath = path.join(options.outputDir, filepath); + filepath = path.join(outputDir, filepath); var dir = path.dirname(filepath); new Directory(dir).createSync(recursive: true); new File.fromUri(node.source.uri).copySync(filepath); @@ -182,7 +188,7 @@ class Compiler implements AbstractCompiler { } if (failureInLib) { _failure = true; - if (!options.forceCompile) return; + if (!options.codegenOptions.forceCompile) return; } for (var cg in _generators) { @@ -221,7 +227,7 @@ class Compiler implements AbstractCompiler { var time = (clock.elapsedMilliseconds / 1000).toStringAsFixed(2); _log.fine('Compiled ${_libraries.length} libraries in ${time} s\n'); return new CheckerResults( - _libraries, rules, _failure || options.forceCompile); + _libraries, rules, _failure || options.codegenOptions.forceCompile); } void _runAgain() { @@ -245,7 +251,7 @@ class Compiler implements AbstractCompiler { var result = (_reporter as SummaryReporter).result; if (!options.serverMode) print(summaryToString(result)); var filepath = options.serverMode - ? path.join(options.outputDir, 'messages.json') + ? path.join(outputDir, 'messages.json') : options.dumpInfoFile; if (filepath == null) return; new File(filepath).writeAsStringSync(JSON.encode(result.toJsonMap())); @@ -260,15 +266,15 @@ class CompilerServer { final String _entryPath; factory CompilerServer(CompilerOptions options) { - var entryPath = path.basename(options.entryPointFile); + var entryPath = path.basename(options.sourceOptions.entryPointFile); var extension = path.extension(entryPath); - if (extension != '.html' && !options.useImplicitHtml) { + if (extension != '.html' && !options.sourceOptions.useImplicitHtml) { print('error: devc in server mode requires an HTML or Dart entry point.'); exit(1); } // TODO(sigmund): allow running without a dir, but keep output in memory? - var outDir = options.outputDir; + var outDir = options.codegenOptions.outputDir; if (outDir == null) { print('error: devc in server mode also requires specifying and ' 'output location for generated code.'); @@ -283,8 +289,9 @@ class CompilerServer { CompilerServer._( Compiler compiler, this.outDir, this.host, this.port, String entryPath) : this.compiler = compiler, - this._entryPath = compiler.options.useImplicitHtml - ? ResolverOptions.implicitHtmlFile + // TODO(jmesserly): this logic is duplicated in a few places + this._entryPath = compiler.options.sourceOptions.useImplicitHtml + ? SourceResolverOptions.implicitHtmlFile : entryPath; Future start() async { diff --git a/pkg/dev_compiler/lib/src/analysis_context.dart b/pkg/dev_compiler/lib/src/analysis_context.dart index 4b8d3106eadf..b41c79248c08 100644 --- a/pkg/dev_compiler/lib/src/analysis_context.dart +++ b/pkg/dev_compiler/lib/src/analysis_context.dart @@ -13,23 +13,40 @@ import 'package:analyzer/src/generated/source.dart' show DartUriResolver; import 'package:analyzer/src/generated/source_io.dart'; import 'package:path/path.dart' as path; +import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; + import 'checker/resolver.dart'; import 'dart_sdk.dart'; import 'multi_package_resolver.dart'; import 'options.dart'; -/// Creates an AnalysisContext with dev_compiler type rules and inference. +/// Creates an [AnalysisContext] with dev_compiler type rules and inference, +/// using [createSourceFactory] to set up its [SourceFactory]. +AnalysisContext createAnalysisContextWithSources( + StrongModeOptions strongOptions, SourceResolverOptions srcOptions, + {DartUriResolver sdkResolver, List fileResolvers}) { + var srcFactory = createSourceFactory(srcOptions, + sdkResolver: sdkResolver, fileResolvers: fileResolvers); + return createAnalysisContext(strongOptions)..sourceFactory = srcFactory; +} + +/// Creates an analysis context that contains our restricted typing rules. +AnalysisContext createAnalysisContext(StrongModeOptions options) { + AnalysisContextImpl res = AnalysisEngine.instance.createAnalysisContext(); + res.libraryResolverFactory = + (context) => new LibraryResolverWithInference(context, options); + return res; +} + +/// Creates a SourceFactory configured by the [options]. /// /// Use [options.useMockSdk] to specify the SDK mode, or use [sdkResolver] /// to entirely override the DartUriResolver. /// /// If supplied, [fileResolvers] will override the default `file:` and /// `package:` URI resolvers. -/// -AnalysisContext createAnalysisContext(CompilerOptions options, +SourceFactory createSourceFactory(SourceResolverOptions options, {DartUriResolver sdkResolver, List fileResolvers}) { - var context = _initContext(options); - var sdkResolver = options.useMockSdk ? createMockSdkResolver(mockSdkSources) : createSdkPathResolver(options.dartSdkPath); @@ -49,8 +66,7 @@ AnalysisContext createAnalysisContext(CompilerOptions options, : new PackageUriResolver([new JavaFile(options.packageRoot)])); } resolvers.addAll(fileResolvers); - context.sourceFactory = new SourceFactory(resolvers); - return context; + return new SourceFactory(resolvers); } /// Creates a [DartUriResolver] that uses a mock 'dart:' library contents. @@ -61,8 +77,8 @@ DartUriResolver createMockSdkResolver(Map mockSources) => DartUriResolver createSdkPathResolver(String sdkPath) => new DartUriResolver(new DirectoryBasedDartSdk(new JavaFile(sdkPath))); -UriResolver _createImplicitEntryResolver(ResolverOptions options) { - var entry = path.absolute(ResolverOptions.implicitHtmlFile); +UriResolver _createImplicitEntryResolver(SourceResolverOptions options) { + var entry = path.absolute(SourceResolverOptions.implicitHtmlFile); var src = path.absolute(options.entryPointFile); var provider = new MemoryResourceProvider(); provider.newFile( @@ -83,13 +99,3 @@ class ExistingSourceUriResolver implements UriResolver { } Uri restoreAbsolute(Source source) => resolver.restoreAbsolute(source); } - -/// Creates an analysis context that contains our restricted typing rules. -AnalysisContext _initContext(ResolverOptions options) { - var analysisOptions = new AnalysisOptionsImpl()..cacheSize = 512; - AnalysisContextImpl res = AnalysisEngine.instance.createAnalysisContext(); - res.analysisOptions = analysisOptions; - res.libraryResolverFactory = - (context) => new LibraryResolverWithInference(context, options); - return res; -} diff --git a/pkg/dev_compiler/lib/src/checker/checker.dart b/pkg/dev_compiler/lib/src/checker/checker.dart index 72283659a9aa..3c44e81204ee 100644 --- a/pkg/dev_compiler/lib/src/checker/checker.dart +++ b/pkg/dev_compiler/lib/src/checker/checker.dart @@ -11,9 +11,9 @@ import 'package:analyzer/src/generated/scanner.dart' show Token, TokenType; import 'package:logging/logging.dart' as logger; import 'package:dev_compiler/src/info.dart'; -import 'package:dev_compiler/src/options.dart'; import 'package:dev_compiler/src/report.dart' show CheckerReporter; import 'package:dev_compiler/src/utils.dart' show getMemberType; +import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; import 'rules.dart'; /// Checks for overriding declarations of fields and methods. This is used to @@ -24,7 +24,7 @@ class _OverrideChecker { final TypeRules _rules; final CheckerReporter _reporter; final bool _inferFromOverrides; - _OverrideChecker(this._rules, this._reporter, CompilerOptions options) + _OverrideChecker(this._rules, this._reporter, StrongModeOptions options) : _inferFromOverrides = options.inferFromOverrides; void check(ClassDeclaration node) { @@ -342,7 +342,7 @@ class CodeChecker extends RecursiveAstVisitor { bool get failure => _failure || _overrideChecker._failure; CodeChecker( - TypeRules rules, CheckerReporter reporter, CompilerOptions options) + TypeRules rules, CheckerReporter reporter, StrongModeOptions options) : _rules = rules, _reporter = reporter, _overrideChecker = new _OverrideChecker(rules, reporter, options); diff --git a/pkg/dev_compiler/lib/src/checker/resolver.dart b/pkg/dev_compiler/lib/src/checker/resolver.dart index 473923ffaaca..8df623c248cc 100644 --- a/pkg/dev_compiler/lib/src/checker/resolver.dart +++ b/pkg/dev_compiler/lib/src/checker/resolver.dart @@ -17,8 +17,8 @@ import 'package:analyzer/src/generated/utilities_collection.dart' show DirectedGraph; import 'package:logging/logging.dart' as logger; -import 'package:dev_compiler/src/options.dart'; import 'package:dev_compiler/src/utils.dart'; +import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; final _log = new logger.Logger('dev_compiler.src.resolver'); @@ -26,7 +26,7 @@ final _log = new logger.Logger('dev_compiler.src.resolver'); /// on the value of the initializer, and on fields and methods based on /// overridden members in super classes. class LibraryResolverWithInference extends LibraryResolver { - final ResolverOptions _options; + final StrongModeOptions _options; LibraryResolverWithInference(context, this._options) : super(context); @@ -213,7 +213,7 @@ class LibraryResolverWithInference extends LibraryResolver { continue; } - // When field is final and overriden getter is dynamic, we can infer from + // When field is final and overridden getter is dynamic, we can infer from // the RHS without breaking subtyping rules (return type is covariant). if (type.returnType.isDynamic) { if (variables.isFinal && variable.initializer != null) { @@ -356,7 +356,7 @@ class RestrictedResolverVisitor extends ResolverVisitor { final _visitedInitializers = new Set(); RestrictedResolverVisitor(Library library, Source source, - TypeProvider typeProvider, ResolverOptions options) + TypeProvider typeProvider, StrongModeOptions options) : _typeProvider = typeProvider, super.con1(library, source, typeProvider, typeAnalyzerFactory: RestrictedStaticTypeAnalyzer.constructor); diff --git a/pkg/dev_compiler/lib/src/checker/rules.dart b/pkg/dev_compiler/lib/src/checker/rules.dart index 72dce22c2634..09ebb228de7f 100644 --- a/pkg/dev_compiler/lib/src/checker/rules.dart +++ b/pkg/dev_compiler/lib/src/checker/rules.dart @@ -9,8 +9,8 @@ import 'package:analyzer/src/generated/element.dart'; import 'package:analyzer/src/generated/resolver.dart'; import 'package:dev_compiler/src/info.dart'; -import 'package:dev_compiler/src/options.dart'; import 'package:dev_compiler/src/utils.dart' as utils; +import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; abstract class TypeRules { final TypeProvider provider; @@ -116,7 +116,7 @@ typedef void MissingTypeReporter(Expression expr); class RestrictedRules extends TypeRules { MissingTypeReporter reportMissingType = null; - final RulesOptions options; + final StrongModeOptions options; final List _nonnullableTypes; DownwardsInference inferrer; diff --git a/pkg/dev_compiler/lib/src/codegen/code_generator.dart b/pkg/dev_compiler/lib/src/codegen/code_generator.dart index 1d4526a9b52c..033833ead97f 100644 --- a/pkg/dev_compiler/lib/src/codegen/code_generator.dart +++ b/pkg/dev_compiler/lib/src/codegen/code_generator.dart @@ -16,7 +16,7 @@ import 'package:dev_compiler/devc.dart' show AbstractCompiler; import 'package:dev_compiler/src/info.dart'; import 'package:dev_compiler/src/utils.dart' show canonicalLibraryName; import 'package:dev_compiler/src/checker/rules.dart'; -import 'package:dev_compiler/src/options.dart'; +import 'package:dev_compiler/src/options.dart' show CodegenOptions; abstract class CodeGenerator { final AbstractCompiler compiler; @@ -24,15 +24,15 @@ abstract class CodeGenerator { final Uri root; final TypeRules rules; final AnalysisContext context; - final CompilerOptions options; + final CodegenOptions options; CodeGenerator(AbstractCompiler compiler) : compiler = compiler, - outDir = path.absolute(compiler.options.outputDir), + outDir = path.absolute(compiler.options.codegenOptions.outputDir), root = compiler.entryPointUri, rules = compiler.rules, context = compiler.context, - options = compiler.options; + options = compiler.options.codegenOptions; /// Return a hash, if any, that can be used for caching purposes. When two /// invocations to this function return the same hash, the underlying diff --git a/pkg/dev_compiler/lib/src/codegen/js_codegen.dart b/pkg/dev_compiler/lib/src/codegen/js_codegen.dart index ea4c24d84ea4..20a1eccf4d49 100644 --- a/pkg/dev_compiler/lib/src/codegen/js_codegen.dart +++ b/pkg/dev_compiler/lib/src/codegen/js_codegen.dart @@ -26,7 +26,7 @@ import 'package:dev_compiler/src/js/js_ast.dart' show js; import 'package:dev_compiler/devc.dart' show AbstractCompiler; import 'package:dev_compiler/src/checker/rules.dart'; import 'package:dev_compiler/src/info.dart'; -import 'package:dev_compiler/src/options.dart'; +import 'package:dev_compiler/src/options.dart' show CodegenOptions; import 'package:dev_compiler/src/utils.dart'; import 'code_generator.dart'; @@ -51,7 +51,7 @@ const DSEND = 'dsend'; class JSCodegenVisitor extends GeneralizingAstVisitor { final AbstractCompiler compiler; - final CompilerOptions options; + final CodegenOptions options; final TypeRules rules; final LibraryInfo libraryInfo; @@ -99,7 +99,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { JSCodegenVisitor(AbstractCompiler compiler, this.libraryInfo, this._extensionTypes, this._fieldsNeedingStorage) : compiler = compiler, - options = compiler.options, + options = compiler.options.codegenOptions, rules = compiler.rules, root = compiler.entryPointUri { _loader = new ModuleItemLoadOrder(_emitModuleItem); diff --git a/pkg/dev_compiler/lib/src/dependency_graph.dart b/pkg/dev_compiler/lib/src/dependency_graph.dart index 99e7d0e77aa8..12d5eb9c4a67 100644 --- a/pkg/dev_compiler/lib/src/dependency_graph.dart +++ b/pkg/dev_compiler/lib/src/dependency_graph.dart @@ -44,7 +44,7 @@ class SourceGraph { /// Analyzer used to resolve source files. final AnalysisContext _context; - final CheckerReporter _reporter; + final CompilerReporter _reporter; final CompilerOptions _options; SourceGraph(this._context, this._reporter, this._options) { @@ -78,7 +78,7 @@ class SourceGraph { }); } - List get resources => _options.resources; + List get resources => _options.sourceOptions.resources; } /// A node in the import graph representing a source file. diff --git a/pkg/dev_compiler/lib/src/info.dart b/pkg/dev_compiler/lib/src/info.dart index 11dbf598f88a..e250135429a9 100644 --- a/pkg/dev_compiler/lib/src/info.dart +++ b/pkg/dev_compiler/lib/src/info.dart @@ -6,8 +6,6 @@ /// emitters to generate code. library dev_compiler.src.info; -import 'dart:mirrors'; - import 'package:analyzer/src/generated/ast.dart'; import 'package:analyzer/src/generated/element.dart'; import 'package:analyzer/src/generated/error.dart' as analyzer; @@ -410,7 +408,7 @@ abstract class InvalidOverride extends StaticError { /// Type (class or interface) that provides the base declaration. final InterfaceType base; - /// Actual type of the overriden member. + /// Actual type of the overridden member. final DartType subType; /// Actual type of the base member. @@ -514,24 +512,6 @@ class InvalidSuperInvocation extends StaticError { "(see http://goo.gl/q1T4BB): $node"; } -/// Automatically infer list of types by scanning this library using mirrors. -final List infoTypes = () { - var allTypes = new Set(); - var baseTypes = new Set(); - var infoMirror = reflectClass(StaticInfo); - var libMirror = infoMirror.owner as LibraryMirror; - var declarations = libMirror.declarations.values; - for (ClassMirror cls in declarations.where((d) => d is ClassMirror)) { - if (cls.isSubtypeOf(infoMirror)) { - allTypes.add(cls); - baseTypes.add(cls.superclass); - } - } - allTypes.removeAll(baseTypes); - return new List.from(allTypes.map((mirror) => mirror.reflectedType)) - ..sort((t1, t2) => '$t1'.compareTo('$t2')); -}(); - class AnalyzerError extends Message { factory AnalyzerError.from(analyzer.AnalysisError error) { var severity = error.errorCode.type.severity; diff --git a/pkg/dev_compiler/lib/src/options.dart b/pkg/dev_compiler/lib/src/options.dart index 7916e58753eb..994f74a90db7 100644 --- a/pkg/dev_compiler/lib/src/options.dart +++ b/pkg/dev_compiler/lib/src/options.dart @@ -9,17 +9,20 @@ import 'dart:io'; import 'package:args/args.dart'; import 'package:cli_util/cli_util.dart' show getSdkDir; -import 'package:dev_compiler/config.dart'; import 'package:logging/logging.dart' show Level; import 'package:path/path.dart' as path; import 'package:yaml/yaml.dart'; -/// Options used by our checker. -// TODO(jmesserly): move useMultiPackage/packageRoot to CompilerOptions. -class ResolverOptions { +import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; + +/// Options used to set up Source URI resolution in the analysis context. +class SourceResolverOptions { /// Whether to resolve 'package:' uris using the multi-package resolver. final bool useMultiPackage; + /// Custom URI mappings, such as "dart:foo" -> "path/to/foo.dart" + final Map customUrlMappings; + /// Package root when resolving 'package:' urls the standard way. final String packageRoot; @@ -29,70 +32,51 @@ class ResolverOptions { /// List of additional non-Dart resources to resolve and serve. final List resources; - /// Whether to infer return types and field types from overriden members. - final bool inferFromOverrides; - static const inferFromOverridesDefault = true; - - /// Whether to infer types for consts and fields by looking at initializers on - /// the RHS. For example, in a constant declaration like: - /// - /// const A = B; - /// - /// We can infer the type of `A` based on the type of `B`. - /// - /// The inference algorithm determines what variables depend on others, and - /// computes types by visiting the variable dependency graph in topological - /// order. This ensures that the inferred type is deterministic when applying - /// inference on library cycles. - /// - /// When this feature is turned off, we don't use the type of `B` to infer the - /// type of `A`, even if `B` has a declared type. - final bool inferTransitively; - static const inferTransitivelyDefault = true; - - /// Restrict inference of fields and top-levels to those that are final and - /// const. - final bool onlyInferConstsAndFinalFields; - static const onlyInferConstAndFinalFieldsDefault = false; - /// File where to start compilation from. + // TODO(jmesserly): this is used to configure SourceFactory resolvers only + // when [useImplicitHtml] is set. Probably useImplicitHtml should be factored + // out into ServerOptions or something along those lines. final String entryPointFile; // True if the resolver should implicitly provide an html entry point. final bool useImplicitHtml; static const String implicitHtmlFile = 'index.html'; - ResolverOptions({this.useMultiPackage: false, this.packageRoot: 'packages/', - this.packagePaths: const [], this.resources: const [], - this.inferFromOverrides: inferFromOverridesDefault, - this.inferTransitively: inferTransitivelyDefault, - this.onlyInferConstsAndFinalFields: onlyInferConstAndFinalFieldsDefault, - this.entryPointFile: null, this.useImplicitHtml: false}); -} - -// TODO(vsm): Merge RulesOptions and TypeOptions -/// Options used by our RestrictedRules. -class RulesOptions extends TypeOptions { - /// Whether to infer types downwards from local context - final bool inferDownwards; - static const inferDownwardsDefault = true; + /// Whether to use a mock-sdk during compilation. + final bool useMockSdk; - /// Whether to inject casts between Dart assignable types. - final bool relaxedCasts; + /// Path to the dart-sdk. Null if `useMockSdk` is true or if the path couldn't + /// be determined + final String dartSdkPath; - RulesOptions( - {this.inferDownwards: inferDownwardsDefault, this.relaxedCasts: true}); + const SourceResolverOptions({this.useMockSdk: false, this.dartSdkPath, + this.useMultiPackage: false, this.customUrlMappings: const {}, + this.packageRoot: 'packages/', this.packagePaths: const [], + this.resources: const [], this.entryPointFile: null, + this.useImplicitHtml: false}); } -class JSCodeOptions { +// TODO(jmesserly): refactor all codegen options here. +class CodegenOptions { /// Whether to emit the source map files. final bool emitSourceMaps; - JSCodeOptions({this.emitSourceMaps: true}); + /// Whether to force compilation of code with static errors. + final bool forceCompile; + + /// Output directory for generated code. + final String outputDir; + + const CodegenOptions( + {this.emitSourceMaps: true, this.forceCompile: false, this.outputDir}); } -/// General options used by the dev compiler. -class CompilerOptions implements RulesOptions, ResolverOptions, JSCodeOptions { +/// General options used by the dev compiler and server. +class CompilerOptions { + final StrongModeOptions strongOptions; + final SourceResolverOptions sourceOptions; + final CodegenOptions codegenOptions; + /// Whether to check the sdk libraries. final bool checkSdk; @@ -103,37 +87,18 @@ class CompilerOptions implements RulesOptions, ResolverOptions, JSCodeOptions { /// summary information (only used if [dumpInfo] is true). final String dumpInfoFile; - /// Whether to force compilation of code with static errors. - final bool forceCompile; - - /// Output directory for generated code. - final String outputDir; - /// Whether to use colors when interacting on the console. final bool useColors; /// Whether the user asked for help. final bool help; - /// Whether to use a mock-sdk during compilation. - final bool useMockSdk; - - /// Path to the dart-sdk. Null if `useMockSdk` is true or if the path couldn't - /// be determined - final String dartSdkPath; - /// Minimum log-level reported on the command-line. final Level logLevel; - /// File where to start compilation from. - final String entryPointFile; - /// Whether to run as a development server. final bool serverMode; - /// Whether to create an implicit HTML entry file in server mode. - final bool useImplicitHtml; - /// Whether to enable hash-based caching of files. final bool enableHashing; @@ -143,75 +108,18 @@ class CompilerOptions implements RulesOptions, ResolverOptions, JSCodeOptions { /// Host name or address for HTTP server when [serverMode] is on. final String host; - /// Whether to inject casts between Dart assignable types. - @override - final bool relaxedCasts; - - /// Whether to resolve 'package:' uris using the multi-package resolver. - @override - final bool useMultiPackage; - - /// Package root when resolving 'package:' urls the standard way. - @override - final String packageRoot; - - /// List of paths used for the multi-package resolver. - @override - final List packagePaths; - - /// List of additional non-Dart resources to resolve and serve. - @override - final List resources; - - /// Whether to infer types downwards from local context - @override - final bool inferDownwards; - - /// Whether to infer return types and field types from overriden members. - @override - final bool inferFromOverrides; - - /// Whether to infer types for consts and static fields by looking at - /// identifiers on the RHS. - @override - final bool inferTransitively; - - /// Restrict inference of fields and top-levels to those that are final and - /// const. - @override - final bool onlyInferConstsAndFinalFields; - - /// List of non-nullable types. - @override - final List nonnullableTypes; - - /// Whether to emit the source map files. - @override - final bool emitSourceMaps; - /// Location for runtime files, such as `dart_runtime.js`. By default this is /// inferred to be under `lib/runtime/` in the location of the `dev_compiler` /// package (if we can infer where that is located). final String runtimeDir; - /// Custom URI mappings, such as "dart:foo" -> "path/to/foo.dart" - final Map customUrlMappings; - - CompilerOptions({this.checkSdk: false, this.dumpInfo: false, - this.dumpInfoFile, this.forceCompile: false, this.outputDir, - this.useColors: true, this.relaxedCasts: true, - this.useMultiPackage: false, this.packageRoot: 'packages/', - this.packagePaths: const [], this.resources: const [], - this.inferDownwards: RulesOptions.inferDownwardsDefault, - this.inferFromOverrides: ResolverOptions.inferFromOverridesDefault, - this.inferTransitively: ResolverOptions.inferTransitivelyDefault, - this.onlyInferConstsAndFinalFields: ResolverOptions.onlyInferConstAndFinalFieldsDefault, - this.nonnullableTypes: TypeOptions.NONNULLABLE_TYPES, this.help: false, - this.useMockSdk: false, this.dartSdkPath, this.logLevel: Level.SEVERE, - this.emitSourceMaps: true, this.entryPointFile: null, - this.serverMode: false, this.useImplicitHtml: false, + CompilerOptions({this.strongOptions: const StrongModeOptions(), + this.sourceOptions: const SourceResolverOptions(), + this.codegenOptions: const CodegenOptions(), this.checkSdk: false, + this.dumpInfo: false, this.dumpInfoFile, this.useColors: true, + this.help: false, this.logLevel: Level.SEVERE, this.serverMode: false, this.enableHashing: false, this.host: 'localhost', this.port: 8080, - this.runtimeDir, this.customUrlMappings: const {}}); + this.runtimeDir}); } /// Parses options from the command-line @@ -261,66 +169,42 @@ CompilerOptions parseOptions(List argv) { var entryPointFile = args.rest.length == 0 ? null : args.rest.first; return new CompilerOptions( + codegenOptions: new CodegenOptions( + emitSourceMaps: args['source-maps'], + forceCompile: args['force-compile'] || serverMode, + outputDir: outputDir), + sourceOptions: new SourceResolverOptions( + useMockSdk: args['mock-sdk'], + dartSdkPath: sdkPath, + entryPointFile: entryPointFile, + useImplicitHtml: serverMode && entryPointFile.endsWith('.dart'), + customUrlMappings: customUrlMappings, + useMultiPackage: args['use-multi-package'], + packageRoot: args['package-root'], + packagePaths: args['package-paths'].split(','), + resources: args['resources'] + .split(',') + .where((s) => s.isNotEmpty) + .toList()), + strongOptions: new StrongModeOptions.fromArguments(args), checkSdk: args['sdk-check'], dumpInfo: dumpInfo, dumpInfoFile: args['dump-info-file'], - forceCompile: args['force-compile'] || serverMode, - outputDir: outputDir, - relaxedCasts: args['relaxed-casts'], useColors: useColors, - customUrlMappings: customUrlMappings, - useMultiPackage: args['use-multi-package'], - packageRoot: args['package-root'], - packagePaths: args['package-paths'].split(','), - resources: args['resources'] - .split(',') - .where((s) => s.isNotEmpty) - .toList(), - inferDownwards: args['infer-downwards'], - inferFromOverrides: args['infer-from-overrides'], - inferTransitively: args['infer-transitively'], - onlyInferConstsAndFinalFields: args['infer-only-finals'], - nonnullableTypes: optionsToList(args['nonnullable'], - defaultValue: TypeOptions.NONNULLABLE_TYPES), help: showUsage, - useMockSdk: args['mock-sdk'], - dartSdkPath: sdkPath, logLevel: logLevel, - emitSourceMaps: args['source-maps'], - entryPointFile: entryPointFile, serverMode: serverMode, - useImplicitHtml: serverMode && entryPointFile.endsWith('.dart'), enableHashing: enableHashing, host: args['host'], port: int.parse(args['port']), runtimeDir: runtimeDir); } -final ArgParser argParser = new ArgParser() - // resolver/checker options +final ArgParser argParser = StrongModeOptions.addArguments(new ArgParser() ..addFlag('sdk-check', abbr: 's', help: 'Typecheck sdk libs', defaultsTo: false) ..addFlag('mock-sdk', abbr: 'm', help: 'Use a mock Dart SDK', defaultsTo: false) - ..addFlag('relaxed-casts', - help: 'Cast between Dart assignable types', defaultsTo: true) - ..addOption('nonnullable', - abbr: 'n', - help: 'Comma separated string of non-nullable types', - defaultsTo: null) - ..addFlag('infer-downwards', - help: 'Infer types downwards from local context', - defaultsTo: RulesOptions.inferDownwardsDefault) - ..addFlag('infer-from-overrides', - help: 'Infer unspecified types of fields and return types from\n' - 'definitions in supertypes', - defaultsTo: ResolverOptions.inferFromOverridesDefault) - ..addFlag('infer-transitively', - help: 'Infer consts/fields from definitions in other libraries', - defaultsTo: ResolverOptions.inferTransitivelyDefault) - ..addFlag('infer-only-finals', - help: 'Do not infer non-const or non-final fields', - defaultsTo: ResolverOptions.onlyInferConstAndFinalFieldsDefault) // input/output options ..addOption('out', abbr: 'o', help: 'Output directory', defaultsTo: null) @@ -368,7 +252,7 @@ final ArgParser argParser = new ArgParser() ..addOption('dump-info-file', abbr: 'f', help: 'Dump info json file (requires dump-info)', - defaultsTo: null); + defaultsTo: null)); /// Tries to find the `lib/runtime/` directory of the dev_compiler package. This /// works when running devc from it's sources or from a snapshot that is diff --git a/pkg/dev_compiler/lib/src/report.dart b/pkg/dev_compiler/lib/src/report.dart index 27452c125f2e..a694c40428b5 100644 --- a/pkg/dev_compiler/lib/src/report.dart +++ b/pkg/dev_compiler/lib/src/report.dart @@ -42,11 +42,16 @@ class Message { // Interface used to report error messages from the checker. abstract class CheckerReporter { + void log(Message message); +} + +// Interface used to report error messages from the compiler. +abstract class CompilerReporter extends CheckerReporter { final AnalysisContext _context; CompilationUnit _unit; Source _unitSource; - CheckerReporter(this._context); + CompilerReporter(this._context); /// Called when starting to process a library. void enterLibrary(Uri uri); @@ -67,8 +72,6 @@ abstract class CheckerReporter { _unitSource = null; } - void log(Message message); - // Called in server-mode. void clearLibrary(Uri uri); void clearHtml(Uri uri); @@ -81,7 +84,7 @@ abstract class CheckerReporter { final _checkerLogger = new Logger('dev_compiler.checker'); /// Simple reporter that logs checker messages as they are seen. -class LogReporter extends CheckerReporter { +class LogReporter extends CompilerReporter { final bool useColors; Source _current; @@ -112,7 +115,7 @@ class LogReporter extends CheckerReporter { } /// A reporter that gathers all the information in a [GlobalSummary]. -class SummaryReporter extends CheckerReporter { +class SummaryReporter extends CompilerReporter { GlobalSummary result = new GlobalSummary(); IndividualSummary _current; final Level _level; @@ -189,10 +192,9 @@ String summaryToString(GlobalSummary summary) { // Declare columns and add header table.declareColumn('package'); table.declareColumn('AnalyzerError', abbreviate: true); - var activeInfoTypes = - infoTypes.where((type) => counter.totals['$type'] != null); - activeInfoTypes - .forEach((type) => table.declareColumn('$type', abbreviate: true)); + + var activeInfoTypes = counter.totals.keys; + activeInfoTypes.forEach((t) => table.declareColumn(t, abbreviate: true)); table.declareColumn('LinesOfCode', abbreviate: true); table.addHeader(); @@ -201,8 +203,7 @@ String summaryToString(GlobalSummary summary) { for (var package in counter.errorCount.keys) { appendCount(package); appendCount(counter.errorCount[package]['AnalyzerError']); - activeInfoTypes - .forEach((e) => appendCount(counter.errorCount[package]['$e'])); + activeInfoTypes.forEach((t) => appendCount(counter.errorCount[package][t])); appendCount(counter.linesOfCode[package]); } @@ -211,7 +212,7 @@ String summaryToString(GlobalSummary summary) { table.addHeader(); table.addEntry('total'); appendCount(counter.totals['AnalyzerError']); - activeInfoTypes.forEach((type) => appendCount(counter.totals['$type'])); + activeInfoTypes.forEach((t) => appendCount(counter.totals[t])); appendCount(counter.totalLinesOfCode); appendPercent(count, total) { @@ -223,8 +224,7 @@ String summaryToString(GlobalSummary summary) { var totalLOC = counter.totalLinesOfCode; table.addEntry('%'); appendPercent(counter.totals['AnalyzerError'], totalLOC); - activeInfoTypes - .forEach((type) => appendPercent(counter.totals['$type'], totalLOC)); + activeInfoTypes.forEach((t) => appendPercent(counter.totals[t], totalLOC)); appendCount(100); return table.toString(); diff --git a/pkg/dev_compiler/lib/src/summary.dart b/pkg/dev_compiler/lib/src/summary.dart index f56c8a715264..7e2307d8d338 100644 --- a/pkg/dev_compiler/lib/src/summary.dart +++ b/pkg/dev_compiler/lib/src/summary.dart @@ -16,7 +16,7 @@ abstract class Summary { /// Summary for the entire program. class GlobalSummary implements Summary { - /// Summary from the system libaries. + /// Summary from the system libraries. final Map system = {}; /// Summary for libraries in packages. diff --git a/pkg/dev_compiler/lib/src/testing.dart b/pkg/dev_compiler/lib/src/testing.dart index 590298ee7564..81c25e304c7f 100644 --- a/pkg/dev_compiler/lib/src/testing.dart +++ b/pkg/dev_compiler/lib/src/testing.dart @@ -4,22 +4,23 @@ library dev_compiler.src.testing; +import 'dart:collection' show Queue; import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/file_system/memory_file_system.dart'; import 'package:analyzer/src/generated/ast.dart'; -import 'package:analyzer/src/generated/engine.dart' show AnalysisContext; +import 'package:analyzer/src/generated/engine.dart' + show AnalysisContext, AnalysisEngine; +import 'package:analyzer/src/generated/error.dart'; import 'package:logging/logging.dart'; import 'package:source_span/source_span.dart'; import 'package:test/test.dart'; -import 'package:dev_compiler/config.dart'; -import 'package:dev_compiler/devc.dart' show Compiler; +import 'package:dev_compiler/strong_mode.dart'; import 'analysis_context.dart'; import 'dependency_graph.dart' show runtimeFilesForServerMode; import 'info.dart'; import 'options.dart'; -import 'report.dart'; import 'utils.dart'; /// Run the checker on a program with files contents as indicated in @@ -46,97 +47,47 @@ import 'utils.dart'; /// ''' /// }); /// -CheckerResults testChecker(Map testFiles, {String sdkDir, - customUrlMappings: const {}, - CheckerReporter createReporter(AnalysisContext context), relaxedCasts: true, - inferDownwards: RulesOptions.inferDownwardsDefault, - inferFromOverrides: ResolverOptions.inferFromOverridesDefault, - inferTransitively: ResolverOptions.inferTransitivelyDefault, - nonnullableTypes: TypeOptions.NONNULLABLE_TYPES}) { +void testChecker(Map testFiles, {String sdkDir, + customUrlMappings: const {}, relaxedCasts: true, + inferDownwards: StrongModeOptions.inferDownwardsDefault, + inferFromOverrides: StrongModeOptions.inferFromOverridesDefault, + inferTransitively: StrongModeOptions.inferTransitivelyDefault, + nonnullableTypes: StrongModeOptions.NONNULLABLE_TYPES}) { expect(testFiles.containsKey('/main.dart'), isTrue, reason: '`/main.dart` is missing in testFiles'); var provider = createTestResourceProvider(testFiles); var uriResolver = new TestUriResolver(provider); + var context = AnalysisEngine.instance.createAnalysisContext(); + context.sourceFactory = createSourceFactory(new SourceResolverOptions( + customUrlMappings: customUrlMappings, + useMockSdk: sdkDir == null, + dartSdkPath: sdkDir, + entryPointFile: '/main.dart'), fileResolvers: [uriResolver]); - var options = new CompilerOptions( + var checker = new StrongChecker(context, new StrongModeOptions( relaxedCasts: relaxedCasts, inferDownwards: inferDownwards, inferFromOverrides: inferFromOverrides, inferTransitively: inferTransitively, nonnullableTypes: nonnullableTypes, - useMockSdk: sdkDir == null, - dartSdkPath: sdkDir, - runtimeDir: '/dev_compiler_runtime/', - entryPointFile: '/main.dart', - customUrlMappings: customUrlMappings); - - var context = createAnalysisContext(options, fileResolvers: [uriResolver]); + hints: true)); // Run the checker on /main.dart. - var mainFile = new Uri.file('/main.dart'); - TestReporter testReporter; - CheckerReporter reporter; - if (createReporter == null) { - reporter = testReporter = new TestReporter(context); - } else { - reporter = createReporter(context); - } - var results = - new Compiler(options, context: context, reporter: reporter).run(); + var mainSource = uriResolver.resolveAbsolute(new Uri.file('/main.dart')); + var initialLibrary = context.resolveCompilationUnit2(mainSource, mainSource); - // Extract expectations from the comments in the test files. - var expectedErrors = >{}; - var visitor = new _ErrorMarkerVisitor(expectedErrors); - var initialLibrary = - context.getLibraryElement(uriResolver.resolveAbsolute(mainFile)); - for (var lib in reachableLibraries(initialLibrary)) { + // Extract expectations from the comments in the test files, and + // check that all errors we emit are included in the expected map. + var allLibraries = reachableLibraries(initialLibrary.element.library); + for (var lib in allLibraries) { for (var unit in lib.units) { - unit.unit.accept(visitor); - } - } - - if (testReporter == null) return results; - - var total = expectedErrors.values.fold(0, (p, l) => p + l.length); - // Check that all errors we emit are included in the expected map. - for (var lib in results.libraries) { - var uri = lib.library.source.uri; - testReporter.infoMap[uri].forEach((node, actual) { - var expected = expectedErrors[node]; - var expectedTotal = expected == null ? 0 : expected.length; - if (actual.length != expectedTotal) { - expect(actual.length, expectedTotal, - reason: 'The checker found ${actual.length} errors on the ' - 'expression `$node`, but we expected $expectedTotal. These are the ' - 'errors the checker found:\n\n ${_unexpectedErrors(node, actual)}'); - } + if (unit.source.uri.scheme == 'dart') continue; - for (int i = 0; i < expected.length; i++) { - expect(actual[i].level, expected[i].level, - reason: 'expected different logging level at:\n\n' - '${_messageWithSpan(actual[i])}'); - expect(actual[i].runtimeType, expected[i].type, - reason: 'expected different error type at:\n\n' - '${_messageWithSpan(actual[i])}'); - } - expectedErrors.remove(node); - }); - } - - // Check that all expected errors are accounted for. - if (!expectedErrors.isEmpty) { - var newTotal = expectedErrors.values.fold(0, (p, l) => p + l.length); - // Non empty error expectation lists remaining - if (newTotal > 0) { - fail('Not all expected errors were reported by the checker. Only' - ' ${total - newTotal} out of $total expected errors were reported.\n' - 'The following errors were not reported:\n' - '${_unreportedErrors(expectedErrors)}'); + var errorInfo = checker.computeErrors(unit.source); + new _ExpectedErrorVisitor(errorInfo.errors).validate(unit.unit); } } - - return results; } /// Creates a [MemoryResourceProvider] with test data @@ -171,66 +122,27 @@ class TestUriResolver extends ResourceUriResolver { } } -class TestReporter extends SummaryReporter { - Map>> infoMap = {}; - Map> _current; +class _ExpectedErrorVisitor extends UnifyingAstVisitor { + final Set _actualErrors; + CompilationUnit _unit; + String _unitSourceCode; - TestReporter(AnalysisContext context) : super(context); + _ExpectedErrorVisitor(List actualErrors) + : _actualErrors = new Set.from(actualErrors); - void enterLibrary(Uri uri) { - super.enterLibrary(uri); - infoMap[uri] = _current = {}; - } + validate(CompilationUnit unit) { + _unit = unit; + // This reads the file. Only safe because tests use MemoryFileSystem. + _unitSourceCode = unit.element.source.contents.data; + + // Visit the compilation unit. + unit.accept(this); - void log(Message info) { - super.log(info); - if (info is StaticInfo) { - _current.putIfAbsent(info.node, () => []).add(info); + if (_actualErrors.isNotEmpty) { + var actualMsgs = _actualErrors.map(_formatActualError).join('\n'); + fail('Unexpected errors reported by checker:\n\n$actualMsgs'); } } -} - -/// Create an error explanation for errors that were not expected, but that the -/// checker produced. -String _unexpectedErrors(AstNode node, List errors) { - final span = _spanFor(node); - return errors.map((e) { - var level = e.level.name.toLowerCase(); - return '$level: ${span.message(e.message, color: colorOf(level))}'; - }).join('\n'); -} - -String _unreportedErrors(Map> expected) { - var sb = new StringBuffer(); - for (var node in expected.keys) { - var span = _spanFor(node); - expected[node].forEach((e) { - var level = e.level.name.toLowerCase(); - sb.write('$level: ${span.message("${e.type}", color: colorOf(level))}\n'); - }); - } - return sb.toString(); -} - -String _messageWithSpan(StaticInfo info) { - var span = _spanFor(info.node); - var level = info.level.name.toLowerCase(); - return '$level: ${span.message(info.message, color: colorOf(level))}'; -} - -SourceSpan _spanFor(AstNode node) { - var unit = node.root as CompilationUnit; - var source = unit.element.source; - // This reads the file. Only safe in tests, because they use MemoryFileSystem. - var content = source.contents.data; - return createSpanHelper(unit, node.offset, node.end, source, content); -} - -/// Visitor that extracts expected errors from comments. -class _ErrorMarkerVisitor extends UnifyingAstVisitor { - Map> expectedErrors; - - _ErrorMarkerVisitor(this.expectedErrors); visitNode(AstNode node) { var token = node.beginToken; @@ -248,36 +160,79 @@ class _ErrorMarkerVisitor extends UnifyingAstVisitor { if (start != -1 && end != -1) { expect(start, lessThan(end)); var errors = commentText.substring(start + 2, end).split(','); - var expectations = errors.map(_ErrorExpectation.parse); - expectedErrors[node] = expectations.where((x) => x != null).toList(); + var expectations = + errors.map(_ErrorExpectation.parse).where((x) => x != null); + + for (var e in expectations) _expectError(node, e); } } } return super.visitNode(node); } + + void _expectError(AstNode node, _ErrorExpectation expected) { + + // See if we can find the expected error in our actual errors + for (var actual in _actualErrors) { + if (actual.offset == node.offset && actual.length == node.length) { + var actualMsg = _formatActualError(actual); + expect(_actualErrorLevel(actual), expected.level, + reason: 'expected different error code at:\n\n$actualMsg'); + expect(actual.errorCode.name, expected.typeName, + reason: 'expected different error type at:\n\n$actualMsg'); + + // We found it. Stop the search. + _actualErrors.remove(actual); + return; + } + } + + var span = _createSpan(node.offset, node.length); + var levelName = expected.level.name.toLowerCase(); + var msg = span.message(expected.typeName, color: colorOf(levelName)); + fail('expected error was not reported at:\n\n$levelName: $msg'); + } + + Level _actualErrorLevel(AnalysisError actual) { + return const { + ErrorSeverity.ERROR: Level.SEVERE, + ErrorSeverity.WARNING: Level.WARNING, + ErrorSeverity.INFO: Level.INFO + }[actual.errorCode.errorSeverity]; + } + + String _formatActualError(AnalysisError actual) { + var span = _createSpan(actual.offset, actual.length); + var levelName = _actualErrorLevel(actual).name.toLowerCase(); + var msg = span.message(actual.message, color: colorOf(levelName)); + return '$levelName: $msg'; + } + + SourceSpan _createSpan(int offset, int len) { + return createSpanHelper( + _unit, offset, offset + len, _unit.element.source, _unitSourceCode); + } } /// Describes an expected message that should be produced by the checker. class _ErrorExpectation { final Level level; - final Type type; - _ErrorExpectation(this.level, this.type); + final String typeName; + _ErrorExpectation(this.level, this.typeName); static _ErrorExpectation _parse(String descriptor) { var tokens = descriptor.split(':'); expect(tokens.length, 2, reason: 'invalid error descriptor'); var name = tokens[0].toUpperCase(); - var typeName = tokens[1].toLowerCase(); + var typeName = tokens[1]; var level = Level.LEVELS.firstWhere((l) => l.name == name, orElse: () => null); expect(level, isNotNull, reason: 'invalid level in error descriptor: `${tokens[0]}`'); - var type = infoTypes.firstWhere((t) => '$t'.toLowerCase() == typeName, - orElse: () => null); - expect(type, isNotNull, + expect(typeName, isNotNull, reason: 'invalid type in error descriptor: ${tokens[1]}'); - return new _ErrorExpectation(level, type); + return new _ErrorExpectation(level, typeName); } static _ErrorExpectation parse(String descriptor) { @@ -293,5 +248,5 @@ class _ErrorExpectation { return _parse(tokens[0]); } - String toString() => '$level $type'; + String toString() => '$level $typeName'; } diff --git a/pkg/dev_compiler/lib/src/utils.dart b/pkg/dev_compiler/lib/src/utils.dart index 979faf306e4e..81f031fe6b01 100644 --- a/pkg/dev_compiler/lib/src/utils.dart +++ b/pkg/dev_compiler/lib/src/utils.dart @@ -79,7 +79,7 @@ String _toIdentifier(String name) { final _invalidCharInIdentifier = new RegExp(r'[^A-Za-z_$0-9]'); /// Returns all libraries transitively imported or exported from [start]. -Iterable reachableLibraries(LibraryElement start) { +List reachableLibraries(LibraryElement start) { var results = []; var seen = new Set(); void find(LibraryElement lib) { diff --git a/pkg/dev_compiler/lib/strong_mode.dart b/pkg/dev_compiler/lib/strong_mode.dart new file mode 100644 index 000000000000..7f78df400cc1 --- /dev/null +++ b/pkg/dev_compiler/lib/strong_mode.dart @@ -0,0 +1,211 @@ +// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +/// Types needed to implement "strong" checking in the Dart analyzer. +/// This is intended to be used by analyzer_cli and analysis_server packages. +library dev_compiler.strong_mode; + +import 'package:analyzer/src/generated/engine.dart' + show AnalysisContextImpl, AnalysisErrorInfo, AnalysisErrorInfoImpl; +import 'package:analyzer/src/generated/error.dart' + show + AnalysisError, + ErrorCode, + CompileTimeErrorCode, + StaticTypeWarningCode, + HintCode; +import 'package:analyzer/src/generated/source.dart' show Source; +import 'package:args/args.dart'; +import 'package:logging/logging.dart' show Level; + +import 'src/checker/checker.dart' show CodeChecker; +import 'src/checker/resolver.dart' show LibraryResolverWithInference; +import 'src/checker/rules.dart' show RestrictedRules; +import 'src/report.dart' show CheckerReporter, Message; + +/// A type checker for Dart code that operates under stronger rules, and has +/// the ability to do local type inference in some situations. +class StrongChecker { + final AnalysisContextImpl _context; + final CodeChecker _checker; + final _ErrorReporter _reporter; + final StrongModeOptions _options; + + StrongChecker._(this._context, this._options, this._checker, this._reporter); + + factory StrongChecker( + AnalysisContextImpl context, StrongModeOptions options) { + // TODO(jmesserly): is there a cleaner way to plug this in? + if (context.libraryResolverFactory != null) { + throw new ArgumentError.value(context, 'context', + 'Analysis context must not have libraryResolverFactory already set.'); + } + context.libraryResolverFactory = + (c) => new LibraryResolverWithInference(c, options); + + var rules = new RestrictedRules(context.typeProvider, options: options); + var reporter = new _ErrorReporter(); + var checker = new CodeChecker(rules, reporter, options); + return new StrongChecker._(context, options, checker, reporter); + } + + /// Computes and returns DDC errors for the [source]. + AnalysisErrorInfo computeErrors(Source source) { + var errors = new List(); + + // TODO(jmesserly): change DDC to emit ErrorCodes directly. + _reporter._log = (Message msg) { + // Skip hints unless requested. + if (msg.level < Level.WARNING && !_options.hints) return; + + var errorCodeFactory = _levelToErrorCode[msg.level]; + var category = '${msg.runtimeType}'; + var errorCode = errorCodeFactory(category, msg.message); + var len = msg.end - msg.begin; + errors.add(new AnalysisError.con2(source, msg.begin, len, errorCode)); + }; + + for (Source librarySource in _context.getLibrariesContaining(source)) { + var resolved = _context.resolveCompilationUnit2(source, librarySource); + _checker.visitCompilationUnit(resolved); + } + _reporter._log = null; + return new AnalysisErrorInfoImpl(errors, _context.getLineInfo(source)); + } +} + +/// Maps a DDC log level to an analyzer ErrorCode subclass. +final _levelToErrorCode = { + Level.SEVERE: (n, m) => new CompileTimeErrorCode(n, m), + Level.WARNING: (n, m) => new StaticTypeWarningCode(n, m), + Level.INFO: (n, m) => new HintCode(n, m) +}; + +class _ErrorReporter implements CheckerReporter { + _CheckerReporterLog _log; + void log(Message message) => _log(message); +} + +class StrongModeOptions { + + /// Whether to infer return types and field types from overridden members. + final bool inferFromOverrides; + static const inferFromOverridesDefault = true; + + /// Whether to infer types for consts and fields by looking at initializers on + /// the RHS. For example, in a constant declaration like: + /// + /// const A = B; + /// + /// We can infer the type of `A` based on the type of `B`. + /// + /// The inference algorithm determines what variables depend on others, and + /// computes types by visiting the variable dependency graph in topological + /// order. This ensures that the inferred type is deterministic when applying + /// inference on library cycles. + /// + /// When this feature is turned off, we don't use the type of `B` to infer the + /// type of `A`, even if `B` has a declared type. + final bool inferTransitively; + static const inferTransitivelyDefault = true; + + /// Restrict inference of fields and top-levels to those that are final and + /// const. + final bool onlyInferConstsAndFinalFields; + static const onlyInferConstAndFinalFieldsDefault = false; + + /// Whether to infer types downwards from local context + final bool inferDownwards; + static const inferDownwardsDefault = true; + + /// Whether to inject casts between Dart assignable types. + final bool relaxedCasts; + + /// A list of non-nullable type names (e.g., 'int') + final List nonnullableTypes; + static const List NONNULLABLE_TYPES = const []; + + /// Whether to include hints about dynamic invokes and runtime checks. + // TODO(jmesserly): this option is not used yet by DDC server mode or batch + // compile to JS. + final bool hints; + + const StrongModeOptions({this.hints: false, + this.inferFromOverrides: inferFromOverridesDefault, + this.inferTransitively: inferTransitivelyDefault, + this.onlyInferConstsAndFinalFields: onlyInferConstAndFinalFieldsDefault, + this.inferDownwards: inferDownwardsDefault, this.relaxedCasts: true, + this.nonnullableTypes: StrongModeOptions.NONNULLABLE_TYPES}); + + StrongModeOptions.fromArguments(ArgResults args, {String prefix: ''}) + : relaxedCasts = args[prefix + 'relaxed-casts'], + inferDownwards = args[prefix + 'infer-downwards'], + inferFromOverrides = args[prefix + 'infer-from-overrides'], + inferTransitively = args[prefix + 'infer-transitively'], + onlyInferConstsAndFinalFields = args[prefix + 'infer-only-finals'], + nonnullableTypes = _optionsToList(args[prefix + 'nonnullable'], + defaultValue: StrongModeOptions.NONNULLABLE_TYPES), + hints = args[prefix + 'hints']; + + static ArgParser addArguments(ArgParser parser, + {String prefix: '', bool hide: false}) { + return parser + ..addFlag(prefix + 'hints', + help: 'Display hints about dynamic casts and dispatch operations', + defaultsTo: false, + hide: hide) + ..addFlag(prefix + 'relaxed-casts', + help: 'Cast between Dart assignable types', + defaultsTo: true, + hide: hide) + ..addOption(prefix + 'nonnullable', + abbr: prefix == '' ? 'n' : null, + help: 'Comma separated string of non-nullable types', + defaultsTo: null, + hide: hide) + ..addFlag(prefix + 'infer-downwards', + help: 'Infer types downwards from local context', + defaultsTo: inferDownwardsDefault, + hide: hide) + ..addFlag(prefix + 'infer-from-overrides', + help: 'Infer unspecified types of fields and return types from\n' + 'definitions in supertypes', + defaultsTo: inferFromOverridesDefault, + hide: hide) + ..addFlag(prefix + 'infer-transitively', + help: 'Infer consts/fields from definitions in other libraries', + defaultsTo: inferTransitivelyDefault, + hide: hide) + ..addFlag(prefix + 'infer-only-finals', + help: 'Do not infer non-const or non-final fields', + defaultsTo: onlyInferConstAndFinalFieldsDefault, + hide: hide); + } + + bool operator ==(Object other) { + if (other is! StrongModeOptions) return false; + StrongModeOptions s = other; + return inferFromOverrides == s.inferFromOverrides && + inferTransitively == s.inferTransitively && + onlyInferConstsAndFinalFields == s.onlyInferConstsAndFinalFields && + inferDownwards == s.inferDownwards && + relaxedCasts == s.relaxedCasts && + nonnullableTypes.length == s.nonnullableTypes.length && + new Set.from(nonnullableTypes).containsAll(s.nonnullableTypes); + } +} + +typedef void _CheckerReporterLog(Message message); +typedef ErrorCode _ErrorCodeFactory(String name, String message); + +List _optionsToList(String option, + {List defaultValue: const []}) { + if (option == null) { + return defaultValue; + } else if (option.isEmpty) { + return []; + } else { + return option.split(','); + } +} diff --git a/pkg/dev_compiler/test/checker/inferred_type_test.dart b/pkg/dev_compiler/test/checker/inferred_type_test.dart index 28373e9b9c14..d5d01e219b26 100644 --- a/pkg/dev_compiler/test/checker/inferred_type_test.dart +++ b/pkg/dev_compiler/test/checker/inferred_type_test.dart @@ -955,7 +955,7 @@ void main() { }, inferFromOverrides: true); }); - test('do not infer overriden fields that explicitly say dynamic', () { + test('do not infer overridden fields that explicitly say dynamic', () { for (bool infer in [true, false]) { testChecker({ '/main.dart': ''' diff --git a/pkg/dev_compiler/test/checker/self_host_test.dart b/pkg/dev_compiler/test/checker/self_host_test.dart index dca6ab551c1a..24d84481ffcb 100644 --- a/pkg/dev_compiler/test/checker/self_host_test.dart +++ b/pkg/dev_compiler/test/checker/self_host_test.dart @@ -16,8 +16,9 @@ import '../test_util.dart' show testDirectory; void main() { test('checker can run on itself ', () { var options = new CompilerOptions( - entryPointFile: '$testDirectory/all_tests.dart', - dartSdkPath: getSdkDir().path); + sourceOptions: new SourceResolverOptions( + entryPointFile: '$testDirectory/all_tests.dart', + dartSdkPath: getSdkDir().path)); new Compiler(options).run(); }); } diff --git a/pkg/dev_compiler/test/codegen_test.dart b/pkg/dev_compiler/test/codegen_test.dart index 54c8852fc41a..e8dc5eb91a73 100644 --- a/pkg/dev_compiler/test/codegen_test.dart +++ b/pkg/dev_compiler/test/codegen_test.dart @@ -62,13 +62,16 @@ main(arguments) { // they're more self-contained. var runtimeDir = path.join(path.dirname(testDirectory), 'lib', 'runtime'); var options = new CompilerOptions( - outputDir: subDir == null ? actualDir : path.join(actualDir, subDir), + sourceOptions: new SourceResolverOptions( + entryPointFile: entryPoint, dartSdkPath: sdkPath), + codegenOptions: new CodegenOptions( + outputDir: subDir == null + ? actualDir + : path.join(actualDir, subDir), + emitSourceMaps: sourceMaps, + forceCompile: checkSdk), useColors: false, - emitSourceMaps: sourceMaps, - forceCompile: checkSdk, checkSdk: checkSdk, - entryPointFile: entryPoint, - dartSdkPath: sdkPath, runtimeDir: runtimeDir, serverMode: serverMode, enableHashing: serverMode); diff --git a/pkg/dev_compiler/test/dependency_graph_test.dart b/pkg/dev_compiler/test/dependency_graph_test.dart index 99cc40396a8d..f32028945336 100644 --- a/pkg/dev_compiler/test/dependency_graph_test.dart +++ b/pkg/dev_compiler/test/dependency_graph_test.dart @@ -18,7 +18,8 @@ import 'package:path/path.dart' as path; void main() { var options = new CompilerOptions( - runtimeDir: '/dev_compiler_runtime/', useMockSdk: true); + runtimeDir: '/dev_compiler_runtime/', + sourceOptions: new SourceResolverOptions(useMockSdk: true)); MemoryResourceProvider testResourceProvider; ResourceUriResolver testUriResolver; var context; @@ -62,7 +63,9 @@ void main() { /// tests (since some tests modify the state of the files). testResourceProvider = createTestResourceProvider(testFiles); testUriResolver = new ResourceUriResolver(testResourceProvider); - context = createAnalysisContext(options, fileResolvers: [testUriResolver]); + context = createAnalysisContextWithSources( + options.strongOptions, options.sourceOptions, + fileResolvers: [testUriResolver]); graph = new SourceGraph(context, new LogReporter(context), options); }); @@ -639,13 +642,14 @@ void main() { group('server-mode', () { setUp(() { - var options2 = new CompilerOptions( + var opts = new CompilerOptions( runtimeDir: '/dev_compiler_runtime/', - useMockSdk: true, + sourceOptions: new SourceResolverOptions(useMockSdk: true), serverMode: true); - context = - createAnalysisContext(options2, fileResolvers: [testUriResolver]); - graph = new SourceGraph(context, new LogReporter(context), options2); + context = createAnalysisContextWithSources( + opts.strongOptions, opts.sourceOptions, + fileResolvers: [testUriResolver]); + graph = new SourceGraph(context, new LogReporter(context), opts); }); test('messages widget is automatically included', () { @@ -1086,8 +1090,9 @@ void main() { group('null for non-existing files', () { setUp(() { - context = - createAnalysisContext(options, fileResolvers: [testUriResolver]); + context = createAnalysisContextWithSources( + options.strongOptions, options.sourceOptions, + fileResolvers: [testUriResolver]); graph = new SourceGraph(context, new LogReporter(context), options); }); diff --git a/pkg/dev_compiler/test/end_to_end_test.dart b/pkg/dev_compiler/test/end_to_end_test.dart index d34a07465dd1..04a8455f8e3f 100644 --- a/pkg/dev_compiler/test/end_to_end_test.dart +++ b/pkg/dev_compiler/test/end_to_end_test.dart @@ -14,7 +14,8 @@ import 'test_util.dart' show testDirectory; main() { _check(testFile) { var options = new CompilerOptions( - entryPointFile: '$testDirectory/$testFile.dart', useMockSdk: true); + sourceOptions: new SourceResolverOptions( + useMockSdk: true, entryPointFile: '$testDirectory/$testFile.dart')); new Compiler(options).run(); } diff --git a/pkg/dev_compiler/test/report_test.dart b/pkg/dev_compiler/test/report_test.dart index 1cf9ae416342..8df2d3e59ce7 100644 --- a/pkg/dev_compiler/test/report_test.dart +++ b/pkg/dev_compiler/test/report_test.dart @@ -7,9 +7,14 @@ library dev_compiler.test.report_test; import 'package:test/test.dart'; -import 'package:dev_compiler/src/testing.dart'; +import 'package:dev_compiler/devc.dart'; +import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; + +import 'package:dev_compiler/src/analysis_context.dart'; +import 'package:dev_compiler/src/options.dart'; import 'package:dev_compiler/src/report.dart'; import 'package:dev_compiler/src/summary.dart'; +import 'package:dev_compiler/src/testing.dart'; void main() { test('toJson/parse', () { @@ -28,11 +33,16 @@ void main() { } '''.replaceAll('\n ', '\n'), }; - testChecker(files); - SummaryReporter reporter; - testChecker(files, - createReporter: (x) => reporter = new SummaryReporter(x)); + var provider = createTestResourceProvider(files); + var uriResolver = new TestUriResolver(provider); + var srcOpts = new SourceResolverOptions( + entryPointFile: '/main.dart', useMockSdk: true); + var context = createAnalysisContextWithSources( + new StrongModeOptions(), srcOpts, fileResolvers: [uriResolver]); + var reporter = new SummaryReporter(context); + new Compiler(new CompilerOptions(sourceOptions: srcOpts), + context: context, reporter: reporter).run(); _verifySummary(GlobalSummary summary) { var mainLib = summary.loose['file:///main.dart'];