Use destructuring assignments for named parameters (#180)

lib/src/js/builder.dart

 // Copyright (c) 2013, 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.
 
 // Utilities for building JS ASTs at runtime.  Contains a builder class
 // and a parser that parses part of the language.
 
 part of js_ast;
 
 
@@ skipping 793 matching lines @@
       return null;
     }
   }
 
   InterpolatedExpression parseInterpolatedExpression() {
     var expression = new InterpolatedExpression(parseHash());
     interpolatedValues.add(expression);
     return expression;
   }
 
+  InterpolatedIdentifier parseInterpolatedIdentifier() {
+    var id = new InterpolatedIdentifier(parseHash());
+    interpolatedValues.add(id);
+    return id;
+  }
+
+  Identifier parseIdentifier() {
+    if (acceptCategory(HASH)) {
+      return parseInterpolatedIdentifier();
+    } else {
+      var id = new Identifier(lastToken);
+      expectCategory(ALPHA);
+      return id;
+    }
+  }
+
   /**
    * CoverParenthesizedExpressionAndArrowParameterList[Yield] :
    *     ( Expression )
    *     ( )
    *     ( ... BindingIdentifier )
    *     ( Expression , ... BindingIdentifier )
    */
   Expression parseExpressionOrArrowFunction() {
     if (acceptCategory(RPAREN)) {
       expectCategory(ARROW);
@@ skipping 305 matching lines @@
   Expression parseExpression() {
     Expression expression = parseAssignment();
     while (acceptCategory(COMMA)) {
       Expression right = parseAssignment();
       expression = new Binary(',', expression, right);
     }
     return expression;
   }
 
   /** Parse a variable declaration list, with `var` or `let` [keyword] */
-  VariableDeclarationList parseVariableDeclarationList(String keyword) {
-    // Supports one form for interpolated variable declaration:
-    //    let # = ...
-    if (acceptCategory(HASH)) {
-      var name = new InterpolatedIdentifier(parseHash());
-      interpolatedValues.add(name);
+  VariableDeclarationList parseVariableDeclarationList(String keyword) =>
+      finishVariableDeclarationList(keyword);

[Jennifer Messerly, 2015/12/01 02:10:27]

why introduce this redirect? If they aren't different, the two methods should be
merged. I'm fine with either name.

[ochafik, 2015/12/02 20:05:46]

Done.

 
-      Expression initializer = acceptString("=") ? parseAssignment() : null;
-      return new VariableDeclarationList(keyword,
-          [new VariableInitialization(name, initializer)]);
-    }
-
-    String firstVariable = lastToken;
-    expectCategory(ALPHA);
-    return finishVariableDeclarationList(keyword, firstVariable);
-  }
-
-  VariableDeclarationList finishVariableDeclarationList(
-      String keyword, String firstVariable) {
+  VariableDeclarationList finishVariableDeclarationList(String keyword) {
     var initialization = [];
 
-    void declare(String variable) {
+    do {
+      var declarator;
+      switch (lastCategory) {

[Jennifer Messerly, 2015/12/01 02:10:27]

this pattern occurs in a couple of places, pull it out into a method?

[ochafik, 2015/12/02 20:05:46]

+1, meet parseVariableBinding()

+        case ALPHA:
+        case HASH:
+          declarator = parseIdentifier();
+          break;
+        case LBRACE:
+        case LSQUARE:
+          declarator = parseDestructuring();
+          break;
+        default:
+          throw new StateError('Unexpected token $lastToken: ${categoryToString(lastCategory)}');

[Jennifer Messerly, 2015/12/01 02:10:27]

long line. FYI, to prevent large churn here we exclude js_ast from dartfmt, so
it must be formatted manually for now.

[ochafik, 2015/12/02 20:05:46]

Done.

+      }
       Expression initializer = null;
       if (acceptString("=")) {
         initializer = parseAssignment();
       }
-      var declaration = new Identifier(variable);
-      initialization.add(new VariableInitialization(declaration, initializer));
+      initialization.add(new VariableInitialization(declarator, initializer));
+    } while (acceptCategory(COMMA));
+
+    return new VariableDeclarationList(keyword, initialization);
+  }
+
+  Destructuring parseDestructuring() {

[Jennifer Messerly, 2015/12/01 02:10:27]

It'd be worth adding a link to where this is defined in the ES6 spec:
http://www.ecma-international.org/ecma-262/6.0/#sec-destructuring-binding-pat...
http://www.ecma-international.org/ecma-262/6.0/#sec-destructuring-assignment

Also good to comment that we only support restructuring in variable
initialization at this time, in particular it's not supported in
LeftHandSideExpression (although I have another comment on this below)

Probably worth adding a TODO that this is not a complete parsing implementation
for destructuring.

[ochafik, 2015/12/02 20:05:46]

Done.

+    if (acceptCategory(LBRACE)) {
+      return parseObjectDestructuring();
+    } else {
+      expectCategory(LSQUARE);
+      return parseArrayDestructuring();
     }
+  }
 
-    declare(firstVariable);
-    while (acceptCategory(COMMA)) {
-      String variable = lastToken;
-      expectCategory(ALPHA);
-      declare(variable);
-    }
-    return new VariableDeclarationList(keyword, initialization);
+  ArrayDestructuring parseArrayDestructuring() {
+    var variables = <DestructuredVariable>[];
+    do {
+      var name;
+      var structure;
+      var defaultValue;
+
+      switch (lastCategory) {
+        case ALPHA:
+        case HASH:
+          name = parseIdentifier();
+          break;
+        case LBRACE:
+        case LSQUARE:
+          structure = parseDestructuring();
+          break;
+        default:
+          throw new StateError('Unexpected token $lastToken: ${categoryToString(lastCategory)}');
+      }
+      if (acceptString("=")) {
+        defaultValue = parseExpression();
+      }
+      variables.add(new DestructuredVariable(
+          name: name, structure: structure, defaultValue: defaultValue));
+    } while (acceptCategory(COMMA));
+
+    expectCategory(RSQUARE);
+    return new ArrayDestructuring(variables);
+  }
+
+  ObjectDestructuring parseObjectDestructuring() {
+    var variables = <DestructuredVariable>[];
+    do {
+      var name = parseIdentifier();
+      var structure;
+      var defaultValue;
+
+      if (acceptCategory(COLON)) {

[Jennifer Messerly, 2015/12/01 02:10:27]

Hmmm, normally we'd do parseAssignment, like the old variable declaration code
did. parseAssignment will parse the left-hand-side, which will include any
nested destructurings.

The parser code here can be a little finicky, but in this case I think
implementing destructuring support in parseLeftHandSide might actually simplify
some of the logic, by allowing more code reuse, and it would also support more
use cases. WDYT?

[ochafik, 2015/12/02 20:05:46]

Hehe, tried that, but... the grammar is a bit sensitive (`{x: []}` can be parsed
as a object literal *or* a ObjectBindingPattern depending on its location, e.g.
`({x: []} = y)` vs `let y = {x: []}`).

Keeping things unchanged for now, and can actually even remove the parsing if
you prefer, since I'm not using it anymore from js_codegen (I'm building the
ObjectBindingPattern manually).

+        structure = parseDestructuring();
+      } else if (acceptString("=")) {
+        defaultValue = parseExpression();
+      }
+      variables.add(new DestructuredVariable(
+          name: name, structure: structure, defaultValue: defaultValue));
+    } while (acceptCategory(COMMA));
+
+    expectCategory(RBRACE);
+    return new ObjectDestructuring(variables);
   }
 
   Expression parseVarDeclarationOrExpression() {
     var keyword = acceptVarOrLet();
     if (keyword != null) {
       return parseVariableDeclarationList(keyword);
     } else {
       return parseExpression();
     }
   }
@@ skipping 201 matching lines @@
             body);
       } else if (acceptString('of')) {
         Expression iterableExpression = parseAssignment();
         expectCategory(RPAREN);
         Statement body = parseStatement();
         return new ForOf(
             _createVariableDeclarationList(keyword, identifier),
             iterableExpression,
             body);
       }
-      var declarations = finishVariableDeclarationList(keyword, identifier);
+      var declarations = finishVariableDeclarationList(keyword);
       expectCategory(SEMICOLON);
       return finishFor(declarations);
     }
 
     Expression init = parseExpression();
     expectCategory(SEMICOLON);
     return finishFor(init);
   }
 
   static VariableDeclarationList _createVariableDeclarationList(
@@ skipping 83 matching lines @@
     expectCategory(LPAREN);
     String identifier = lastToken;
     expectCategory(ALPHA);
     expectCategory(RPAREN);
     expectCategory(LBRACE);
     Block body = parseBlock();
     return new Catch(new Identifier(identifier), body);
   }
 
   ClassExpression parseClass() {
-    Identifier name;
-    if (acceptCategory(HASH)) {
-      var interpolatedName = new InterpolatedIdentifier(parseHash());
-      interpolatedValues.add(interpolatedName);
-      name = interpolatedName;
-    } else {
-      name = new Identifier(lastToken);
-      expectCategory(ALPHA);
-    }
+    Identifier name = parseIdentifier();
     Expression heritage = null;
     if (acceptString('extends')) {
       heritage = parseLeftHandSide();
     }
     expectCategory(LBRACE);
     var methods = new List<Method>();
     while (lastCategory != RBRACE) {
       methods.add(parseMethodOrProperty(onlyMethods: true));
     }
     expectCategory(RBRACE);
@@ skipping 63 matching lines @@
       expectCategory(RSQUARE);
       return expr;
     } else if (acceptCategory(HASH)) {
       return parseInterpolatedExpression();
     } else {
       error('Expected property name');
       return null;
     }
   }
 }