Fix parsing of arrow function formal parameters

src/preparser.h

Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index 18fc2330eedd7f646683035bba73e311d6f8d12a..f8f20e530aa18d2f7e4c8c1d892470d33dc662c6 100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -2988,6 +2988,7 @@ ParserBase<Traits>::ParseConditionalExpression(bool accept_IN,
   ExpressionT expression =
       this->ParseBinaryExpression(4, accept_IN, classifier, CHECK_OK);
   if (peek() != Token::CONDITIONAL) return expression;
+  ArrowFormalParametersUnexpectedToken(classifier);
   BindingPatternUnexpectedToken(classifier);
   Consume(Token::CONDITIONAL);
   // In parsing the first assignment expression in conditional
@@ -3013,6 +3014,7 @@ ParserBase<Traits>::ParseBinaryExpression(int prec, bool accept_IN,
     // prec1 >= 4
     while (Precedence(peek(), accept_IN) == prec1) {
       BindingPatternUnexpectedToken(classifier);
+      ArrowFormalParametersUnexpectedToken(classifier);
       Token::Value op = Next();
       Scanner::Location op_location = scanner()->location();
       int pos = position();
@@ -3075,6 +3077,7 @@ ParserBase<Traits>::ParseUnaryExpression(ExpressionClassifier* classifier,
   Token::Value op = peek();
   if (Token::IsUnaryOp(op)) {
     BindingPatternUnexpectedToken(classifier);
+    ArrowFormalParametersUnexpectedToken(classifier);
 
     op = Next();
     int pos = position();
@@ -3097,6 +3100,7 @@ ParserBase<Traits>::ParseUnaryExpression(ExpressionClassifier* classifier,
     return this->BuildUnaryExpression(expression, op, pos, factory());
   } else if (Token::IsCountOp(op)) {
     BindingPatternUnexpectedToken(classifier);
+    ArrowFormalParametersUnexpectedToken(classifier);
     op = Next();
     int beg_pos = peek_position();
     ExpressionT expression = this->ParseUnaryExpression(classifier, CHECK_OK);
@@ -3129,6 +3133,7 @@ ParserBase<Traits>::ParsePostfixExpression(ExpressionClassifier* classifier,
   if (!scanner()->HasAnyLineTerminatorBeforeNext() &&
       Token::IsCountOp(peek())) {
     BindingPatternUnexpectedToken(classifier);
+    ArrowFormalParametersUnexpectedToken(classifier);
 
     expression = this->CheckAndRewriteReferenceExpression(
         expression, lhs_beg_pos, scanner()->location().end_pos,
@@ -3160,6 +3165,7 @@ ParserBase<Traits>::ParseLeftHandSideExpression(
     switch (peek()) {
       case Token::LBRACK: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);
         Consume(Token::LBRACK);
         int pos = position();
         ExpressionT index = ParseExpression(true, classifier, CHECK_OK);
@@ -3170,6 +3176,7 @@ ParserBase<Traits>::ParseLeftHandSideExpression(
 
       case Token::LPAREN: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);
 
         if (is_strong(language_mode()) && this->IsIdentifier(result) &&
             this->IsEval(this->AsIdentifier(result))) {
@@ -3231,6 +3238,7 @@ ParserBase<Traits>::ParseLeftHandSideExpression(
 
       case Token::PERIOD: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);
         Consume(Token::PERIOD);
         int pos = position();
         IdentifierT name = ParseIdentifierName(CHECK_OK);
@@ -3243,6 +3251,7 @@ ParserBase<Traits>::ParseLeftHandSideExpression(
       case Token::TEMPLATE_SPAN:
       case Token::TEMPLATE_TAIL: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);
         result = ParseTemplateLiteral(result, position(), classifier, CHECK_OK);
         break;
       }
@@ -3280,6 +3289,7 @@ ParserBase<Traits>::ParseMemberWithNewPrefixesExpression(
 
   if (peek() == Token::NEW) {
     BindingPatternUnexpectedToken(classifier);
+    ArrowFormalParametersUnexpectedToken(classifier);
     Consume(Token::NEW);
     int new_pos = position();
     ExpressionT result = this->EmptyExpression();
@@ -3333,6 +3343,7 @@ ParserBase<Traits>::ParseMemberExpression(ExpressionClassifier* classifier,
   ExpressionT result = this->EmptyExpression();
   if (peek() == Token::FUNCTION) {
     BindingPatternUnexpectedToken(classifier);
+    ArrowFormalParametersUnexpectedToken(classifier);
 
     Consume(Token::FUNCTION);
     int function_token_position = position();
@@ -3576,6 +3587,7 @@ ParserBase<Traits>::ParseMemberExpressionContinuation(
     switch (peek()) {
       case Token::LBRACK: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);
 
         Consume(Token::LBRACK);
         int pos = position();
@@ -3589,6 +3601,7 @@ ParserBase<Traits>::ParseMemberExpressionContinuation(
       }
       case Token::PERIOD: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);

[caitp (gmail), 2015/08/20 15:22:57]

probably want to make sure destructuring still works properly, `({ x: x =
foo.bar }) => {}` for example. I don't expect `({ x = foo.bar }) => {}` to work
properly because CoverInitializedName parsing is pretty messed up in
maybe-expressions, might be worth marking that as a TODO though in the test case

This also applies to other tokens which can legally occur in binding patterns
within arrow formals, I guess.

If this is covered by other tests, then that's ok

[wingo, 2015/08/20 15:47:57]

Thanks for having a look!

I'm pretty sure that destructuring should still work properly, and the reasoning
is a bit convoluted: an assignmentexpression is a valid arrow formal parameter
list if it is a bare identifier, or if it starts with a parenthesis, only
contains binding patterns separated by commas, possibly ending in a rest pattern
(which, oddly, can't contain patterns).  OK.  That is what was already the case
before and is covered by DestructuringPositiveTests and
DestructuringNegativeTests in test-parser.cc.

The way the above classifier works is that when parsing a parenthesized list in
ParsePrimaryExpression, the expression as a whole remains a valid
ArrowFormalParameterList iff the inner productions are valid BindingPatterns. 
Specifically the invalid-ArrowFormalParameterList *does not* accumulate out from
the individual arguments -- instead it's the BindingPattern flag that propagates
out.  See the gnarly special case in ExpressionClassifier::Accumulate.  So I
don't think additional tests are necessary for existing destructuring foo.

What we missed is that the classifier would accept things as valid
ArrowFormalParameterLists that weren't valid.  That's what this patch fixes. 
These additional flags won't cause problems for arrow function formals because
they are explicitly ignored there.

 
         Consume(Token::PERIOD);
         int pos = position();
@@ -3603,6 +3616,7 @@ ParserBase<Traits>::ParseMemberExpressionContinuation(
       case Token::TEMPLATE_SPAN:
       case Token::TEMPLATE_TAIL: {
         BindingPatternUnexpectedToken(classifier);
+        ArrowFormalParametersUnexpectedToken(classifier);
         int pos;
         if (scanner()->current_token() == Token::IDENTIFIER) {
           pos = position();