simplify generation of functions

lib/src/codegen/js_codegen.dart

 // 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.
 
 import 'dart:collection' show HashSet, HashMap, SplayTreeSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/constant.dart';
@@ skipping 412 matching lines @@
         ctors.add(member);
       } else if (member is FieldDeclaration) {
         (member.isStatic ? staticFields : fields).add(member);
       } else if (member is MethodDeclaration) {
         methods.add(member);
       }
     }
 
     var classExpr = new JS.ClassExpression(new JS.Identifier(type.name),
         _classHeritage(classElem), _emitClassMethods(node, ctors, fields),
-        typeParams: _emitTypeParams(classElem).toList(),
+        typeParams: _emitTypeFormals(classElem.typeParameters),
         fields:
             _emitFieldDeclarations(classElem, fields, staticFields).toList());
 
     String jsPeerName;
     var jsPeer = findAnnotation(classElem, isJsPeerInterface);
     // Only look at "Native" annotations on registered extension types.
     // E.g., we're current ignoring the ones in dart:html.
     if (jsPeer == null && _extensionTypes.contains(classElem)) {
       jsPeer = findAnnotation(classElem, isNativeAnnotation);
     }
@@ skipping 22 matching lines @@
       // want to support construction of instances with generic types other
       // than dynamic. See issue #154 for Array and List<E> related bug.
       var copyMembers = js.statement(
           'dart.registerExtension(dart.global.#, #);',
           [_propertyName(jsPeerName), classElem.name]);
       return _statement([result, copyMembers]);
     }
     return result;
   }
 
