Allow trailing commas in param lists and argument lists

Fixes #2552 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=160980975
google · Jul 6, 2017 · 3065437 · 3065437
1 parent c97a285
commit 3065437
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 24 deletions.
diff --git a/src/com/google/javascript/jscomp/parsing/ParserRunner.java b/src/com/google/javascript/jscomp/parsing/ParserRunner.java
@@ -159,9 +159,13 @@ private static com.google.javascript.jscomp.parsing.parser.Parser.Config newPars
         break;
 
       case ECMASCRIPT6:
+        parserConfigLanguageMode = Mode.ES6;
+        break;
       case ECMASCRIPT7:
+        parserConfigLanguageMode = Mode.ES7;
+        break;
       case ECMASCRIPT8:
-        parserConfigLanguageMode = Mode.ES6_OR_GREATER;
+        parserConfigLanguageMode = Mode.ES8_OR_GREATER;
         break;
 
       default:

diff --git a/src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java b/src/com/google/javascript/jscomp/parsing/parser/FeatureSet.java
@@ -113,11 +113,12 @@ public enum Feature {
     DEFAULT_PARAMETERS("default parameter", LangVersion.ES6),
     MODULES("modules", LangVersion.ES6),
 
-    // '**' operator
+    // ES 2016 only added one new feature:
     EXPONENT_OP("exponent operator (**)", LangVersion.ES7),
 
-    // http://tc39.github.io/ecmascript-asyncawait/
+    // ES 2017 features:
     ASYNC_FUNCTIONS("async function", LangVersion.ES8),
+    TRAILING_COMMA_IN_PARAM_LIST("trailing comma in param list", LangVersion.ES8),
 
     // ES6 typed features that are not at all implemented in browsers
     ACCESSIBILITY_MODIFIER("accessibility modifier", LangVersion.TYPESCRIPT),

diff --git a/src/com/google/javascript/jscomp/parsing/parser/Parser.java b/src/com/google/javascript/jscomp/parsing/parser/Parser.java
@@ -212,7 +212,9 @@ public static class Config {
     public static enum Mode {
       ES3,
       ES5,
-      ES6_OR_GREATER,
+      ES6,
+      ES7,
+      ES8_OR_GREATER,
       TYPESCRIPT,
     }
 
@@ -223,16 +225,18 @@ public static enum Mode {
     //     this is false.
     private final boolean parseTypeSyntax;
     private final boolean atLeast6;
+    private final boolean atLeast8;
     private final boolean isStrictMode;
     private final boolean warnTrailingCommas;
 
     public Config() {
-      this(Mode.ES6_OR_GREATER, /* isStrictMode */ false);
+      this(Mode.ES8_OR_GREATER, /* isStrictMode */ false);
     }
 
     public Config(Mode mode, boolean isStrictMode) {
       parseTypeSyntax = mode == Mode.TYPESCRIPT;
       atLeast6 = !(mode == Mode.ES3 || mode == Mode.ES5);
+      atLeast8 = mode == Mode.ES8_OR_GREATER;
       this.isStrictMode = isStrictMode;
 
       // Generally, we allow everything that is valid in any mode
@@ -1347,7 +1351,10 @@ private FormalParameterListTree parseFormalParameterList(ParamContext context) {
       if (!peek(TokenType.CLOSE_PAREN)) {
         Token comma = eat(TokenType.COMMA);
         if (peek(TokenType.CLOSE_PAREN)) {
-          reportError(comma, "Invalid trailing comma in formal parameter list");
+          features = features.with(Feature.TRAILING_COMMA_IN_PARAM_LIST);
+          if (!config.atLeast8) {
+            reportError(comma, "Invalid trailing comma in formal parameter list");
+          }
         }
       }
     }
@@ -3480,9 +3487,12 @@ private ArgumentListTree parseArguments() {
       arguments.add(parseAssignmentOrSpread());
 
       if (!peek(TokenType.CLOSE_PAREN)) {
-        eat(TokenType.COMMA);
+        Token comma = eat(TokenType.COMMA);
         if (peek(TokenType.CLOSE_PAREN)) {
-          reportError("Invalid trailing comma in arguments list");
+          features = features.with(Feature.TRAILING_COMMA_IN_PARAM_LIST);
+          if (!config.atLeast8) {
+            reportError(comma, "Invalid trailing comma in arguments list");
+          }
         }
       }
     }

diff --git a/test/com/google/javascript/jscomp/parsing/ParserTest.java b/test/com/google/javascript/jscomp/parsing/ParserTest.java
@@ -122,13 +122,51 @@ public void testExponentAssignmentOperator() {
   public void testFunction() {
     parse("var f = function(x,y,z) { return 0; }");
     parse("function f(x,y,z) { return 0; }");
+  }
+
+  public void testFunctionTrailingComma() {
+    mode = LanguageMode.ECMASCRIPT8;
+
+    expectFeatures(Feature.TRAILING_COMMA_IN_PARAM_LIST);
+    parse("var f = function(x,y,z,) {}");
+    parse("function f(x,y,z,) {}");
+  }
+
+  public void testFunctionTrailingCommaPreES8() {
+    mode = LanguageMode.ECMASCRIPT7;
 
-    parseError("var f = function(x,y,z,) {}",
+    parseError(
+        "var f = function(x,y,z,) {}",
         "Invalid trailing comma in formal parameter list");
-    parseError("function f(x,y,z,) {}",
+    parseError(
+        "function f(x,y,z,) {}",
         "Invalid trailing comma in formal parameter list");
   }
 
+  public void testFunctionExtraTrailingComma() {
+    parseError("var f = function(x,y,z,,) {}", "')' expected");
+    parseError("function f(x,y,z,,) {}", "')' expected");
+  }
+
+  public void testCallTrailingComma() {
+    mode = LanguageMode.ECMASCRIPT8;
+
+    expectFeatures(Feature.TRAILING_COMMA_IN_PARAM_LIST);
+    parse("f(x,y,z,);");
+  }
+
+  public void testCallTrailingCommaPreES8() {
+    mode = LanguageMode.ECMASCRIPT7;
+
+    parseError(
+        "f(x,y,z,);",
+        "Invalid trailing comma in arguments list");
+  }
+
+  public void testCallExtraTrailingComma() {
+    parseError("f(x,y,z,,);", "')' expected");
+  }
+
   public void testWhile() {
     parse("while(1) { break; }");
   }
@@ -2662,20 +2700,6 @@ public void testGetPropFunctionName() {
         "'(' expected");
   }
 
-  public void testGetPropFunctionNameIdeMode() {
-    // In IDE mode, we try to fix up the tree, but sometimes
-    // this leads to even more errors.
-    isIdeMode = true;
-    parseError("function a.b() {}",
-        "'(' expected",
-        "',' expected",
-        "Invalid trailing comma in formal parameter list");
-    parseError("var x = function a.b() {}",
-        "'(' expected",
-        "',' expected",
-        "Invalid trailing comma in formal parameter list");
-  }
-
   public void testIdeModePartialTree() {
     Node partialTree = parseError("function Foo() {} f.",
         "'identifier' expected");