[parser] Track trailing commas in extras instead of state

babel · Sep 24, 2019 · d5a7b29 · d5a7b29
1 parent eaa1474
commit d5a7b29
Show file tree

Hide file tree

Showing 13 changed files with 85 additions and 42 deletions.
diff --git a/packages/babel-parser/src/parser/expression.js b/packages/babel-parser/src/parser/expression.js
@@ -168,9 +168,6 @@ export default class ExpressionParser extends LValParser {
       }
     }
 
-    const oldCommaAfterSpreadAt = this.state.commaAfterSpreadAt;
-    this.state.commaAfterSpreadAt = -1;
-
     let failOnShorthandAssign;
     if (refShorthandDefaultPos) {
       failOnShorthandAssign = false;
@@ -230,18 +227,13 @@ export default class ExpressionParser extends LValParser {
         );
       }
 
-      if (patternErrorMsg) this.checkCommaAfterRestFromSpread();
-      this.state.commaAfterSpreadAt = oldCommaAfterSpreadAt;
-
       this.next();
       node.right = this.parseMaybeAssign(noIn);
       return this.finishNode(node, "AssignmentExpression");
     } else if (failOnShorthandAssign && refShorthandDefaultPos.start) {
       this.unexpected(refShorthandDefaultPos.start);
     }
 
-    this.state.commaAfterSpreadAt = oldCommaAfterSpreadAt;
-
     return left;
   }
 
