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 318554 5c23428eec9b
parent 318553 2821a9fdfd25
child 318555 3b4965bb11c0
push id30843
push usercbook@mozilla.com
push dateWed, 19 Oct 2016 15:02:57 +0000
treeherdermozilla-central@f40960c63bfa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1041341
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);