[parser] report invalid rest parameter errors in Arrow functions

Based on vogelheim's CL at https://codereview.chromium.org/1657783002/

BUG=chromium:582626, v8:2700
LOG=N
R=adamk@chromium.org, rossberg@chromium.org, vogelheim@chromium.org

Review URL: https://codereview.chromium.org/1656993002

Cr-Commit-Position: refs/heads/master@{#33651}

src/messages.h

@@ -71,7 +71,6 @@ class CallSite {
   int32_t pos_;
 };
 
-
 #define MESSAGE_TEMPLATES(T)                                                   \
   /* Error */                                                                  \
   T(None, "")                                                                  \
@@ -417,6 +416,8 @@ class CallSite {
   T(NoCatchOrFinally, "Missing catch or finally after try")                    \
   T(NotIsvar, "builtin %%IS_VAR: not a variable")                              \
   T(ParamAfterRest, "Rest parameter must be last formal parameter")            \
+  T(InvalidRestParameter,                                                      \
+    "Rest parameter must be an identifier or destructuring pattern")           \
   T(PushPastSafeLength,                                                        \
     "Pushing % elements on an array-like of length % "                         \
     "is disallowed, as the total surpasses 2**53-1")                           \

src/parsing/parser-base.h

@@ -1329,8 +1329,14 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
                                           MessageTemplate::kUnexpectedToken,
                                           Token::String(Token::ELLIPSIS));
         classifier->RecordNonSimpleParameter();
-        ExpressionT expr =
-            this->ParseAssignmentExpression(true, classifier, CHECK_OK);
+        ExpressionT expr = this->ParseAssignmentExpression(
+            true, kIsPossibleArrowFormals, classifier, CHECK_OK);
+        if (!this->IsIdentifier(expr) && !expr->IsObjectLiteral() &&
+            !expr->IsArrayLiteral()) {
+          classifier->RecordArrowFormalParametersError(
+              Scanner::Location(ellipsis_pos, scanner()->location().end_pos),
+              MessageTemplate::kInvalidRestParameter);
+        }
         if (peek() == Token::COMMA) {
           ReportMessageAt(scanner()->peek_location(),
                           MessageTemplate::kParamAfterRest);
@@ -1458,7 +1464,15 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseExpression(
     int pos = position(), expr_pos = peek_position();
     ExpressionT right = this->ParseAssignmentExpression(
         accept_IN, flags, &binding_classifier, CHECK_OK);
-    if (is_rest) right = factory()->NewSpread(right, pos, expr_pos);
+    if (is_rest) {
+      if (!this->IsIdentifier(right) && !right->IsObjectLiteral() &&
+          !right->IsArrayLiteral()) {
+        classifier->RecordArrowFormalParametersError(
+            Scanner::Location(pos, scanner()->location().end_pos),
+            MessageTemplate::kInvalidRestParameter);
+      }
+      right = factory()->NewSpread(right, pos, expr_pos);
+    }
     is_simple_parameter_list =
         is_simple_parameter_list && this->IsIdentifier(right);
     classifier->Accumulate(binding_classifier,

test/cctest/test-parsing.cc

@@ -3712,6 +3712,8 @@ TEST(ErrorsArrowFormalParameters) {
 TEST(ErrorsArrowFunctions) {
   // Tests that parser and preparser generate the same kind of errors
   // on invalid arrow function syntax.
+
+  // clang-format off
   const char* context_data[][2] = {
     {"", ";"},
     {"v = ", ";"},
@@ -3812,8 +3814,14 @@ TEST(ErrorsArrowFunctions) {
     "(c, a.b) => {}",
     "(a['b'], c) => {}",
     "(c, a['b']) => {}",
+
+    // crbug.com/582626
+    "(...rest - a) => b",
+    "(a, ...b - 10) => b",
+
     NULL
   };
+  // clang-format on
 
   // The test is quite slow, so run it with a reduced set of flags.
   static const ParserFlag flags[] = {kAllowLazy};
@@ -7976,9 +7984,20 @@ TEST(EscapedKeywords) {
 
 
 TEST(MiscSyntaxErrors) {
+  // clang-format off
   const char* context_data[][2] = {
-      {"'use strict'", ""}, {"", ""}, {NULL, NULL}};
-  const char* error_data[] = {"for (();;) {}", NULL};
+    { "'use strict'", "" },
+    { "", "" },
+    { NULL, NULL }
+  };
+  const char* error_data[] = {
+    "for (();;) {}",
+
+    // crbug.com/582626
+    "{ NaN ,chA((evarA=new t ( l = !.0[((... co -a0([1]))=> greturnkf",
+    NULL
+  };
+  // clang-format on
 
   RunParserSyncTest(context_data, error_data, kError, NULL, 0, NULL, 0);
 }

test/message/arrow-invalid-rest-2.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+//
+
+var f = (a, ...x = 10) => x;
+f(1, 2, 3, 4, 5);

test/message/arrow-invalid-rest-2.out

+*%(basename)s:7: SyntaxError: Rest parameter must be an identifier or destructuring pattern
+var f = (a, ...x = 10) => x;
+            ^^^^^^^^^
+SyntaxError: Rest parameter must be an identifier or destructuring pattern

test/message/arrow-invalid-rest.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+//
+
+var f = (...x = 10) => x;
+f(1, 2, 3, 4, 5);

test/message/arrow-invalid-rest.out

+*%(basename)s:7: SyntaxError: Rest parameter must be an identifier or destructuring pattern
+var f = (...x = 10) => x;
+         ^^^^^^^^^
+SyntaxError: Rest parameter must be an identifier or destructuring pattern