Compile package:js.{rest, spread} helpers.

Note: need new package:js release before merging this in (#24623). Maybe it's time to think again about sneaking JsName + JsPeerInterface into it? (dart-archive/dev_compiler#135)

BUG=dart-archive/dev_compiler#310
[email protected]

Review URL: https://codereview.chromium.org/1424133007 .

Author: Olivier Chafik

pkg/dev_compiler/lib/src/codegen/js_codegen.dart

@@ -31,6 +31,7 @@ import '../utils.dart';
 
 import 'code_generator.dart';
 import 'js_field_storage.dart';
+import 'js_interop.dart';
 import 'js_names.dart' as JS;
 import 'js_metalet.dart' as JS;
 import 'js_module_item_order.dart';
@@ -247,7 +248,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
   void visitLibraryDirective(LibraryDirective node) {
     assert(_jsModuleValue == null);
 
-    var jsName = findAnnotation(node.element, _isJsNameAnnotation);
+    var jsName = findAnnotation(node.element, isJsNameAnnotation);
     _jsModuleValue =
         getConstantField(jsName, 'name', types.stringType)?.toStringValue();
   }
@@ -420,7 +421,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
   JS.Statement visitClassDeclaration(ClassDeclaration node) {
     var classElem = node.element;
     var type = classElem.type;
-    var jsName = findAnnotation(classElem, _isJsNameAnnotation);
+    var jsName = findAnnotation(classElem, isJsNameAnnotation);
 
     if (jsName != null) return _emitJsType(node.name.name, jsName);
 
@@ -441,7 +442,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
         _classHeritage(classElem), _emitClassMethods(node, ctors, fields));
 
     String jsPeerName;
-    var jsPeer = findAnnotation(classElem, _isJsPeerInterface);
+    var jsPeer = findAnnotation(classElem, isJsPeerInterface);
     if (jsPeer != null) {
       jsPeerName =
           getConstantField(jsPeer, 'name', types.stringType)?.toStringValue();
@@ -596,7 +597,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
       jsMethods.add(_emitImplicitConstructor(node, fields));
     }
 
-    bool hasJsPeer = findAnnotation(element, _isJsPeerInterface) != null;
+    bool hasJsPeer = findAnnotation(element, isJsPeerInterface) != null;
 
     bool hasIterator = false;
     for (var m in node.members) {
@@ -1853,6 +1854,9 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
     for (var arg in node.arguments) {
       if (arg is NamedExpression) {
         named.add(_visit(arg));
+      } else if (arg is MethodInvocation && isJsSpreadInvocation(arg)) {
+        args.add(
+            new JS.RestParameter(_visit(arg.argumentList.arguments.single)));
       } else {
         args.add(_visit(arg));
       }
@@ -1871,8 +1875,8 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
   }
 
   @override
-  List<JS.Identifier> visitFormalParameterList(FormalParameterList node) {
-    var result = <JS.Identifier>[];
+  List<JS.Parameter> visitFormalParameterList(FormalParameterList node) {
+    var result = <JS.Parameter>[];
     for (FormalParameter param in node.parameters) {
       if (param.kind == ParameterKind.NAMED) {
         result.add(_namedArgTemp);
@@ -2519,8 +2523,12 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
       _visit(node.expression);
 
   @override
-  visitFormalParameter(FormalParameter node) =>
-      visitSimpleIdentifier(node.identifier);
+  visitFormalParameter(FormalParameter node) {
+    var id = visitSimpleIdentifier(node.identifier);
+
+    var isRestArg = findAnnotation(node.element, isJsRestAnnotation) != null;
+    return isRestArg ? new JS.RestParameter(id) : id;
+  }
 
   @override
   JS.This visitThisExpression(ThisExpression node) => new JS.This();
@@ -2682,7 +2690,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ClosureAnnotator {
   // TODO(jmesserly): ideally we'd check the method and see if it is marked
   // `external`, but that doesn't work because it isn't in the element model.
   bool _useNativeJsIndexer(DartType type) =>
-      findAnnotation(type.element, _isJsNameAnnotation) != null;
+      findAnnotation(type.element, isJsNameAnnotation) != null;
 
   /// Gets the target of a [PropertyAccess], [IndexExpression], or
   /// [MethodInvocation]. These three nodes can appear in a [CascadeExpression].
@@ -3301,12 +3309,6 @@ String jsLibraryName(LibraryElement library) => canonicalLibraryName(library);
 // TODO(jmesserly): avoid the round tripping through quoted form.
 JS.LiteralString _propertyName(String name) => js.string(name, "'");
 
-// TODO(jmesserly): validate the library. See issue #135.
-bool _isJsNameAnnotation(DartObject value) => value.type.name == 'JsName';
-
-bool _isJsPeerInterface(DartObject value) =>
-    value.type.name == 'JsPeerInterface';
-
 // TODO(jacobr): we would like to do something like the following
 // but we don't have summary support yet.
 // bool _supportJsExtensionMethod(AnnotatedNode node) =>

pkg/dev_compiler/lib/src/codegen/js_interop.dart

@@ -0,0 +1,31 @@
+// 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.js_interop;
+
+import 'package:analyzer/src/generated/ast.dart';
+import 'package:analyzer/src/generated/element.dart';
+import 'package:analyzer/src/generated/constant.dart';
+
+bool _isJsLibType(String expectedName, Element e) =>
+    e?.name == expectedName && _isJsLib(e.library);
+
+bool _isJsLib(LibraryElement e) {
+  var libName = e?.name;
+  return libName == 'js' ||
+      libName == 'js.varargs' ||
+      libName == 'dart._js_helper';
+}
+
+bool isJsRestAnnotation(DartObjectImpl value) =>
+    _isJsLibType('_Rest', value.type.element);
+
+bool isJsSpreadInvocation(MethodInvocation i) =>
+    _isJsLibType('spread', i.methodName?.bestElement);
+
+// TODO(jmesserly): Move JsName, JsPeerInterface to package:js (see issue #135).
+bool isJsNameAnnotation(DartObjectImpl value) => value.type.name == 'JsName';
+
+bool isJsPeerInterface(DartObjectImpl value) =>
+    value.type.name == 'JsPeerInterface';

pkg/dev_compiler/lib/src/compiler.dart

@@ -479,9 +479,11 @@ abstract class AbstractCompiler {
 
 AnalysisErrorListener createErrorReporter(
     AnalysisContext context, CompilerOptions options) {
-  return options.htmlReport ? new HtmlReporter(context) : options.dumpInfo
-      ? new SummaryReporter(context, options.logLevel)
-      : new LogReporter(context, useColors: options.useColors);
+  return options.htmlReport
+      ? new HtmlReporter(context)
+      : options.dumpInfo
+          ? new SummaryReporter(context, options.logLevel)
+          : new LogReporter(context, useColors: options.useColors);
 }
 
 // TODO(jmesserly): find a better home for these.

pkg/dev_compiler/pubspec.yaml

@@ -14,6 +14,7 @@ dependencies:
   cli_util: ^0.0.1
   crypto: ^0.9.0
   html: ^0.12.0
+  js: ^0.6.0-beta.6
   logging: ">=0.9.2 <0.12.0"
   path: ^1.3.0
   pub_semver: ^1.1.0

pkg/dev_compiler/test/codegen/expect/js/src/varargs.js

@@ -0,0 +1,23 @@
+dart_library.library('js/src/varargs', null, /* Imports */[
+  "dart/_runtime",
+  'dart/core'
+], /* Lazy imports */[
+], function(exports, dart, core) {
+  'use strict';
+  let dartx = dart.dartx;
+  class _Rest extends core.Object {
+    _Rest() {
+    }
+  }
+  dart.setSignature(_Rest, {
+    constructors: () => ({_Rest: [_Rest, []]})
+  });
+  let rest = dart.const(new _Rest());
+  function spread(args) {
+    dart.throw(new core.StateError('The spread function cannot be called, ' + 'it should be compiled away.'));
+  }
+  dart.fn(spread);
+  // Exports:
+  exports.rest = rest;
+  exports.spread = spread;
+});

pkg/dev_compiler/test/codegen/expect/js/src/varargs.txt

@@ -0,0 +1 @@
+// Messages from compiling varargs.dart

pkg/dev_compiler/test/codegen/expect/varargs.js

@@ -0,0 +1,20 @@
+dart_library.library('varargs', null, /* Imports */[
+  "dart/_runtime"
+], /* Lazy imports */[
+], function(exports, dart) {
+  'use strict';
+  let dartx = dart.dartx;
+  function varargsTest(x, ...others) {
+    let args = [1, others];
+    dart.dsend(x, 'call', ...args);
+  }
+  dart.fn(varargsTest);
+  function varargsTest2(x, ...others) {
+    let args = [1, others];
+    dart.dsend(x, 'call', ...args);
+  }
+  dart.fn(varargsTest2);
+  // Exports:
+  exports.varargsTest = varargsTest;
+  exports.varargsTest2 = varargsTest2;
+});

pkg/dev_compiler/test/codegen/expect/varargs.txt

@@ -0,0 +1 @@
+// Messages from compiling varargs.dart

pkg/dev_compiler/test/codegen/varargs.dart

@@ -0,0 +1,12 @@
+import 'package:js/src/varargs.dart' as js;
+import 'package:js/src/varargs.dart' show rest, spread;
+
+varargsTest(x, @js.rest others) {
+  var args = [1, others];
+  x.call(js.spread(args));
+}
+
+varargsTest2(x, @rest others) {
+  var args = [1, others];
+  x.call(spread(args));
+}

pkg/dev_compiler/tool/input_sdk/private/js_helper.dart

@@ -24,7 +24,6 @@ class _Patch {
 
 const _Patch patch = const _Patch();
 
-
 /// Marks the internal map in dart2js, so that internal libraries can is-check
 // them.
 abstract class InternalMap {
@@ -176,7 +175,7 @@ class Primitives {
   /// In minified mode, uses the unminified names if available.
   static String objectToString(Object object) {
     // String name = objectTypeName(object);
-    String name = JS('String', 'dart.typeName(dart.realRuntimeType(#))', object); 
+    String name = JS('String', 'dart.typeName(dart.realRuntimeType(#))', object);
     return "Instance of '$name'";
   }
 

pkg/dev_compiler/tool/sdk_expected_errors.txt

@@ -15,8 +15,8 @@ warning: [DownCastComposite] IterableMixinWorkaround.firstWhere(this, test, orEl
 warning: [DownCastComposite] IterableMixinWorkaround.lastWhereList(this, test, orElse) (dynamic) will need runtime check to cast to type E (dart:_interceptors/js_array.dart, line 173, col 12)
 warning: [DownCastComposite] IterableMixinWorkaround.singleWhere(this, test) (dynamic) will need runtime check to cast to type E (dart:_interceptors/js_array.dart, line 177, col 12)
 warning: [DownCastComposite] JS('var', '#[#]', this, index) (dynamic) will need runtime check to cast to type E (dart:_interceptors/js_array.dart, line 313, col 12)
-warning: [DownCastComposite] charCodes (dynamic) will need runtime check to cast to type List<int> (dart:_js_helper, line 265, col 31)
-warning: [DownCastComposite] JS('', '#.value', ret) (dynamic) will need runtime check to cast to type E (dart:_js_helper, line 780, col 16)
+warning: [DownCastComposite] charCodes (dynamic) will need runtime check to cast to type List<int> (dart:_js_helper, line 264, col 31)
+warning: [DownCastComposite] JS('', '#.value', ret) (dynamic) will need runtime check to cast to type E (dart:_js_helper, line 779, col 16)
 warning: [DownCastComposite] JS('JSExtendableArray', r'#.split(#)', this, pattern) (dynamic) will need runtime check to cast to type List<String> (dart:_interceptors/js_string.dart, line 86, col 14)
 warning: [DownCastComposite] JS('JSExtendableArray', r'#.split(#)', this, re) (dynamic) will need runtime check to cast to type List<String> (dart:_interceptors/js_string.dart, line 89, col 14)
 warning: [DownCastComposite] JS('JSExtendableArray|Null', r'#.exec(#)', _nativeRegExp, checkString(string)) (dynamic) will need runtime check to cast to type List<String> (dart:_js_helper/regexp_helper.dart, line 108, col 22)