From 7d3c0b01ff685cfd75c2f4f135becea989102f5a Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 30 Jul 2015 10:40:48 +0100 Subject: [PATCH] Output some basic type annotations for the Closure Compiler when --closure is set. --- lib/runtime/_operations.js | 2 +- lib/src/closure/closure_annotation.dart | 111 ++++++++++++++++ lib/src/closure/closure_annotator.dart | 140 ++++++++++++++++++++ lib/src/closure/closure_type.dart | 80 ++++++++++++ lib/src/codegen/js_codegen.dart | 112 ++++++++++++---- lib/src/js/js_ast.dart | 2 + lib/src/js/nodes.dart | 11 ++ lib/src/js/printer.dart | 16 +++ lib/src/options.dart | 9 ++ test/all_tests.dart | 6 + test/closure/closure_annotation_test.dart | 123 ++++++++++++++++++ test/closure/closure_type_test.dart | 82 ++++++++++++ test/codegen/closure.dart | 59 +++++++++ test/codegen/expect/closure.js | 149 ++++++++++++++++++++++ test/codegen/expect/closure.txt | 2 + test/codegen_test.dart | 9 +- tool/build_sdk.sh | 1 + 17 files changed, 886 insertions(+), 28 deletions(-) create mode 100644 lib/src/closure/closure_annotation.dart create mode 100644 lib/src/closure/closure_annotator.dart create mode 100644 lib/src/closure/closure_type.dart create mode 100644 test/closure/closure_annotation_test.dart create mode 100644 test/closure/closure_type_test.dart create mode 100644 test/codegen/closure.dart create mode 100644 test/codegen/expect/closure.js create mode 100644 test/codegen/expect/closure.txt diff --git a/lib/runtime/_operations.js b/lib/runtime/_operations.js index c018641c..696f540f 100644 --- a/lib/runtime/_operations.js +++ b/lib/runtime/_operations.js @@ -95,7 +95,7 @@ dart_library.library('dart_runtime/_operations', null, /* Imports */[ let names = getOwnPropertyNames(opts); // Type is something other than a map if (names.length == 0) return false; - for (name of names) { + for (var name of names) { if (!(hasOwnProperty.call(type.named, name))) { return false; } diff --git a/lib/src/closure/closure_annotation.dart b/lib/src/closure/closure_annotation.dart new file mode 100644 index 00000000..e05deba4 --- /dev/null +++ b/lib/src/closure/closure_annotation.dart @@ -0,0 +1,111 @@ +// 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. + +library dev_compiler.src.closure.closure_annotation; + +import 'closure_type.dart'; + +/// Set of closure annotations that can be [toString]ed to a single JsDoc comment. +/// See https://developers.google.com/closure/compiler/docs/js-for-compiler +/// +/// TODO(ochafik): Support inclusion of 'normal' comments (including @param comments). +class ClosureAnnotation { + final bool isConst; + final bool isConstructor; + final bool isFinal; + final bool isNoCollapse; + final bool isNoSideEffects; + final bool isOverride; + final bool isPrivate; + final bool isProtected; + final bool isStruct; + final bool isTypedef; + final ClosureType lendsToType; + final ClosureType returnType; + final ClosureType superType; + final ClosureType thisType; + final ClosureType throwsType; + final ClosureType type; + final List interfaces; + final List templates; + final Map paramTypes; + + ClosureAnnotation({this.interfaces: const [], this.isConst: false, + this.isConstructor: false, this.isFinal: false, this.isNoCollapse: false, + this.isNoSideEffects: false, this.isOverride: false, + this.isPrivate: false, this.isProtected: false, this.isStruct: false, + this.isTypedef: false, this.lendsToType, this.paramTypes: const {}, + this.returnType, this.superType, this.templates: const [], this.thisType, + this.throwsType, this.type}); + + @override + int get hashCode => _cachedString.hashCode; + + @override + bool operator ==(other) => + other is ClosureAnnotation && _cachedString == other._cachedString; + + @override + String toString([String indent = '']) => + _cachedString.replaceAll('\n', '\n$indent'); + + String __cachedString; + String get _cachedString { + if (__cachedString == null) { + bool isNonWildcard(ClosureType t) => + t != null && !t.isAll && !t.isUnknown; + + var lines = []; + if (templates != null && templates.isNotEmpty) { + lines.add('@template ${templates.join(', ')}'); + } + if (thisType != null) lines.add('@this {$thisType}'); + if (isOverride) lines.add('@override'); + if (isNoSideEffects) lines.add('@nosideeffects'); + if (isNoCollapse) lines.add('@nocollapse'); + if (lendsToType != null) lines.add('@lends {$lendsToType}'); + + { + var typeHolders = []; + if (isPrivate) typeHolders.add('@private'); + if (isProtected) typeHolders.add('@protected'); + if (isFinal) typeHolders.add('@final'); + if (isConst) typeHolders.add('@const'); + if (isTypedef) typeHolders.add('@typedef'); + if (isNonWildcard(type)) { + if (typeHolders.isEmpty) typeHolders.add('@type'); + typeHolders.add('{$type}'); + } + if (!typeHolders.isEmpty) lines.add(typeHolders.join(' ')); + } + + { + List constructorLine = []; + if (isConstructor) constructorLine.add('@constructor'); + if (isStruct) constructorLine.add('@struct'); + if (isNonWildcard(superType)) { + constructorLine.add('@extends {$superType}'); + } + + if (constructorLine.isNotEmpty) lines.add(constructorLine.join(' ')); + } + + for (var interface in interfaces) { + if (isNonWildcard(interface)) lines.add('@implements {$interface}'); + } + + paramTypes.forEach((String paramName, ClosureType paramType) { + // Must output params even with wildcard type. + lines.add('@param {$paramType} $paramName'); + }); + if (isNonWildcard(returnType)) lines.add('@return {$returnType}'); + if (isNonWildcard(throwsType)) lines.add('@throws {$throwsType}'); + + if (lines.length == 0) return ''; + if (lines.length == 1) return '/** ${lines.single} */'; + __cachedString = '/**\n' + lines.map((l) => ' * $l').join('\n') + '\n */'; + } + return __cachedString; + } +} diff --git a/lib/src/closure/closure_annotator.dart b/lib/src/closure/closure_annotator.dart new file mode 100644 index 00000000..2a03ddbe --- /dev/null +++ b/lib/src/closure/closure_annotator.dart @@ -0,0 +1,140 @@ +// 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. + +library dev_compiler.src.closure.closure_codegen; + +import 'package:analyzer/analyzer.dart' show ParameterKind; +import 'package:analyzer/src/generated/element.dart'; + +import 'closure_annotation.dart'; +import 'closure_type.dart'; +import 'package:analyzer/src/generated/resolver.dart' show TypeProvider; + +/// Mixin that can generate [ClosureAnnotation]s for Dart elements and types. +abstract class ClosureAnnotator { + TypeProvider get types; + + /// Must return a JavaScript qualified name that can be used to refer to [type]. + String getQualifiedName(TypeDefiningElement type); + + ClosureAnnotation closureAnnotationForVariable(VariableElement e) => + new ClosureAnnotation(type: _closureTypeForDartType(e.type), + // Note: we don't set isConst here because Closure's constness and + // Dart's are not really compatible. + isFinal: e.isFinal || e.isConst); + + /// We don't use Closure's `@typedef` annotations + ClosureAnnotation closureAnnotationForTypeDef(FunctionTypeAliasElement e) => + new ClosureAnnotation( + type: _closureTypeForDartType(e.type, forceTypeDefExpansion: true), + isTypedef: true); + + ClosureAnnotation closureAnnotationForDefaultConstructor(ClassElement e) => + new ClosureAnnotation( + superType: _closureTypeForDartType(e.supertype), + interfaces: e.interfaces.map(_closureTypeForDartType).toList()); + + ClosureAnnotation closureAnnotationFor( + ExecutableElement e, String namedArgsMapName) { + var paramTypes = {}; + var namedArgs = {}; + for (var param in e.parameters) { + var t = _closureTypeForDartType(param.type); + switch (param.parameterKind) { + case ParameterKind.NAMED: + namedArgs[param.name] = t.orUndefined(); + break; + case ParameterKind.POSITIONAL: + paramTypes[param.name] = t.toOptional(); + break; + case ParameterKind.REQUIRED: + paramTypes[param.name] = t; + break; + } + } + if (namedArgs.isNotEmpty) { + paramTypes[namedArgsMapName] = + new ClosureType.record(namedArgs).toOptional(); + } + + var returnType = e is ConstructorElement + ? (e.isFactory ? _closureTypeForClass(e.enclosingElement) : null) + : _closureTypeForDartType(e.returnType); + + return new ClosureAnnotation(isOverride: e.isOverride, + // Note: Dart and Closure privacy are not compatible: don't set `isPrivate: e.isPrivate`. + paramTypes: paramTypes, returnType: returnType); + } + + Map __commonTypes; + Map get _commonTypes { + if (__commonTypes == null) { + var numberType = new ClosureType.number().toNullable(); + __commonTypes = { + types.intType: numberType, + types.numType: numberType, + types.doubleType: numberType, + types.boolType: new ClosureType.boolean().toNullable(), + types.stringType: new ClosureType.string(), + }; + } + return __commonTypes; + } + + ClosureType _closureTypeForClass(ClassElement classElement, [DartType type]) { + ClosureType closureType = _commonTypes[type]; + if (closureType != null) return closureType; + + var fullName = _getFullName(classElement); + switch (fullName) { + // TODO(ochafik): Test DartTypes directly if possible. + case "dart.js.JsArray": + return new ClosureType.array( + type is InterfaceType && type.typeArguments.length == 1 + ? _closureTypeForDartType(type.typeArguments.single) + : null); + case "dart.js.JsObject": + return new ClosureType.map(); + case "dart.js.JsFunction": + return new ClosureType.function(); + default: + return new ClosureType.type(getQualifiedName(classElement)); + } + } + + ClosureType _closureTypeForDartType(DartType type, + {bool forceTypeDefExpansion: false}) { + if (type == null || type.isBottom || type.isDynamic) { + return new ClosureType.unknown(); + } + if (type.isVoid) return null; + if (type is FunctionType) { + if (!forceTypeDefExpansion && type.element.name != '') { + return new ClosureType.type(getQualifiedName(type.element)); + } + + var args = [] + ..addAll(type.normalParameterTypes.map(_closureTypeForDartType)) + ..addAll(type.optionalParameterTypes + .map((t) => _closureTypeForDartType(t).toOptional())); + if (type.namedParameterTypes.isNotEmpty) { + var namedArgs = {}; + type.namedParameterTypes.forEach((n, t) { + namedArgs[n] = _closureTypeForDartType(t).orUndefined(); + }); + args.add(new ClosureType.record(namedArgs).toOptional()); + } + return new ClosureType.function( + args, _closureTypeForDartType(type.returnType)); + } + if (type is InterfaceType) { + return _closureTypeForClass(type.element, type); + } + return new ClosureType.unknown(); + } + + /// TODO(ochafik): Use a package-and-file-uri-dependent naming, since libraries can collide. + String _getFullName(ClassElement type) => + type.library.name == '' ? type.name : '${type.library.name}.${type.name}'; +} diff --git a/lib/src/closure/closure_type.dart b/lib/src/closure/closure_type.dart new file mode 100644 index 00000000..357c016b --- /dev/null +++ b/lib/src/closure/closure_type.dart @@ -0,0 +1,80 @@ +// 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. + +library dev_compiler.src.closure.closure_type; + +/// Poor-man's representation of a Closure type. +/// See https://developers.google.com/closure/compiler/docs/js-for-compiler +/// +/// The goal here is not to completely support Closure's type system, but to +/// be able to generate just the types needed for DDC's JS output. +/// +/// TODO(ochafik): Consider convergence with TypeScript, which has no nullability-awareness +/// (see http://www.typescriptlang.org/Handbook). +class ClosureType { + static const ClosureType _ALL = const ClosureType._("*"); + static const ClosureType _UNKNOWN = const ClosureType._("?"); + + final String _representation; + final bool isNullable; + + const ClosureType._(this._representation, {this.isNullable: true}); + + bool get isAll => _representation == "*"; + bool get isUnknown => _representation == "?"; + + @override toString() => _representation; + + factory ClosureType.all() => _ALL; + factory ClosureType.unknown() => _UNKNOWN; + + factory ClosureType.record(Map fieldTypes) { + var entries = []; + fieldTypes.forEach((n, t) => entries.add('$n: $t')); + return new ClosureType._('{${entries.join(', ')}}'); + } + factory ClosureType.function([List paramTypes, ClosureType returnType]) { + if (paramTypes == null && returnType == null) { + return new ClosureType.type("Function"); + } + var suffix = returnType == null ? '' : ':$returnType'; + return new ClosureType._( + 'function(${paramTypes == null ? '...*' : paramTypes.join(', ')})$suffix'); + } + + factory ClosureType.map([ClosureType keyType, ClosureType valueType]) => + new ClosureType._("Object<${keyType ?? _ALL}, ${valueType ?? _ALL}>"); + + factory ClosureType.type([String className = "Object"]) => + new ClosureType._(className); + + factory ClosureType.array([ClosureType componentType]) => + new ClosureType._("Array<${componentType ?? _ALL}>"); + + factory ClosureType.undefined() => new ClosureType._("undefined", isNullable: false); + factory ClosureType.number() => new ClosureType._("number", isNullable: false); + factory ClosureType.boolean() => new ClosureType._("boolean", isNullable: false); + factory ClosureType.string() => new ClosureType._("string"); + + ClosureType toOptional() => + new ClosureType._("$this="); + + ClosureType toNullable() => + isNullable ? this : new ClosureType._( + _representation.startsWith('!') ? _representation.substring(1) : "?$this", + isNullable: true); + + ClosureType toNonNullable() => + !isNullable ? this : new ClosureType._( + _representation.startsWith('?') ? _representation.substring(1) : "!$this", + isNullable: false); + + /// TODO(ochafik): See which optimizations make sense here (it could be that `(*|undefined)` + /// cannot be optimized to `*` when used to model optional record fields). + ClosureType or(ClosureType other) => + new ClosureType._("($this|$other)", isNullable: isNullable || other.isNullable); + + ClosureType orUndefined() => + or(new ClosureType.undefined()); +} diff --git a/lib/src/codegen/js_codegen.dart b/lib/src/codegen/js_codegen.dart index 9caf6308..937b9070 100644 --- a/lib/src/codegen/js_codegen.dart +++ b/lib/src/codegen/js_codegen.dart @@ -23,6 +23,7 @@ import 'package:dev_compiler/src/codegen/reify_coercions.dart' import 'package:dev_compiler/src/js/js_ast.dart' as JS; import 'package:dev_compiler/src/js/js_ast.dart' show js; +import 'package:dev_compiler/src/closure/closure_annotator.dart' show ClosureAnnotator; import 'package:dev_compiler/src/compiler.dart' show AbstractCompiler; import 'package:dev_compiler/src/checker/rules.dart'; import 'package:dev_compiler/src/info.dart'; @@ -49,7 +50,7 @@ const DSETINDEX = 'dsetindex'; const DCALL = 'dcall'; const DSEND = 'dsend'; -class JSCodegenVisitor extends GeneralizingAstVisitor { +class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator { final AbstractCompiler compiler; final CodegenOptions options; final TypeRules rules; @@ -366,8 +367,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { var type = element.type; var name = element.name; - var fnType = js.statement('let # = dart.typedef(#, () => #);', - [name, js.string(name, "'"), _emitTypeName(type, lowerTypedef: true)]); + var fnType = annotateTypeDef(js.statement('let # = dart.typedef(#, () => #);', + [name, js.string(name, "'"), _emitTypeName(type, lowerTypedef: true)]), + node.element); return _finishClassDef(type, fnType); } @@ -871,7 +873,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { [body, superCall] ]; var name = _constructorName(node.element.unnamedConstructor); - return new JS.Method(name, js.call('function() { #; }', body) as JS.Fun); + return annotateDefaultConstructor( + new JS.Method(name, js.call('function() { #; }', body) as JS.Fun), + node.element); } JS.Method _emitConstructor(ConstructorDeclaration node, InterfaceType type, @@ -890,7 +894,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { var params = _visit(node.parameters); var fun = js.call('function(#) { return $newKeyword #(#); }', [params, _visit(redirect), params,]) as JS.Fun; - return new JS.Method(name, fun, isStatic: true)..sourceInformation = node; + return annotate( + new JS.Method(name, fun, isStatic: true)..sourceInformation = node, + node.element); } // Factory constructors are essentially static methods. @@ -901,7 +907,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { body.add(_visit(node.body)); var fun = new JS.Fun( _visit(node.parameters) as List, new JS.Block(body)); - return new JS.Method(name, fun, isStatic: true)..sourceInformation = node; + return annotate( + new JS.Method(name, fun, isStatic: true)..sourceInformation = node, + node.element); } // Code generation for Object's constructor. @@ -936,9 +944,11 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { // We generate constructors as initializer methods in the class; // this allows use of `super` for instance methods/properties. // It also avoids V8 restrictions on `super` in default constructors. - return new JS.Method( - name, new JS.Fun(_visit(node.parameters) as List, body)) - ..sourceInformation = node; + return annotate( + new JS.Method( + name, new JS.Fun(_visit(node.parameters) as List, body)) + ..sourceInformation = node, + node.element); } JS.Expression _constructorName(ConstructorElement ctor) { @@ -1151,6 +1161,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { // Parameters will be passed using their real names, not the (possibly // renamed) local variable. var paramName = js.string(param.identifier.name, "'"); + + // TODO(ochafik): Fix `'prop' in obj` to please Closure's renaming + // (either test if `obj.prop !== void 0`, or use JSCompiler_renameProperty). body.add(js.statement('let # = # && # in # ? #.# : #;', [ jsParam, _namedArgTemp, @@ -1215,10 +1228,12 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { ..sourceInformation = fn.sourceInformation; } - return new JS.Method(_elementMemberName(node.element), fn, - isGetter: node.isGetter, - isSetter: node.isSetter, - isStatic: node.isStatic); + return annotate( + new JS.Method(_elementMemberName(node.element), fn, + isGetter: node.isGetter, + isSetter: node.isSetter, + isStatic: node.isStatic), + node.element); } @override @@ -1239,7 +1254,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { var name = node.name.name; var id = new JS.Identifier(name); - body.add(new JS.FunctionDeclaration(id, _visit(node.functionExpression))); + body.add(annotate( + new JS.FunctionDeclaration(id, _visit(node.functionExpression)), + node.element)); body.add(_emitFunctionTagged(id, node.element.type, topLevel: true) .toStatement()); @@ -1249,8 +1266,10 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { JS.Method _emitTopLevelProperty(FunctionDeclaration node) { var name = node.name.name; - return new JS.Method(_propertyName(name), _visit(node.functionExpression), - isGetter: node.isGetter, isSetter: node.isSetter); + return annotate( + new JS.Method(_propertyName(name), _visit(node.functionExpression), + isGetter: node.isGetter, isSetter: node.isSetter), + node.element); } bool _executesAtTopLevel(AstNode node) { @@ -1414,6 +1433,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { } else { declareFn = new JS.FunctionDeclaration(name, fn); } + declareFn = annotate(declareFn, node.functionDeclaration.element); return new JS.Block([ declareFn, @@ -1990,11 +2010,15 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { // anything they depend on first. if (isPublic(fieldName)) _addExport(fieldName); - return js.statement('let # = #;', [new JS.Identifier(fieldName), jsInit]); + return annotateVariable( + js.statement('let # = #;', [new JS.Identifier(fieldName), jsInit]), + field.element); } if (eagerInit && !JS.invalidStaticFieldName(fieldName)) { - return js.statement('# = #;', [_visit(field.name), jsInit]); + return annotateVariable( + js.statement('# = #;', [_visit(field.name), jsInit]), + field.element); } var body = []; @@ -2029,16 +2053,20 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { var name = node.name.name; var element = node.element; var access = _emitMemberName(name, type: element.type, isStatic: true); - methods.add(new JS.Method( - access, - js.call('function() { return #; }', _visit(node.initializer)) - as JS.Fun, - isGetter: true)); + methods.add(annotate( + new JS.Method( + access, + js.call('function() { return #; }', _visit(node.initializer)) + as JS.Fun, + isGetter: true), + _findAccessor(element, getter: true))); // TODO(jmesserly): use a dummy setter to indicate writable. if (!node.isFinal) { - methods.add(new JS.Method(access, js.call('function(_) {}') as JS.Fun, - isSetter: true)); + methods.add(annotate( + new JS.Method(access, js.call('function(_) {}') as JS.Fun, + isSetter: true), + _findAccessor(element, getter: false))); } } @@ -2052,6 +2080,14 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { 'dart.defineLazyProperties(#, { # });', [objExpr, methods]); } + PropertyAccessorElement _findAccessor(VariableElement element, {bool getter}) { + var parent = element.enclosingElement; + if (parent is ClassElement) { + return getter ? parent.getGetter(element.name) : parent.getSetter(element.name); + } + return null; + } + void _flushLibraryProperties(List body) { if (_properties.isEmpty) return; body.add(js.statement('dart.copyProperties(#, { # });', @@ -3154,6 +3190,32 @@ class JSCodegenVisitor extends GeneralizingAstVisitor { } DartType getStaticType(Expression e) => rules.getStaticType(e); + + @override + String getQualifiedName(TypeDefiningElement type) { + JS.TemporaryId id = _imports[type.library]; + return id == null ? type.name : '${id.name}.${type.name}'; + } + + JS.Node annotate(JS.Node method, ExecutableElement e) => + options.closure && e != null + ? method.withClosureAnnotation(closureAnnotationFor(e, _namedArgTemp.name)) + : method; + + JS.Node annotateDefaultConstructor(JS.Node method, ClassElement e) => + options.closure && e != null + ? method.withClosureAnnotation(closureAnnotationForDefaultConstructor(e)) + : method; + + JS.Node annotateVariable(JS.Node node, VariableElement e) => + options.closure && e != null + ? node.withClosureAnnotation(closureAnnotationForVariable(e)) + : node; + + JS.Node annotateTypeDef(JS.Node node, FunctionTypeAliasElement e) => + options.closure && e != null + ? node.withClosureAnnotation(closureAnnotationForTypeDef(e)) + : node; } class JSGenerator extends CodeGenerator { diff --git a/lib/src/js/js_ast.dart b/lib/src/js/js_ast.dart index ee8289ec..a21472c9 100644 --- a/lib/src/js/js_ast.dart +++ b/lib/src/js/js_ast.dart @@ -4,6 +4,8 @@ library js_ast; +import 'package:dev_compiler/src/closure/closure_annotation.dart'; + import 'precedence.dart'; import 'characters.dart' as charCodes; diff --git a/lib/src/js/nodes.dart b/lib/src/js/nodes.dart index b8ebd315..5f099872 100644 --- a/lib/src/js/nodes.dart +++ b/lib/src/js/nodes.dart @@ -204,6 +204,10 @@ abstract class Node { /// setting this after construction. Object sourceInformation; + ClosureAnnotation _closureAnnotation; + /// Closure annotation of this node. + ClosureAnnotation get closureAnnotation => _closureAnnotation; + accept(NodeVisitor visitor); void visitChildren(NodeVisitor visitor); @@ -211,6 +215,13 @@ abstract class Node { // private method is create a copy with a new position. Node _clone(); + withClosureAnnotation(ClosureAnnotation closureAnnotation) { + if (this.closureAnnotation == closureAnnotation) return this; + + return _clone() + ..sourceInformation = sourceInformation + .._closureAnnotation = closureAnnotation; + } // Returns a node equivalent to [this], but with new source position and end // source position. Node withSourceInformation(sourceInformation) { diff --git a/lib/src/js/printer.dart b/lib/src/js/printer.dart index 39ac5d01..fd7e4227 100644 --- a/lib/src/js/printer.dart +++ b/lib/src/js/printer.dart @@ -268,6 +268,7 @@ class Printer implements NodeVisitor { visitExpressionStatement(ExpressionStatement expressionStatement) { indent(); + outClosureAnnotation(expressionStatement); visitNestedExpression(expressionStatement.expression, EXPRESSION, newInForInit: false, newAtStatementBegin: true); outSemicolonLn(); @@ -548,6 +549,7 @@ class Printer implements NodeVisitor { visitFunctionDeclaration(FunctionDeclaration declaration) { indent(); + outClosureAnnotation(declaration); functionOut(declaration.function, declaration.name); lineOut(); } @@ -586,6 +588,7 @@ class Printer implements NodeVisitor { } visitVariableDeclarationList(VariableDeclarationList list) { + outClosureAnnotation(list); out(list.keyword); out(" "); visitCommaSeparated(list.declarations, ASSIGNMENT, @@ -609,6 +612,7 @@ class Printer implements NodeVisitor { } visitVariableInitialization(VariableInitialization initialization) { + outClosureAnnotation(initialization); visitAssignment(initialization); } @@ -1053,6 +1057,7 @@ class Printer implements NodeVisitor { } visitMethod(Method node) { + outClosureAnnotation(node); if (node.isStatic) { out('static '); } @@ -1083,6 +1088,17 @@ class Printer implements NodeVisitor { localNamer.leaveScope(); } + void outClosureAnnotation(Node node) { + if (node != null && node.closureAnnotation != null) { + String comment = node.closureAnnotation.toString(indentation); + if (comment.isNotEmpty) { + out(comment); + lineOut(); + indent(); + } + } + } + void propertyNameOut(Expression node, {bool inMethod: false, bool inAccess: false}) { diff --git a/lib/src/options.dart b/lib/src/options.dart index b99dd019..b152e5f4 100644 --- a/lib/src/options.dart +++ b/lib/src/options.dart @@ -15,6 +15,8 @@ import 'package:yaml/yaml.dart'; import 'package:dev_compiler/strong_mode.dart' show StrongModeOptions; +const bool _CLOSURE_DEFAULT = false; + /// Options used to set up Source URI resolution in the analysis context. class SourceResolverOptions { /// Whether to resolve 'package:' uris using the multi-package resolver. @@ -65,6 +67,9 @@ class CodegenOptions { /// Output directory for generated code. final String outputDir; + /// Emit Closure Compiler-friendly code. + final bool closure; + /// Whether to emit a workaround for missing arrow function bind-this in /// other V8 builds final bool arrowFnBindThisWorkaround; @@ -72,6 +77,7 @@ class CodegenOptions { const CodegenOptions( {this.emitSourceMaps: true, this.forceCompile: false, + this.closure: _CLOSURE_DEFAULT, this.outputDir, this.arrowFnBindThisWorkaround: false}); } @@ -213,6 +219,7 @@ CompilerOptions parseOptions(List argv, {bool forceOutDir: false}) { codegenOptions: new CodegenOptions( emitSourceMaps: args['source-maps'], forceCompile: args['force-compile'] || serverMode, + closure: args['closure'], outputDir: outputDir, arrowFnBindThisWorkaround: args['arrow-fn-bind-this']), sourceOptions: new SourceResolverOptions( @@ -294,6 +301,8 @@ final ArgParser argParser = StrongModeOptions.addArguments(new ArgParser() ..addOption('port', help: 'Port to serve files from (used only when --serve is on)', defaultsTo: '8080') + ..addFlag('closure', + help: 'Emit Closure Compiler-friendly code (experimental)', defaultsTo: _CLOSURE_DEFAULT) ..addFlag('force-compile', help: 'Compile code with static errors', defaultsTo: false) ..addOption('log', abbr: 'l', help: 'Logging level (defaults to severe)') diff --git a/test/all_tests.dart b/test/all_tests.dart index 6a227b1e..bd974447 100644 --- a/test/all_tests.dart +++ b/test/all_tests.dart @@ -10,6 +10,8 @@ import 'package:test/test.dart'; import 'checker/checker_test.dart' as checker_test; import 'checker/inferred_type_test.dart' as inferred_type_test; import 'checker/self_host_test.dart' as self_host; +import 'closure/closure_annotation_test.dart' as closure_annotation_test; +import 'closure/closure_type_test.dart' as closure_type_test; import 'codegen_test.dart' as codegen_test; import 'end_to_end_test.dart' as e2e; import 'report_test.dart' as report_test; @@ -23,4 +25,8 @@ void main() { group('dependency_graph', dependency_graph_test.main); group('codegen', () => codegen_test.main([])); group('self_host', self_host.main); + group('closure', () { + closure_annotation_test.main(); + closure_type_test.main(); + }); } diff --git a/test/closure/closure_annotation_test.dart b/test/closure/closure_annotation_test.dart new file mode 100644 index 00000000..8793b073 --- /dev/null +++ b/test/closure/closure_annotation_test.dart @@ -0,0 +1,123 @@ +// 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. + +library dev_compiler.test.closure_annotation_test; + +import 'package:test/test.dart'; + +import 'package:dev_compiler/src/closure/closure_annotation.dart'; +import 'package:dev_compiler/src/closure/closure_type.dart'; + +void main() { + group('ClosureAnnotation', () { + var allType = new ClosureType.all(); + var unknownType = new ClosureType.unknown(); + var numberType = new ClosureType.number(); + var stringType = new ClosureType.string(); + var booleanType = new ClosureType.boolean(); + var fooType = new ClosureType.type("foo.Foo"); + var barType = new ClosureType.type("bar.Bar"); + var bazType = new ClosureType.type("baz.Baz"); + var bamType = new ClosureType.type("bam.Bam"); + var batType = new ClosureType.type("bat.Bat"); + + test('gives empty comment when no has no meaningful info', () { + expect(new ClosureAnnotation().toString(), ""); + expect(new ClosureAnnotation(type: allType).toString(), ""); + expect(new ClosureAnnotation(type: unknownType).toString(), ""); + }); + + test('gives single line comment when it fits', () { + expect(new ClosureAnnotation(type: numberType).toString(), + "/** @type {number} */"); + expect(new ClosureAnnotation(paramTypes: {'foo': allType}).toString(), + "/** @param {*} foo */"); + expect(new ClosureAnnotation(paramTypes: {'foo': unknownType}).toString(), + "/** @param {?} foo */"); + }); + + test('gives multiple line comment when it it does not fit on one line', () { + expect(new ClosureAnnotation( + returnType: stringType, paramTypes: {'foo': numberType}) + .toString(), + "/**\n" + " * @param {number} foo\n" + " * @return {string}\n" + " */"); + }); + + test('inserts indentation', () { + expect(new ClosureAnnotation( + returnType: stringType, paramTypes: {'foo': numberType}) + .toString(" "), + "/**\n" // No indent on first line. + " * @param {number} foo\n" + " * @return {string}\n" + " */"); + }); + + test('compresses @type, @final, @const, @private, @protected, @typedef', + () { + expect(new ClosureAnnotation(type: stringType).toString(), + "/** @type {string} */"); + expect(new ClosureAnnotation(type: stringType, isConst: true).toString(), + "/** @const {string} */"); + expect(new ClosureAnnotation(type: stringType, isFinal: true).toString(), + "/** @final {string} */"); + expect( + new ClosureAnnotation(type: stringType, isPrivate: true).toString(), + "/** @private {string} */"); + expect( + new ClosureAnnotation(type: stringType, isTypedef: true).toString(), + "/** @typedef {string} */"); + expect( + new ClosureAnnotation(type: stringType, isProtected: true).toString(), + "/** @protected {string} */"); + expect(new ClosureAnnotation( + type: stringType, + isPrivate: true, + isConst: true, + isFinal: true, + isProtected: true, + isTypedef: true).toString(), + "/** @private @protected @final @const @typedef {string} */"); + }); + + test('supports a full constructor annotation', () { + expect(new ClosureAnnotation( + returnType: booleanType, + throwsType: bamType, + thisType: fooType, + superType: barType, + lendsToType: batType, + interfaces: [bazType], + isStruct: true, + isPrivate: true, + isProtected: true, + isOverride: true, + isFinal: true, + isConst: true, + isConstructor: true, + isNoSideEffects: true, + isNoCollapse: true, + paramTypes: {'x': stringType, 'y': numberType}, + templates: ['A', 'B']).toString(), + '/**\n' + ' * @template A, B\n' + ' * @this {foo.Foo}\n' + ' * @override\n' + ' * @nosideeffects\n' + ' * @nocollapse\n' + ' * @lends {bat.Bat}\n' + ' * @private @protected @final @const\n' + ' * @constructor @struct @extends {bar.Bar}\n' + ' * @implements {baz.Baz}\n' + ' * @param {string} x\n' + ' * @param {number} y\n' + ' * @return {boolean}\n' + ' * @throws {bam.Bam}\n' + ' */'); + }); + }); +} diff --git a/test/closure/closure_type_test.dart b/test/closure/closure_type_test.dart new file mode 100644 index 00000000..93a9e3d3 --- /dev/null +++ b/test/closure/closure_type_test.dart @@ -0,0 +1,82 @@ +// 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. + +library dev_compiler.test.closure_type_test; + +import 'package:test/test.dart'; + +import 'package:dev_compiler/src/closure/closure_type.dart'; + +void main() { + expectToString(ClosureType t, String s, + {String nullable, String nonNullable}) { + expect(t.toString(), s); + if (nullable != null) { + expect(t.toNullable().toString(), nullable); + } + if (nonNullable != null) { + expect(t.toNonNullable().toString(), nonNullable); + } + } + + group('ClosureType', () { + test('supports simple types', () { + expectToString(new ClosureType.number(), "number", + nullable: "?number", nonNullable: "number"); + expectToString(new ClosureType.boolean(), "boolean", + nullable: "?boolean", nonNullable: "boolean"); + expectToString(new ClosureType.string(), "string", + nullable: "string", nonNullable: "!string"); + expectToString(new ClosureType.type("foo.Bar"), "foo.Bar", + nullable: "foo.Bar", nonNullable: "!foo.Bar"); + }); + + test('supports array types', () { + expectToString(new ClosureType.array(), "Array<*>", + nullable: "Array<*>", nonNullable: "!Array<*>"); + expectToString( + new ClosureType.array(new ClosureType.type("Foo")), "Array", + nullable: "Array", nonNullable: "!Array"); + }); + + test('supports map types', () { + expectToString(new ClosureType.map( + new ClosureType.type("Foo"), new ClosureType.type("Bar")), + "Object", + nullable: "Object", nonNullable: "!Object"); + expectToString(new ClosureType.map(), "Object<*, *>", + nullable: "Object<*, *>", nonNullable: "!Object<*, *>"); + }); + + test('supports function types', () { + expectToString(new ClosureType.function(), "Function", + nullable: "Function", nonNullable: "!Function"); + expectToString(new ClosureType.function([new ClosureType.number()]), + "function(number)"); + expectToString(new ClosureType.function(null, new ClosureType.number()), + "function(...*):number"); + expectToString(new ClosureType.function([ + new ClosureType.number(), + new ClosureType.string() + ], new ClosureType.boolean()), "function(number, string):boolean"); + }); + + test('supports union types', () { + expectToString(new ClosureType.number().or(new ClosureType.boolean()), + "(number|boolean)"); + expectToString( + new ClosureType.number().orUndefined(), "(number|undefined)"); + }); + + test('supports record types', () { + expectToString(new ClosureType.record( + {'x': new ClosureType.number(), 'y': new ClosureType.boolean()}), + "{x: number, y: boolean}"); + }); + + test('supports optional pseudo-types', () { + expectToString(new ClosureType.number().toOptional(), "number="); + }); + }); +} diff --git a/test/codegen/closure.dart b/test/codegen/closure.dart new file mode 100644 index 00000000..d40e8a75 --- /dev/null +++ b/test/codegen/closure.dart @@ -0,0 +1,59 @@ +library test; +import 'dart:js'; + + +typedef void Callback({int i}); + +class Foo { + final int i; + bool b; + String s; + T v; + + Foo(this.i, this.v); + + factory Foo.build() => new Foo(1, null); + + untyped_method(a, b) {} + + T pass(T t) => t; + + String typed_method( + Foo foo, List list, + int i, num n, double d, bool b, String s, + JsArray a, JsObject o, JsFunction f) { + return ''; + } + + optional_params(a, [b, c]) {} + + static named_params(a, {b, c}) {} + + nullary_method() {} + + function_params(int f(x, [y]), g(x, {String y, z}), Callback cb) { + cb(i: i); + } + + String get prop => null; + set prop(String value) {} + + static String get staticProp => null; + static set staticProp(String value) {} + + static const String some_static_constant = "abc"; + static final String some_static_final = "abc"; + static String some_static_var = "abc"; +} + +class Bar {} + +class Baz extends Foo with Bar { + Baz(int i) : super(i, 123); +} + +void main(args) {} + +const String some_top_level_constant = "abc"; +final String some_top_level_final = "abc"; +String some_top_level_var = "abc"; \ No newline at end of file diff --git a/test/codegen/expect/closure.js b/test/codegen/expect/closure.js new file mode 100644 index 00000000..0dbfa377 --- /dev/null +++ b/test/codegen/expect/closure.js @@ -0,0 +1,149 @@ +dart_library.library('closure', null, /* Imports */[ + "dart_runtime/dart", + 'dart/core', + 'dart/js' +], /* Lazy imports */[ +], function(exports, dart, core, js) { + 'use strict'; + let dartx = dart.dartx; + /** @typedef {function({i: (?number|undefined)}=)} */ + let Callback = dart.typedef('Callback', () => dart.functionType(dart.void, [], {i: core.int})); + let Foo$ = dart.generic(function(T) { + class Foo extends core.Object { + /** + * @param {?number} i + * @param {?} v + */ + Foo(i, v) { + this.i = i; + this.v = v; + this.b = null; + this.s = null; + } + /** @return {Foo} */ + static build() { + return new (Foo$(T))(1, null); + } + /** + * @param {?} a + * @param {?} b + */ + untyped_method(a, b) {} + /** @param {?} t */ + pass(t) { + dart.as(t, T); + return t; + } + /** + * @param {Foo} foo + * @param {core.List} list + * @param {?number} i + * @param {?number} n + * @param {?number} d + * @param {?boolean} b + * @param {string} s + * @param {Array} a + * @param {Object<*, *>} o + * @param {Function} f + * @return {string} + */ + typed_method(foo, list, i, n, d, b, s, a, o, f) { + return ''; + } + /** + * @param {?} a + * @param {?=} b + * @param {?=} c + */ + optional_params(a, b, c) { + if (b === void 0) + b = null; + if (c === void 0) + c = null; + } + /** + * @param {?} a + * @param {{b: (?|undefined), c: (?|undefined)}=} opts + */ + static named_params(a, opts) { + let b = opts && 'b' in opts ? opts.b : null; + let c = opts && 'c' in opts ? opts.c : null; + } + nullary_method() {} + /** + * @param {function(?, ?=):?number} f + * @param {function(?, {y: (string|undefined), z: (?|undefined)}=):?} g + * @param {Callback} cb + */ + function_params(f, g, cb) { + dart.as(f, dart.functionType(core.int, [dart.dynamic], [dart.dynamic])); + dart.as(g, dart.functionType(dart.dynamic, [dart.dynamic], {y: core.String, z: dart.dynamic})); + cb({i: this.i}); + } + /** @return {string} */ + get prop() { + return null; + } + /** @param {string} value */ + set prop(value) {} + /** @return {string} */ + static get staticProp() { + return null; + } + /** @param {string} value */ + static set staticProp(value) {} + } + dart.setSignature(Foo, { + constructors: () => ({ + Foo: [Foo$(T), [core.int, T]], + build: [Foo$(T), []] + }), + methods: () => ({ + untyped_method: [dart.dynamic, [dart.dynamic, dart.dynamic]], + pass: [T, [T]], + typed_method: [core.String, [Foo$(), core.List, core.int, core.num, core.double, core.bool, core.String, js.JsArray, js.JsObject, js.JsFunction]], + optional_params: [dart.dynamic, [dart.dynamic], [dart.dynamic, dart.dynamic]], + nullary_method: [dart.dynamic, []], + function_params: [dart.dynamic, [dart.functionType(core.int, [dart.dynamic], [dart.dynamic]), dart.functionType(dart.dynamic, [dart.dynamic], {y: core.String, z: dart.dynamic}), Callback]] + }), + statics: () => ({named_params: [dart.dynamic, [dart.dynamic], {b: dart.dynamic, c: dart.dynamic}]}), + names: ['named_params'] + }); + return Foo; + }); + let Foo = Foo$(); + /** @final {string} */ + Foo.some_static_constant = "abc"; + /** @final {string} */ + Foo.some_static_final = "abc"; + /** @type {string} */ + Foo.some_static_var = "abc"; + class Bar extends core.Object {} + class Baz extends dart.mixin(Foo$(core.int), Bar) { + /** @param {?number} i */ + Baz(i) { + super.Foo(i, 123); + } + } + dart.setSignature(Baz, { + constructors: () => ({Baz: [Baz, [core.int]]}) + }); + /** @param {?} args */ + function main(args) { + } + dart.fn(main, dart.void, [dart.dynamic]); + /** @final {string} */ + let some_top_level_constant = "abc"; + /** @final {string} */ + exports.some_top_level_final = "abc"; + /** @type {string} */ + exports.some_top_level_var = "abc"; + // Exports: + exports.Callback = Callback; + exports.Foo$ = Foo$; + exports.Foo = Foo; + exports.Bar = Bar; + exports.Baz = Baz; + exports.main = main; + exports.some_top_level_constant = some_top_level_constant; +}); diff --git a/test/codegen/expect/closure.txt b/test/codegen/expect/closure.txt new file mode 100644 index 00000000..1a2a51d2 --- /dev/null +++ b/test/codegen/expect/closure.txt @@ -0,0 +1,2 @@ +// Messages from compiling closure.dart +info: [InferredTypeAllocation] new Foo(1, null) has inferred type Foo (test/codegen/closure.dart, line 15, col 26) diff --git a/test/codegen_test.dart b/test/codegen_test.dart index c8fa1f6e..35715b29 100644 --- a/test/codegen_test.dart +++ b/test/codegen_test.dart @@ -58,7 +58,8 @@ main(arguments) { var expectDir = path.join(inputDir, 'expect'); bool compile(String entryPoint, AnalysisContext context, - {bool checkSdk: false, bool sourceMaps: false}) { + {bool checkSdk: false, bool sourceMaps: false, + bool closure: false}) { // TODO(jmesserly): add a way to specify flags in the test file, so // they're more self-contained. var runtimeDir = path.join(path.dirname(testDirectory), 'lib', 'runtime'); @@ -66,6 +67,7 @@ main(arguments) { codegenOptions: new CodegenOptions( outputDir: expectDir, emitSourceMaps: sourceMaps, + closure: closure, forceCompile: checkSdk), useColors: false, checkSdk: checkSdk, @@ -138,10 +140,13 @@ main(arguments) { compilerMessages.writeln('// Messages from compiling $filename.dart'); // TODO(jmesserly): this was added to get some coverage of source maps + // and closure annotations. // We need a more comprehensive strategy to test them. var sourceMaps = filename == 'map_keys'; + var closure = filename == 'closure'; var success = - compile(filePath, realSdkContext, sourceMaps: sourceMaps); + compile(filePath, realSdkContext, sourceMaps: sourceMaps, + closure: closure); // Write compiler messages to disk. new File(path.join(outDir, '$filename.txt')) diff --git a/tool/build_sdk.sh b/tool/build_sdk.sh index 9e3dde2f..bca4405e 100755 --- a/tool/build_sdk.sh +++ b/tool/build_sdk.sh @@ -17,6 +17,7 @@ fi # https://github.com/dart-lang/dev_compiler/issues/219 dart -c bin/devc.dart --no-source-maps --arrow-fn-bind-this --sdk-check \ --force-compile -l warning --dart-sdk tool/generated_sdk -o lib/runtime/ \ + "$@" \ dart:js dart:mirrors \ > tool/sdk_expected_errors.txt || true