-  Iterable<JS.Identifier> _emitTypeParams(TypeParameterizedElement e) sync* {
-    if (!options.closure) return;
-    for (var typeParam in e.typeParameters) {
-      yield new JS.Identifier(typeParam.name);
-    }
+  List<JS.Identifier> _emitTypeFormals(List<TypeParameterElement> typeFormals) {
+    if (!options.closure) return null;
+    return typeFormals
+        .map((t) => new JS.Identifier(t.name))
+        .toList(growable: false);
   }
 
   /// Emit field declarations for TypeScript & Closure's ES6_TYPED
   /// (e.g. `class Foo { i: string; }`)
   Iterable<JS.VariableDeclarationList> _emitFieldDeclarations(
       ClassElement classElem,
       List<FieldDeclaration> fields,
       List<FieldDeclaration> staticFields) sync* {
     if (!options.closure) return;
 
@@ skipping 559 matching lines @@
     var returnType = emitTypeRef(node.element.enclosingElement.type);
 
     // Wacky factory redirecting constructors: factory Foo.q(x, y) = Bar.baz;
     var redirect = node.redirectedConstructor;
     if (redirect != null) {
       var newKeyword = redirect.staticElement.isFactory ? '' : 'new';
       // Pass along all arguments verbatim, and let the callee handle them.
       // TODO(jmesserly): we'll need something different once we have
       // rest/spread support, but this should work for now.
       var params =
-          _emitFormalParameterList(node.parameters, allowDestructuring: false);
+          visitFormalParameterList(node.parameters, destructure: false);
 
       var fun = new JS.Fun(
           params,
           js.statement(
               '{ return $newKeyword #(#); }', [_visit(redirect), params]),
           returnType: returnType);
       return annotate(
           new JS.Method(name, fun, isStatic: true), node, node.element);
     }
 
     // For const constructors we need to ensure default values are
     // available for use by top-level constant initializers.
     ClassDeclaration cls = node.parent;
     if (node.constKeyword != null) _loader.startTopLevel(cls.element);
-    var params = _emitFormalParameterList(node.parameters);
+    var params = visitFormalParameterList(node.parameters);
     if (node.constKeyword != null) _loader.finishTopLevel(cls.element);
 
     // Factory constructors are essentially static methods.
     if (node.factoryKeyword != null) {
       var body = <JS.Statement>[];
       var init = _emitArgumentInitializers(node, constructor: true);
       if (init != null) body.add(init);
       body.add(_visit(node.body));
       var fun = new JS.Fun(params, new JS.Block(body), returnType: returnType);
       return annotate(
@@ skipping 285 matching lines @@
       t is ParameterizedType && t.typeArguments.any(_hasUnsoundTypeParameter);
 
   JS.Expression _defaultParamValue(FormalParameter param) {
     if (param is DefaultFormalParameter && param.defaultValue != null) {
       return _visit(param.defaultValue);
     } else {
       return new JS.LiteralNull();
     }
   }
 
-  JS.Fun _emitNativeFunctionBody(List<JS.Parameter> params,
-      List<JS.Expression> paramRefs, MethodDeclaration node) {
+  JS.Fun _emitNativeFunctionBody(MethodDeclaration node) {
     if (node.isStatic) {
       // TODO(vsm): Do we need to handle this case?
       return null;
     }
 
+    var params = visitFormalParameterList(node.parameters, destructure: false);
     String name = node.name.name;
     var annotation = findAnnotation(node.element, isJsName);
     if (annotation != null) {
       name = getConstantField(annotation, 'name', types.stringType)
           ?.toStringValue();
     }
     if (node.isGetter) {
       return new JS.Fun(params, js.statement('{ return this.#; }', [name]));
     } else if (node.isSetter) {
       return new JS.Fun(
-          params, js.statement('{ this.# = #; }', [name, paramRefs.last]));
+          params, js.statement('{ this.# = #; }', [name, params.last]));
     } else {
       return new JS.Fun(
-          params, js.statement('{ return this.#(#); }', [name, paramRefs]));
+          params, js.statement('{ return this.#(#); }', [name, params]));
     }
   }
 
   JS.Method _emitMethodDeclaration(DartType type, MethodDeclaration node) {
     if (node.isAbstract) {
       return null;
     }
 
-    var params = _visit(node.parameters) as List<JS.Parameter>;
-    if (params == null) params = <JS.Parameter>[];
-    var paramRefs = _emitParameterReferences(node.parameters);
-
     JS.Fun fn;
     if (_externalOrNative(node)) {
-      fn = _emitNativeFunctionBody(params, paramRefs, node);
+      fn = _emitNativeFunctionBody(node);
       // TODO(vsm): Remove if / when we handle the static case above.
       if (fn == null) return null;
     } else {
-      var typeParams = _emitTypeParams(node.element).toList();
-      var returnType = emitTypeRef(node.element.returnType);
-      fn = _emitFunctionBody(
-          params, paramRefs, node.body, typeParams, returnType);
-    }
+      fn = _emitFunctionBody(node.element, node.parameters, node.body);
 
-    if (node.operatorKeyword != null &&
-        node.name.name == '[]=' &&
-        params.isNotEmpty) {
-      // []= methods need to return the value. We could also address this at
-      // call sites, but it's cleaner to instead transform the operator method.
-      var returnValue = new JS.Return(params.last);
-      var body = fn.body;
-      if (JS.Return.foundIn(fn)) {
-        // If a return is inside body, transform `(params) { body }` to
-        // `(params) { (() => { body })(); return value; }`.
-        // TODO(jmesserly): we could instead generate the return differently,
-        // and avoid the immediately invoked function.
-        body = new JS.Call(new JS.ArrowFun([], fn.body), []).toStatement();
+      if (node.operatorKeyword != null &&
+          node.name.name == '[]=' &&
+          fn.params.isNotEmpty) {
+        // []= methods need to return the value. We could also address this at
+        // call sites, but it's cleaner to instead transform the operator method.
+        fn = _alwaysReturnLastParameter(fn);
       }
-      // Rewrite the function to include the return.
-      fn = new JS.Fun(fn.params, new JS.Block([body, returnValue]),
-          typeParams: fn.typeParams,
-          returnType: fn.returnType)..sourceInformation = fn.sourceInformation;
     }
 
     return annotate(
         new JS.Method(_elementMemberName(node.element), fn,
             isGetter: node.isGetter,
             isSetter: node.isSetter,
             isStatic: node.isStatic),
         node,
         node.element);
   }
 
+  /// Transform the function so the last parameter is always returned.
+  ///
+  /// This is useful for indexed set methods, which otherwise would not have
+  /// the right return value in JS.
+  JS.Fun _alwaysReturnLastParameter(JS.Fun fn) {
+    var body = fn.body;
+    if (JS.Return.foundIn(fn)) {
+      // If a return is inside body, transform `(params) { body }` to
+      // `(params) { (() => { body })(); return value; }`.
+      // TODO(jmesserly): we could instead generate the return differently,
+      // and avoid the immediately invoked function.
+      body = new JS.Call(new JS.ArrowFun([], fn.body), []).toStatement();
+    }
+    // Rewrite the function to include the return.
+    return new JS.Fun(
+        fn.params, new JS.Block([body, new JS.Return(fn.params.last)]),
+        typeParams: fn.typeParams,
+        returnType: fn.returnType)..sourceInformation = fn.sourceInformation;
+  }
+
   @override
   JS.Statement visitFunctionDeclaration(FunctionDeclaration node) {
     assert(node.parent is CompilationUnit);
 
     if (_externalOrNative(node)) return null;
 
     if (node.isGetter || node.isSetter) {
       // Add these later so we can use getter/setter syntax.
       _properties.add(node);
       return null;
@@ skipping 108 matching lines @@
       if (lazy) {
         return js.call('dart.fn(#, () => #)', [clos, _emitFunctionRTTI(type)]);
       }
       return js.call('dart.fn(#, #)', [clos, _emitFunctionTypeParts(type)]);
     }
     throw 'Function has non function type: $type';
   }
 
   @override
   JS.Expression visitFunctionExpression(FunctionExpression node) {
-    var params = _visit(node.parameters) as List<JS.Parameter>;
-    if (params == null) params = <JS.Parameter>[];
+    var params = visitFormalParameterList(node.parameters);
 
+    var body = node.body;
     var parent = node.parent;
     var inStmt = parent.parent is FunctionDeclarationStatement;
-    var typeParams = _emitTypeParams(node.element).toList();
-    var returnType = emitTypeRef(node.element.returnType);
     if (parent is FunctionDeclaration) {
-      var paramRefs = _emitParameterReferences(node.parameters);
-      return _emitFunctionBody(
-          params, paramRefs, node.body, typeParams, returnType);
+      return _emitFunctionBody(node.element, node.parameters, body);
+    }
+
+    JS.Node jsBody;
+    if (body.isGenerator || body.isAsynchronous) {
+      jsBody = _emitGeneratorFunctionBody(node.element, node.parameters, body);
+    } else if (body is ExpressionFunctionBody) {
+      jsBody = _visit(body.expression);
+    } else {
+      jsBody = _visit(body);
+    }
+
+    var type = node.element.type;
+    var typeFormals = _emitTypeFormals(type.typeFormals);
+    var returnType = emitTypeRef(type.returnType);
+
+    JS.FunctionExpression fn;
+    if (node.parameters.parameters
+        .every((p) => p.kind != ParameterKind.NAMED)) {
+      fn = new JS.ArrowFun(params, jsBody,
+          typeParams: typeFormals, returnType: returnType);
     } else {
       // Chrome Canary does not accept default values with destructuring in
       // arrow functions yet (e.g. `({a} = {}) => 1`) but happily accepts them
       // with regular functions (e.g. `function({a} = {}) { return 1 }`).
       // Note that Firefox accepts both syntaxes just fine.
       // TODO(ochafik): Simplify this code when Chrome Canary catches up.
-      var canUseArrowFun = !node.parameters.parameters.any(_isNamedParam);
 
-      JS.Node jsBody;
-      var body = node.body;
-      if (body.isGenerator || body.isAsynchronous) {
-        var paramRefs = _emitParameterReferences(node.parameters);
-        jsBody =
-            _emitGeneratorFunctionBody(params, paramRefs, body, returnType);
-      } else if (body is ExpressionFunctionBody) {
-        jsBody = _visit(body.expression);
-      } else {
-        jsBody = _visit(body);
-      }
-      if (jsBody is JS.Expression && !canUseArrowFun) {
+      if (jsBody is JS.Expression) {
         jsBody = js.statement("{ return #; }", [jsBody]);
       }
-      var clos = canUseArrowFun
-          ? new JS.ArrowFun(params, jsBody,
-              typeParams: typeParams, returnType: returnType)
-          : new JS.Fun(params, jsBody,
-              typeParams: typeParams, returnType: returnType);
-      if (!inStmt) {
-        var type = getStaticType(node);
-        return _emitFunctionTagged(clos, type,
-            topLevel: _executesAtTopLevel(node));
-      }
-      return clos;
+      fn = new JS.Fun(params, jsBody,
+          typeParams: typeFormals, returnType: returnType);
     }
+
+    if (!inStmt) {
+      var type = getStaticType(node);
+      return _emitFunctionTagged(fn, type, topLevel: _executesAtTopLevel(node));
+    }
+    return fn;
   }
 
-  JS.Fun _emitFunctionBody(
-      List<JS.Parameter> params,
-      List<JS.Expression> paramRefs,
-      FunctionBody body,
-      List<JS.Identifier> typeParams,
-      JS.TypeRef returnType) {
+  JS.Fun _emitFunctionBody(ExecutableElement element,
+      FormalParameterList parameters, FunctionBody body) {
+    var returnType = emitTypeRef(element.returnType);
+
     // sync*, async, async*
-    if (body.isAsynchronous || body.isGenerator) {
-      // TODO(ochafik): Refine params: we don't need default values in the
-      // nested function, so we'd need to generate a custom, simpler params
-      // list here.
+    if (element.isAsynchronous || element.isGenerator) {
       return new JS.Fun(
-          params,
-          js.statement('{ return #; }', [
-            _emitGeneratorFunctionBody(params, paramRefs, body, returnType)
-          ]),
+          visitFormalParameterList(parameters, destructure: false),
+          js.statement('{ return #; }',
+              [_emitGeneratorFunctionBody(element, parameters, body)]),
           returnType: returnType);
     }
     // normal function (sync)
-    return new JS.Fun(params, _visit(body),
-        typeParams: typeParams, returnType: returnType);
+    return new JS.Fun(visitFormalParameterList(parameters), _visit(body),
+        typeParams: _emitTypeFormals(element.typeParameters),
+        returnType: returnType);
   }
 
-  JS.Expression _emitGeneratorFunctionBody(List<JS.Parameter> params,
-      List<JS.Expression> paramRefs, FunctionBody body, JS.TypeRef returnType) {
-    var kind = body.isSynchronous ? 'sync' : 'async';
-    if (body.isGenerator) kind += 'Star';
+  JS.Expression _emitGeneratorFunctionBody(ExecutableElement element,
+      FormalParameterList parameters, FunctionBody body) {
+    var kind = element.isSynchronous ? 'sync' : 'async';
+    if (element.isGenerator) kind += 'Star';
 
     // Transforms `sync*` `async` and `async*` function bodies
     // using ES6 generators.
     //
     // `sync*` wraps a generator in a Dart Iterable<T>:
     //
     // function name(<args>) {
     //   return dart.syncStar(function*(<args>) {
     //     <body>
     //   }, T, <args>).bind(this);
@@ skipping 13 matching lines @@
     // In the body of a `sync*` and `async`, `yield`/`await` are both generated
     // simply as `yield`.
     //
     // `async*` uses the `dart.asyncStar` helper, and also has an extra `stream`
     // argument to the generator, which is used for passing values to the
     // _AsyncStarStreamController implementation type.
     // `yield` is specially generated inside `async*`, see visitYieldStatement.
     // `await` is generated as `yield`.
     // runtime/_generators.js has an example of what the code is generated as.
     var savedController = _asyncStarController;
-    List jsParams;
+    List jsParams = visitFormalParameterList(parameters);
     if (kind == 'asyncStar') {
       _asyncStarController = new JS.TemporaryId('stream');
-      jsParams = [_asyncStarController]..addAll(params);
+      jsParams.insert(0, _asyncStarController);
     } else {
       _asyncStarController = null;
-      jsParams = params;
     }
-    JS.Expression gen = new JS.Fun(jsParams, _visit(body),
-        isGenerator: true, returnType: returnType);
+    // Visit the body with our async* controller set.
+    var jsBody = _visit(body);
+    _asyncStarController = savedController;
+
+    DartType returnType = _getExpectedReturnType(element);
+    JS.Expression gen = new JS.Fun(jsParams, jsBody,
+        isGenerator: true, returnType: emitTypeRef(returnType));
     if (JS.This.foundIn(gen)) {
       gen = js.call('#.bind(this)', gen);
     }
-    _asyncStarController = savedController;
 
-    var T = _emitTypeName(_getExpectedReturnType(body));
+    var T = _emitTypeName(returnType);
     return js.call('dart.#(#)', [
       kind,
-      [gen, T]..addAll(paramRefs)
+      [gen, T]..addAll(visitFormalParameterList(parameters, destructure: false))
     ]);
   }
 
   @override
   JS.Statement visitFunctionDeclarationStatement(
       FunctionDeclarationStatement node) {
     var func = node.functionDeclaration;
     if (func.isGetter || func.isSetter) {
       return js.comment('Unimplemented function get/set statement: $node');
     }
@@ skipping 484 matching lines @@
     return args;
   }
 
   @override
   JS.Property visitNamedExpression(NamedExpression node) {
     assert(node.parent is ArgumentList);
     return new JS.Property(
         _propertyName(node.name.label.name), _visit(node.expression));
   }
 
-  bool _isNamedParam(FormalParameter param) =>
-      param.kind == ParameterKind.NAMED;
+  @override
+  List<JS.Parameter> visitFormalParameterList(FormalParameterList node,
+      {bool destructure: true}) {
+    if (node == null) return [];
 
-  @override
-  List<JS.Parameter> visitFormalParameterList(FormalParameterList node) =>
-      _emitFormalParameterList(node);
+    destructure = destructure && options.destructureNamedParams;
 
-  // TODO(ochafik): Decouple Parameter from Identifier.
-  List<JS.Expression> _emitParameterReferences(FormalParameterList node) =>
-      node == null
-          ? <JS.Expression>[]
-          : _emitFormalParameterList(node, allowDestructuring: false)
-              .map((JS.Parameter p) {
-              if (p is JS.RestParameter) return new JS.Spread(p.parameter);
-              return p as JS.Identifier;
-            }).toList();
-
-  List<JS.Parameter> _emitFormalParameterList(FormalParameterList node,
-      {bool allowDestructuring: true}) {
     var result = <JS.Parameter>[];
-
     var namedVars = <JS.DestructuredVariable>[];
-    var destructure = allowDestructuring && options.destructureNamedParams;
     var hasNamedArgsConflictingWithObjectProperties = false;
     var needsOpts = false;
 
     for (FormalParameter param in node.parameters) {
       if (param.kind == ParameterKind.NAMED) {
         if (destructure) {
           if (_jsObjectProperties.contains(param.identifier.name)) {
             hasNamedArgsConflictingWithObjectProperties = true;
           }
           JS.Expression name;
@@ skipping 1430 matching lines @@
     MethodElement element = _types.objectType.element.getMethod(name);
     return (element != null && !element.isStatic);
   }
 
   bool _isObjectProperty(String name) {
     return _isObjectGetter(name) || _isObjectMethod(name);
   }
 
   // TODO(leafp): Various analyzer pieces computed similar things.
   // Share this logic somewhere?
-  DartType _getExpectedReturnType(FunctionBody body) {
-    FunctionType functionType;
-    var parent = body.parent;
-    if (parent is Declaration) {
-      functionType = (parent.element as dynamic)?.type;
-    } else {
-      assert(parent is FunctionExpression);
-      functionType = parent.staticType;
-    }
+  DartType _getExpectedReturnType(ExecutableElement element) {
+    FunctionType functionType = element.type;
     if (functionType == null) {
       return DynamicTypeImpl.instance;
     }
     var type = functionType.returnType;
 
     InterfaceType expectedType = null;
-    if (body.isAsynchronous) {
-      if (body.isGenerator) {
+    if (element.isAsynchronous) {
+      if (element.isGenerator) {
         // Stream<T> -> T
         expectedType = _types.streamType;
       } else {
         // Future<T> -> T
         // TODO(vsm): Revisit with issue #228.
         expectedType = _types.futureType;
       }
     } else {
-      if (body.isGenerator) {
+      if (element.isGenerator) {
         // Iterable<T> -> T
         expectedType = _types.iterableType;
       } else {
         // T -> T
         return type;
       }
     }
     if (type.isDynamic) {
       return type;
     } else if (type is InterfaceType && type.element == expectedType.element) {
@@ skipping 101 matching lines @@
 
 /// A special kind of element created by the compiler, signifying a temporary
 /// variable. These objects use instance equality, and should be shared
 /// everywhere in the tree where they are treated as the same variable.
 class TemporaryVariableElement extends LocalVariableElementImpl {
   TemporaryVariableElement.forNode(Identifier name) : super.forNode(name);
 
   int get hashCode => identityHashCode(this);
   bool operator ==(Object other) => identical(this, other);
 }