@@ -690,14 +682,12 @@ export default class ExpressionParser extends LValParser {
       let node = this.startNodeAt(startPos, startLoc);
       node.callee = base;
 
-      const oldCommaAfterSpreadAt = this.state.commaAfterSpreadAt;
-      this.state.commaAfterSpreadAt = -1;
-
       node.arguments = this.parseCallExpressionArguments(
         tt.parenR,
         maybeAsyncArrow,
         base.type === "Import",
         base.type !== "Super",
+        node,
       );
       if (!state.optionalChainMember) {
         this.finishCallExpression(node);
@@ -708,8 +698,6 @@ export default class ExpressionParser extends LValParser {
       if (maybeAsyncArrow && this.shouldParseAsyncArrow()) {
         state.stop = true;
 
-        this.checkCommaAfterRestFromSpread();
-
         node = this.parseAsyncArrowFromCallExpression(
           this.startNodeAt(startPos, startLoc),
           node,
@@ -727,7 +715,6 @@ export default class ExpressionParser extends LValParser {
       }
 
       this.state.maybeInArrowParameters = oldMaybeInArrowParameters;
-      this.state.commaAfterSpreadAt = oldCommaAfterSpreadAt;
 
       return node;
     } else if (this.match(tt.backQuote)) {
@@ -809,6 +796,7 @@ export default class ExpressionParser extends LValParser {
     possibleAsyncArrow: boolean,
     dynamicImport?: boolean,
     allowPlaceholder?: boolean,
+    nodeForExtra?: ?N.Node,
   ): $ReadOnlyArray<?N.Expression> {
     const elts = [];
     let innerParenStart;
@@ -828,6 +816,13 @@ export default class ExpressionParser extends LValParser {
               "Trailing comma is disallowed inside import(...) arguments",
             );
           }
+          if (nodeForExtra) {
+            this.addExtra(
+              nodeForExtra,
+              "trailingComma",
+              this.state.lastTokStart,
+            );
+          }
           break;
         }
       }
@@ -867,7 +862,12 @@ export default class ExpressionParser extends LValParser {
     call: N.CallExpression,
   ): N.ArrowFunctionExpression {
     this.expect(tt.arrow);
-    this.parseArrowExpression(node, call.arguments, true);
+    this.parseArrowExpression(
+      node,
+      call.arguments,
+      true,
+      call.extra?.trailingComma,
+    );
     return node;
   }
 
@@ -1030,6 +1030,7 @@ export default class ExpressionParser extends LValParser {
           tt.bracketR,
           true,
           refShorthandDefaultPos,
+          node,
         );
         if (!this.state.maybeInArrowParameters) {
           // This could be an array pattern:
@@ -1488,7 +1489,11 @@ export default class ExpressionParser extends LValParser {
         first = false;
       } else {
         this.expect(tt.comma);
-        if (this.eat(tt.braceR)) break;
+        if (this.match(tt.braceR)) {
+          this.addExtra(node, "trailingComma", this.state.lastTokStart);
+          this.next();
+          break;
+        }
       }
 
       const prop = this.parseObjectMember(isPattern, refShorthandDefaultPos);
@@ -1840,6 +1845,7 @@ export default class ExpressionParser extends LValParser {
     node: N.ArrowFunctionExpression,
     params: ?(N.Expression[]),
     isAsync: boolean,
+    trailingCommaPos: ?number,
   ): N.ArrowFunctionExpression {
     this.scope.enter(functionFlags(isAsync, false) | SCOPE_ARROW);
     this.initFunction(node, isAsync);
@@ -1851,7 +1857,7 @@ export default class ExpressionParser extends LValParser {
     this.state.yieldPos = 0;
     this.state.awaitPos = 0;
 
-    if (params) this.setArrowFunctionParameters(node, params);
+    if (params) this.setArrowFunctionParameters(node, params, trailingCommaPos);
     this.parseFunctionBody(node, true);
 
     this.scope.exit();
@@ -1865,11 +1871,13 @@ export default class ExpressionParser extends LValParser {
   setArrowFunctionParameters(
     node: N.ArrowFunctionExpression,
     params: N.Expression[],
+    trailingCommaPos: ?number,
   ): void {
     node.params = this.toAssignableList(
       params,
       true,
       "arrow function parameters",
+      trailingCommaPos,
     );
   }
 
@@ -1996,6 +2004,7 @@ export default class ExpressionParser extends LValParser {
     close: TokenType,
     allowEmpty?: boolean,
     refShorthandDefaultPos?: ?Pos,
+    nodeForExtra?: ?N.Node,
   ): $ReadOnlyArray<?N.Expression> {
     const elts = [];
     let first = true;
@@ -2005,7 +2014,17 @@ export default class ExpressionParser extends LValParser {
         first = false;
       } else {
         this.expect(tt.comma);
-        if (this.eat(close)) break;
+        if (this.match(close)) {
+          if (nodeForExtra) {
+            this.addExtra(
+              nodeForExtra,
+              "trailingComma",
+              this.state.lastTokStart,
+            );
+          }
+          this.next();
+          break;
+        }
       }
 
       elts.push(this.parseExprListItem(allowEmpty, refShorthandDefaultPos));

diff --git a/packages/babel-parser/src/parser/lval.js b/packages/babel-parser/src/parser/lval.js
@@ -60,6 +60,14 @@ export default class LValParser extends NodeUtils {
             const prop = node.properties[i];
             const isLast = i === last;
             this.toAssignableObjectExpressionProp(prop, isBinding, isLast);
+
+            if (
+              isLast &&
+              prop.type === "RestElement" &&
+              node.extra?.trailingComma
+            ) {
+              this.raiseRestNotLast(node.extra.trailingComma);
+            }
           }
           break;
 
@@ -78,7 +86,12 @@ export default class LValParser extends NodeUtils {
 
         case "ArrayExpression":
           node.type = "ArrayPattern";
-          this.toAssignableList(node.elements, isBinding, contextDescription);
+          this.toAssignableList(
+            node.elements,
+            isBinding,
+            contextDescription,
+            node.extra?.trailingComma,
+          );
           break;
 
         case "AssignmentExpression":
@@ -142,6 +155,7 @@ export default class LValParser extends NodeUtils {
     exprList: Expression[],
     isBinding: ?boolean,
     contextDescription: string,
+    trailingCommaPos?: ?number,
   ): $ReadOnlyArray<Pattern> {
     let end = exprList.length;
     if (end) {
@@ -160,6 +174,11 @@ export default class LValParser extends NodeUtils {
         ) {
           this.unexpected(arg.start);
         }
+
+        if (trailingCommaPos) {
+          this.raiseRestNotLast(trailingCommaPos);
+        }
+
         --end;
       }
     }
@@ -213,11 +232,6 @@ export default class LValParser extends NodeUtils {
       undefined,
       refNeedsArrowPos,
     );
-
-    if (this.state.commaAfterSpreadAt === -1 && this.match(tt.comma)) {
-      this.state.commaAfterSpreadAt = this.state.start;
-    }
-
     return this.finishNode(node, "SpreadElement");
   }
 
@@ -461,12 +475,6 @@ export default class LValParser extends NodeUtils {
     }
   }
 
-  checkCommaAfterRestFromSpread(): void {
-    if (this.state.commaAfterSpreadAt > -1) {
-      this.raiseRestNotLast(this.state.commaAfterSpreadAt);
-    }
-  }
-
   raiseRestNotLast(pos: number) {
     this.raise(pos, `Rest element must be last element`);
   }

diff --git a/packages/babel-parser/src/plugins/flow.js b/packages/babel-parser/src/plugins/flow.js
@@ -1789,6 +1789,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
               ((node.params: any): N.Expression[]),
               true,
               "arrow function parameters",
+              node.extra?.trailingComma,
             );
             // Enter scope, as checkParams defines bindings
             this.scope.enter(functionFlags(false, false) | SCOPE_ARROW);
@@ -1811,6 +1812,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
             ((node.params: any): N.Expression[]),
             true,
             "arrow function parameters",
+            node.extra?.trailingComma,
           );
         }
         return [arrows, []];
@@ -1822,6 +1824,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
             ((node.params: any): N.Expression[]),
             true,
             "arrow function parameters",
+            node.extra?.trailingComma,
           );
           return true;
         } catch (err) {
@@ -1996,14 +1999,20 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       exprList: N.Expression[],
       isBinding: ?boolean,
       contextDescription: string,
+      trailingCommaPos?: ?number,
     ): $ReadOnlyArray<N.Pattern> {
       for (let i = 0; i < exprList.length; i++) {
         const expr = exprList[i];
         if (expr && expr.type === "TypeCastExpression") {
           exprList[i] = this.typeCastToParameter(expr);
         }
       }
-      return super.toAssignableList(exprList, isBinding, contextDescription);
+      return super.toAssignableList(
+        exprList,
+        isBinding,
+        contextDescription,
+        trailingCommaPos,
+      );
     }
 
     // this is a list of nodes, from something like a call expression, we need to filter the

diff --git a/packages/babel-parser/src/plugins/typescript/index.js b/packages/babel-parser/src/plugins/typescript/index.js
@@ -2360,11 +2360,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       }
     }
 
