Bug 1041341 - Part 2: Report a SyntaxError for destructuring rest with trailing comma. r=arai
authorAndré Bargull <andre.bargull@gmail.com>
Mon, 10 Oct 2016 13:33:19 -0700
changeset 318560 5c23428eec9b050e4e000dbccb91b6683009adf6
parent 318559 2821a9fdfd258ef73a74608ec4056c186e68b170
child 318561 3b4965bb11c0649ce8e0757c708b6c13d5f1e854
push id20725
push userphilringnalda@gmail.com
push dateThu, 20 Oct 2016 01:36:01 +0000
treeherderfx-team@998ad5a74da8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1041341
milestone52.0a1
Bug 1041341 - Part 2: Report a SyntaxError for destructuring rest with trailing comma. r=arai
js/src/frontend/Parser.cpp
js/src/jit-test/tests/basic/destructuring-rest.js
js/src/js.msg
js/src/tests/ecma_6/Destructuring/rest-with-trailing-comma.js
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -8566,16 +8566,18 @@ Parser<ParseHandler>::arrayInitializer(Y
                 /* If we didn't already match TOK_COMMA in above case. */
                 bool matched;
                 if (!tokenStream.matchToken(&matched, TOK_COMMA))
                     return null();
                 if (!matched) {
                     modifier = TokenStream::None;
                     break;
                 }
+                if (tt == TOK_TRIPLEDOT && possibleError)
+                    possibleError->setPendingDestructuringError(null(), JSMSG_REST_WITH_COMMA);
             }
         }
 
         MUST_MATCH_TOKEN_MOD(TOK_RB, modifier, JSMSG_BRACKET_AFTER_LIST);
     }
     handler.setEndPosition(literal, pos().end);
     return literal;
 }
--- a/js/src/jit-test/tests/basic/destructuring-rest.js
+++ b/js/src/jit-test/tests/basic/destructuring-rest.js
@@ -7,23 +7,17 @@ assertThrowsInstanceOf(() => new Functio
 assertThrowsInstanceOf(() => new Function('[...a=b] = []'), SyntaxError, 'assignment expression');
 assertThrowsInstanceOf(() => new Function('[...a()] = []'), SyntaxError, 'call expression');
 assertThrowsInstanceOf(() => new Function('[...(a,b)] = []'), SyntaxError, 'comma expression');
 assertThrowsInstanceOf(() => new Function('[...a++] = []'), SyntaxError, 'postfix expression');
 assertThrowsInstanceOf(() => new Function('[...!a] = []'), SyntaxError, 'unary expression');
 assertThrowsInstanceOf(() => new Function('[...a+b] = []'), SyntaxError, 'binary expression');
 assertThrowsInstanceOf(() => new Function('var [...a.x] = []'), SyntaxError, 'lvalue expression in declaration');
 assertThrowsInstanceOf(() => new Function('var [...(b)] = []'), SyntaxError);
-
-// XXX: The way the current parser works, a trailing comma is lost before we
-//      check for destructuring.  See bug 1041341. Once fixed, please update
-//      this assertion.
-assertThrowsInstanceOf(() =>
-	assertThrowsInstanceOf(() => new Function('[...b,] = []'), SyntaxError)
-	, Error);
+assertThrowsInstanceOf(() => new Function('[...b,] = []'), SyntaxError);
 
 assertThrowsInstanceOf(() => {
   try {
     eval('let [...[...x]] = (() => { throw "foo"; } )();');
   } catch(e) {
     assertEq(e, "foo");
   }
   x;
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -296,16 +296,17 @@ MSG_DEF(JSMSG_PAREN_BEFORE_COND,       0
 MSG_DEF(JSMSG_PAREN_BEFORE_FORMAL,     0, JSEXN_SYNTAXERR, "missing ( before formal parameters")
 MSG_DEF(JSMSG_PAREN_BEFORE_SWITCH,     0, JSEXN_SYNTAXERR, "missing ( before switch expression")
 MSG_DEF(JSMSG_PAREN_BEFORE_WITH,       0, JSEXN_SYNTAXERR, "missing ( before with-statement object")
 MSG_DEF(JSMSG_PAREN_IN_PAREN,          0, JSEXN_SYNTAXERR, "missing ) in parenthetical")
 MSG_DEF(JSMSG_RC_AFTER_EXPORT_SPEC_LIST, 0, JSEXN_SYNTAXERR, "missing '}' after export specifier list")
 MSG_DEF(JSMSG_RC_AFTER_IMPORT_SPEC_LIST, 0, JSEXN_SYNTAXERR, "missing '}' after module specifier list")
 MSG_DEF(JSMSG_REDECLARED_CATCH_IDENTIFIER, 1, JSEXN_SYNTAXERR, "redeclaration of identifier '{0}' in catch")
 MSG_DEF(JSMSG_RESERVED_ID,             1, JSEXN_SYNTAXERR, "{0} is a reserved identifier")
+MSG_DEF(JSMSG_REST_WITH_COMMA,         0, JSEXN_SYNTAXERR, "rest element may not have a trailing comma")
 MSG_DEF(JSMSG_REST_WITH_DEFAULT,       0, JSEXN_SYNTAXERR, "rest parameter may not have a default")
 MSG_DEF(JSMSG_SELFHOSTED_TOP_LEVEL_LEXICAL, 1, JSEXN_SYNTAXERR, "self-hosted code cannot contain top-level {0} declarations")
 MSG_DEF(JSMSG_SELFHOSTED_METHOD_CALL,  0, JSEXN_SYNTAXERR, "self-hosted code may not contain direct method calls. Use callFunction() or callContentFunction()")
 MSG_DEF(JSMSG_SELFHOSTED_UNBOUND_NAME, 0, JSEXN_TYPEERR, "self-hosted code may not contain unbound name lookups")
 MSG_DEF(JSMSG_SEMI_AFTER_FOR_COND,     0, JSEXN_SYNTAXERR, "missing ; after for-loop condition")
 MSG_DEF(JSMSG_SEMI_AFTER_FOR_INIT,     0, JSEXN_SYNTAXERR, "missing ; after for-loop initializer")
 MSG_DEF(JSMSG_SEMI_BEFORE_STMNT,       0, JSEXN_SYNTAXERR, "missing ; before statement")
 MSG_DEF(JSMSG_SOURCE_TOO_LONG,         0, JSEXN_RANGEERR, "source is too long")
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Destructuring/rest-with-trailing-comma.js
@@ -0,0 +1,45 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+const invalidSyntax = [
+    "[...r, ]",
+    "[a, ...r, ]",
+    "[a = 0, ...r, ]",
+    "[[], ...r, ]",
+    "[[...r,]]",
+    "[[...r,], ]",
+    "[[...r,], a]",
+];
+
+const validSyntax = [
+    "[, ]",
+    "[a, ]",
+    "[[], ]",
+];
+
+const destructuringForms = [
+    a => `${a} = [];`,
+    a => `var ${a} = [];`,
+    a => `let ${a} = [];`,
+    a => `const ${a} = [];`,
+    a => `(${a}) => {};`,
+    a => `(${a} = []) => {};`,
+    a => `function f(${a}) {}`,
+];
+
+for (let invalid of invalidSyntax) {
+    for (let fn of destructuringForms) {
+        assertThrowsInstanceOf(() => Function(fn(invalid)), SyntaxError);
+    }
+}
+
+for (let invalid of validSyntax) {
+    for (let fn of destructuringForms) {
+        Function(fn(invalid));
+    }
+}
+
+
+if (typeof reportCompare === "function")
+    reportCompare(0, 0);