Use destructuring assignments for named parameters (#180)

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.
 
 library dev_compiler.src.codegen.js_codegen;
 
 import 'dart:collection' show HashSet, HashMap, SplayTreeSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 17 matching lines @@
 import '../info.dart';
 import '../options.dart' show CodegenOptions;
 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';
+import 'js_names.dart';
 import 'js_printer.dart' show writeJsLibrary;
 import 'side_effect_analysis.dart';
 import 'package:collection/equality.dart';
 
 // Various dynamic helpers we call.
 // If renaming these, make sure to check other places like the
 // _runtime.js file and comments.
 // TODO(jmesserly): ideally we'd have a "dynamic call" dart library we can
 // import and generate calls to, rather than dart_runtime.js
 const DPUT = 'dput';
@@ skipping 887 matching lines @@
 
     var name = _constructorName(node.element);
 
     // 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 = _visit(node.parameters);
+      var params =
+          _emitFormalParameterList(node.parameters, allowDestructuring: false);
+
       var fun = js.call('function(#) { return $newKeyword #(#); }',
-          [params, _visit(redirect), params,]) as JS.Fun;
+          [params, _visit(redirect), params]) as JS.Fun;
       return annotate(
           new JS.Method(name, fun, isStatic: true)..sourceInformation = node,
           node.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);
@@ skipping 240 matching lines @@
     if (node is ConstructorDeclaration != constructor) return null;
 
     var parameters = _parametersOf(node);
     if (parameters == null) return null;
 
     var body = <JS.Statement>[];
     for (var param in parameters.parameters) {
       var jsParam = _visit(param.identifier);
 
       if (param.kind == ParameterKind.NAMED) {
-        // Parameters will be passed using their real names, not the (possibly
-        // renamed) local variable.
-        var paramName = js.string(param.identifier.name, "'");
+        if (!_isDestructurableNamedParam(param)) {
+          // 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,
-          paramName,
-          _namedArgTemp,
-          _namedArgTemp,
-          paramName,
-          _defaultParamValue(param),
-        ]));
+          // TODO(ochafik): Fix `'prop' in obj` to please Closure's renaming.
+          body.add(js.statement('let # = # && # in # ? #.# : #;', [
+            jsParam,
+            _namedArgTemp,
+            paramName,
+            _namedArgTemp,
+            _namedArgTemp,
+            paramName,
+            _defaultParamValue(param),
+          ]));
+        }
       } else if (param.kind == ParameterKind.POSITIONAL) {
         body.add(js.statement('if (# === void 0) # = #;',
             [jsParam, jsParam, _defaultParamValue(param)]));
       }
 
       // TODO(jmesserly): various problems here, see:
       // https://github.com/dart-lang/dev_compiler/issues/161
       var paramType = param.element.type;
       if (!constructor && _hasTypeParameter(paramType)) {
         body.add(js.statement(
@@ skipping 182 matching lines @@
   @override
   JS.Expression visitFunctionExpression(FunctionExpression node) {
     var params = _visit(node.parameters) as List<JS.Parameter>;
     if (params == null) params = <JS.Parameter>[];
 
     var parent = node.parent;
     var inStmt = parent.parent is FunctionDeclarationStatement;
     if (parent is FunctionDeclaration) {
       return _emitFunctionBody(params, node.body);
     } else {
-      String code;
+      // 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);
+
+      String code = canUseArrowFun ? '(#) => #' : 'function(#) { return # }';
       JS.Node jsBody;
       var body = node.body;
       if (body.isGenerator || body.isAsynchronous) {
-        code = '(#) => #';
         jsBody = _emitGeneratorFunctionBody(params, body);
       } else if (body is ExpressionFunctionBody) {
-        code = '(#) => #';
         jsBody = _visit(body.expression);
       } else {
-        code = '(#) => { #; }';
+        code = canUseArrowFun ? '(#) => { #; }' : 'function(#) { #; }';
         jsBody = _visit(body);
       }
       var clos = js.call(code, [params, jsBody]);
       if (!inStmt) {
         var type = getStaticType(node);
         return _emitFunctionTagged(clos, type,
             topLevel: _executesAtTopLevel(node));
       }
       return clos;
     }
@@ skipping 520 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;
+
+  /// We cannot destructure named params that clash with JS reserved names:
+  /// see discussion in https://github.com/dart-lang/dev_compiler/issues/392.
+  bool _isDestructurableNamedParam(FormalParameter param) =>
+      _isNamedParam(param) && !invalidVariableName(param.identifier.name);
+
   @override
-  List<JS.Parameter> visitFormalParameterList(FormalParameterList node) {
+  List<JS.Parameter> visitFormalParameterList(FormalParameterList node) =>
+      _emitFormalParameterList(node);
+
+  List<JS.Parameter> _emitFormalParameterList(FormalParameterList node,
+      {bool allowDestructuring: true}) {
     var result = <JS.Parameter>[];
+
+    var namedVars = <JS.DestructuredVariable>[];
+    var destructure = allowDestructuring &&
+        node.parameters.where(_isNamedParam).every(_isDestructurableNamedParam);
+    var hasNamedArgsConflictingWithObjectProperties = false;
+    var needsOpts = false;
+
     for (FormalParameter param in node.parameters) {
       if (param.kind == ParameterKind.NAMED) {
-        result.add(_namedArgTemp);
-        break;
+        if (destructure) {
+          if (_jsObjectProperties.contains(param.identifier.name)) {
+            hasNamedArgsConflictingWithObjectProperties = true;
+          }
+          namedVars.add(new JS.DestructuredVariable(
+              name: _visit(param.identifier),
+              defaultValue: _defaultParamValue(param)));
+        } else {
+          needsOpts = true;
+        }
+      } else {
+        result.add(_visit(param));
       }
-      result.add(_visit(param));
+    }
+
+    if (needsOpts) {
+      result.add(_namedArgTemp);
+    } else if (namedVars.isNotEmpty) {
+      // Note: `var {valueOf} = {}` extracts `Object.prototype.valueOf`, so
+      // in case there are conflicting names we create an object without
+      // any prototype.
+      var defaultOpts = hasNamedArgsConflictingWithObjectProperties
+          ? js.call('Object.create(null)')
+          : js.call('{}');
+      result.add(new JS.DestructuredVariable(
+          structure: new JS.ObjectBindingPattern(namedVars),
+          defaultValue: defaultOpts));
     }
     return result;
   }
 
+  /// See ES6 spec (and `Object.getOwnPropertyNames(Object.prototype)`):
+  /// http://www.ecma-international.org/ecma-262/6.0/#sec-properties-of-the-object-prototype-object
+  /// http://www.ecma-international.org/ecma-262/6.0/#sec-additional-properties-of-the-object.prototype-object
+  static final Set<String> _jsObjectProperties = new Set<String>()
+    ..addAll([
+      "constructor",
+      "toString",
+      "toLocaleString",
+      "valueOf",
+      "hasOwnProperty",
+      "isPrototypeOf",
+      "propertyIsEnumerable",
+      "__defineGetter__",
+      "__lookupGetter__",
+      "__defineSetter__",
+      "__lookupSetter__",
+      "__proto__"
+    ]);
+
   @override
   JS.Statement visitExpressionStatement(ExpressionStatement node) =>
       _visit(node.expression).toStatement();
 
   @override
   JS.EmptyStatement visitEmptyStatement(EmptyStatement node) =>
       new JS.EmptyStatement();
 
   @override
   JS.Statement visitAssertStatement(AssertStatement node) =>
@@ skipping 1418 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);
 }