[[FIX]] allow trailing comma in ArrayBindingPattern

Also, disallow commas after rest elements in ArrayBindingPattern. Closes #2222
jshint · Feb 28, 2015 · 3477933 · 3477933
1 parent 162dee6
commit 3477933
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 9 deletions.
diff --git a/src/jshint.js b/src/jshint.js
@@ -1457,17 +1457,21 @@ var JSHINT = (function() {
   // fnparam means that this identifier is being defined as a function
   // argument
   // prop means that this identifier is that of an object property
-  function identifier(fnparam, prop) {
+  function identifier(fnparam, prop, norest) {
     var i = optionalidentifier(fnparam, prop, false);
     if (i) {
       return i;
     }
 
     // parameter destructuring with rest operator
     if (state.tokens.next.value === "...") {
-      if (!state.option.esnext) {
+      if (norest === true) {
+        warning("E024", state.tokens.next, "...");
+      } else if (!state.option.esnext) {
         warning("W119", state.tokens.next, "spread/rest operator");
       }
+      advance("...");
+      return identifier(fnparam, prop, true);
     } else {
       error("E030", state.tokens.next, state.tokens.next.value);
 
@@ -3313,22 +3317,33 @@ var JSHINT = (function() {
         nextInnerDE();
         advance(")");
       } else {
+        var is_rest = checkPunctuators(state.tokens.next, ["..."]);
         ident = identifier();
         if (ident)
           identifiers.push({ id: ident, token: state.tokens.curr });
+        return is_rest;
       }
+      return false;
     };
     if (checkPunctuators(state.tokens.next, ["["])) {
       advance("[");
-      nextInnerDE();
+      var element_after_rest = false;
+      if (nextInnerDE() && checkPunctuators(state.tokens.next, [","]) &&
+          !element_after_rest) {
+        warning("W130", state.tokens.next);
+        element_after_rest = true;
+      }
       while (!checkPunctuators(state.tokens.next, ["]"])) {
         advance(",");
         if (checkPunctuators(state.tokens.next, ["]"])) {
-          // Trailing commas are not allowed in ArrayBindingPattern
-          warning("W130", state.tokens.next);
+          // Trailing comma
           break;
         }
-        nextInnerDE();
+        if (nextInnerDE() && checkPunctuators(state.tokens.next, [","]) &&
+            !element_after_rest) {
+          warning("W130", state.tokens.next);
+          element_after_rest = true;
+        }
       }
       advance("]");
     } else if (checkPunctuators(state.tokens.next, ["{"])) {

diff --git a/src/messages.js b/src/messages.js
@@ -205,7 +205,8 @@ var warnings = {
   W128: "Empty array elements require elision=true.",
   W129: "'{a}' is defined in a future version of JavaScript. Use a " +
     "different variable name to avoid migration issues.",
-  W130: "Trailing ',' is not valid in array destructuring assignment."
+  W130: "Invalid element after rest element.",
+  W131: "Unexpected rest operator."
 };
 
 var info = {

diff --git a/tests/unit/parser.js b/tests/unit/parser.js
@@ -5549,7 +5549,6 @@ exports.trailingCommaInArrayBindingPattern = function (test) {
   ];
 
   TestRun(test)
-    .addError(2, "Trailing ',' is not valid in array destructuring assignment.")
     .test(code, { esnext: true });
 
   test.done();
@@ -5563,7 +5562,55 @@ exports.trailingCommaInArrayBindingPatternParameters = function (test) {
   ];
 
   TestRun(test)
-    .addError(1, "Trailing ',' is not valid in array destructuring assignment.")
+    .test(code, { esnext: true });
+
+  test.done();
+};
+
+
+exports.commaAfterRestElementInArrayBindingPattern = function (test) {
+  var code = [
+    'function fn(O) {',
+    '  var [a, b, ...c,] = O;',
+    '  var [...d,] = O;',
+    '}',
+    'fn([1, 2, 3]);'
+  ];
+
+  TestRun(test)
+    .addError(2, "Invalid element after rest element.")
+    .addError(3, "Invalid element after rest element.")
+    .test(code, { esnext: true });
+
+  test.done();
+};
+
+
+exports.commaAfterRestElementInArrayBindingPatternParameters = function (test) {
+  var code = [
+    'function fn([a, b, ...c,]) { }',
+    'function fn2([...c,]) { }',
+    'fn([1, 2, 3]);',
+    'fn2([1,2,3]);'
+  ];
+
+  TestRun(test)
+    .addError(1, "Invalid element after rest element.")
+    .addError(2, "Invalid element after rest element.")
+    .test(code, { esnext: true });
+
+  test.done();
+};
+
+
+exports.extraRestOperator = function (test) {
+  var code = [
+    'function fn([a, b, ......c]) { }',
+    'fn([1, 2, 3]);'
+  ];
+
+  TestRun(test)
+    .addError(1, "Unexpected '...'.")
     .test(code, { esnext: true });
 
   test.done();