-    toAssignableList(
-      exprList: N.Expression[],
-      isBinding: ?boolean,
-      contextDescription: string,
-    ): $ReadOnlyArray<N.Pattern> {
+    toAssignableList(exprList: N.Expression[]): $ReadOnlyArray<N.Pattern> {
       for (let i = 0; i < exprList.length; i++) {
         const expr = exprList[i];
         if (!expr) continue;
@@ -2381,7 +2377,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
             break;
         }
       }
-      return super.toAssignableList(exprList, isBinding, contextDescription);
+      return super.toAssignableList(...arguments);
     }
 
     typeCastToParameter(node: N.TsTypeCastExpression): N.Node {

diff --git a/packages/babel-parser/src/tokenizer/state.js b/packages/babel-parser/src/tokenizer/state.js
@@ -55,11 +55,6 @@ export default class State {
   //          ^
   noArrowParamsConversionAt: number[] = [];
 
-  // A comma after "...a" is only allowed in spread, but not in rest.
-  // Since we parse destructuring patterns as array/object literals
-  // and then convert them, we need to track it.
-  commaAfterSpreadAt: number = -1;
-
   // Flags to track
   inParameters: boolean = false;
   maybeInArrowParameters: boolean = false;

diff --git a/...es/babel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-for-in/input.js b/...es/babel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-for-in/input.js
@@ -0,0 +1 @@
+for ([...a,] in []);
diff --git a/...abel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-for-in/options.json b/...abel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-for-in/options.json
@@ -0,0 +1,3 @@
+{
+  "throws": "Rest element must be last element (1:10)"
+}
diff --git a/...es/babel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-nested/input.js b/...es/babel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-nested/input.js
@@ -0,0 +1 @@
+[[...a,]] = [];
diff --git a/...abel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-nested/options.json b/...abel-parser/test/fixtures/es2015/array-rest-spread/comma-after-spread-nested/options.json
@@ -0,0 +1,3 @@
+{
+  "throws": "Rest element must be last element (1:6)"
+}
diff --git a/...s/babel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-for-in/input.js b/...s/babel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-for-in/input.js
@@ -0,0 +1 @@
+for ({...a,} in []);
diff --git a/...bel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-for-in/options.json b/...bel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-for-in/options.json
@@ -0,0 +1,3 @@
+{
+  "throws": "Rest element must be last element (1:10)"
+}
diff --git a/...s/babel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-nested/input.js b/...s/babel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-nested/input.js
@@ -0,0 +1 @@
+[{...a,}] = [];
diff --git a/...bel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-nested/options.json b/...bel-parser/test/fixtures/es2018/object-rest-spread/comma-after-spread-nested/options.json
@@ -0,0 +1,3 @@
+{
+  "throws": "Rest element must be last element (1:6)"
+}