Merge pull request #3141 from ruby/multi-target

Fix up multi target parsing

include/prism/parser.h

@@ -392,6 +392,9 @@ typedef enum {
     /** a rescue statement within a module statement */
     PM_CONTEXT_MODULE_RESCUE,
 
+    /** a multiple target expression */
+    PM_CONTEXT_MULTI_TARGET,
+
     /** a parenthesized expression */
     PM_CONTEXT_PARENS,
 

src/prism.c

@@ -8570,6 +8570,7 @@ context_terminator(pm_context_t context, pm_token_t *token) {
         case PM_CONTEXT_MAIN:
         case PM_CONTEXT_DEF_PARAMS:
         case PM_CONTEXT_DEFINED:
+        case PM_CONTEXT_MULTI_TARGET:
         case PM_CONTEXT_TERNARY:
         case PM_CONTEXT_RESCUE_MODIFIER:
             return token->type == PM_TOKEN_EOF;
@@ -8774,6 +8775,7 @@ context_human(pm_context_t context) {
         case PM_CONTEXT_LOOP_PREDICATE: return "loop predicate";
         case PM_CONTEXT_MAIN: return "top level context";
         case PM_CONTEXT_MODULE: return "module definition";
+        case PM_CONTEXT_MULTI_TARGET: return "multiple targets";
         case PM_CONTEXT_PARENS: return "parentheses";
         case PM_CONTEXT_POSTEXE: return "'END' block";
         case PM_CONTEXT_PREDICATE: return "predicate";
@@ -13799,6 +13801,13 @@ parse_targets(pm_parser_t *parser, pm_node_t *first_target, pm_binding_power_t b
             pm_node_t *splat = (pm_node_t *) pm_splat_node_create(parser, &star_operator, name);
             pm_multi_target_node_targets_append(parser, result, splat);
             has_rest = true;
+        } else if (match1(parser, PM_TOKEN_PARENTHESIS_LEFT)) {
+            context_push(parser, PM_CONTEXT_MULTI_TARGET);
+            pm_node_t *target = parse_expression(parser, binding_power, false, false, PM_ERR_EXPECT_EXPRESSION_AFTER_COMMA, (uint16_t) (depth + 1));
+            target = parse_target(parser, target, true, false);
+
+            pm_multi_target_node_targets_append(parser, result, target);
+            context_pop(parser);
         } else if (token_begins_expression_p(parser->current.type)) {
             pm_node_t *target = parse_expression(parser, binding_power, false, false, PM_ERR_EXPECT_EXPRESSION_AFTER_COMMA, (uint16_t) (depth + 1));
             target = parse_target(parser, target, true, false);
@@ -15502,6 +15511,7 @@ parse_return(pm_parser_t *parser, pm_node_t *node) {
             case PM_CONTEXT_IF:
             case PM_CONTEXT_LOOP_PREDICATE:
             case PM_CONTEXT_MAIN:
+            case PM_CONTEXT_MULTI_TARGET:
             case PM_CONTEXT_PARENS:
             case PM_CONTEXT_POSTEXE:
             case PM_CONTEXT_PREDICATE:
@@ -15630,6 +15640,7 @@ parse_block_exit(pm_parser_t *parser, pm_node_t *node) {
             case PM_CONTEXT_MODULE_ENSURE:
             case PM_CONTEXT_MODULE_RESCUE:
             case PM_CONTEXT_MODULE:
+            case PM_CONTEXT_MULTI_TARGET:
             case PM_CONTEXT_PARENS:
             case PM_CONTEXT_PREDICATE:
             case PM_CONTEXT_RESCUE_MODIFIER:
@@ -17781,6 +17792,7 @@ parse_retry(pm_parser_t *parser, const pm_node_t *node) {
             case PM_CONTEXT_LAMBDA_BRACES:
             case PM_CONTEXT_LAMBDA_DO_END:
             case PM_CONTEXT_LOOP_PREDICATE:
+            case PM_CONTEXT_MULTI_TARGET:
             case PM_CONTEXT_PARENS:
             case PM_CONTEXT_POSTEXE:
             case PM_CONTEXT_PREDICATE:
@@ -17864,6 +17876,7 @@ parse_yield(pm_parser_t *parser, const pm_node_t *node) {
             case PM_CONTEXT_LAMBDA_ENSURE:
             case PM_CONTEXT_LAMBDA_RESCUE:
             case PM_CONTEXT_LOOP_PREDICATE:
+            case PM_CONTEXT_MULTI_TARGET:
             case PM_CONTEXT_PARENS:
             case PM_CONTEXT_POSTEXE:
             case PM_CONTEXT_PREDICATE:
@@ -18121,14 +18134,32 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
                     multi_target->base.location.start = lparen_loc.start;
                     multi_target->base.location.end = rparen_loc.end;
 
-                    if (match1(parser, PM_TOKEN_COMMA)) {
-                        if (binding_power == PM_BINDING_POWER_STATEMENT) {
-                            return parse_targets_validate(parser, (pm_node_t *) multi_target, PM_BINDING_POWER_INDEX, (uint16_t) (depth + 1));
-                        }
-                        return (pm_node_t *) multi_target;
+                    pm_node_t *result;
+                    if (match1(parser, PM_TOKEN_COMMA) && (binding_power == PM_BINDING_POWER_STATEMENT)) {
+                        result = parse_targets(parser, (pm_node_t *) multi_target, PM_BINDING_POWER_INDEX, (uint16_t) (depth + 1));
+                        accept1(parser, PM_TOKEN_NEWLINE);
+                    } else {
+                        result = (pm_node_t *) multi_target;
+                    }
+
+                    if (context_p(parser, PM_CONTEXT_MULTI_TARGET)) {
+                        // All set, this is explicitly allowed by the parent
+                        // context.
+                    } else if (context_p(parser, PM_CONTEXT_FOR_INDEX) && match1(parser, PM_TOKEN_KEYWORD_IN)) {
+                        // All set, we're inside a for loop and we're parsing
+                        // multiple targets.
+                    } else if (binding_power != PM_BINDING_POWER_STATEMENT) {
+                        // Multi targets are not allowed when it's not a
+                        // statement level.
+                        pm_parser_err_node(parser, result, PM_ERR_WRITE_TARGET_UNEXPECTED);
+                    } else if (!match2(parser, PM_TOKEN_EQUAL, PM_TOKEN_PARENTHESIS_RIGHT)) {
+                        // Multi targets must be followed by an equal sign in
+                        // order to be valid (or a right parenthesis if they are
+                        // nested).
+                        pm_parser_err_node(parser, result, PM_ERR_WRITE_TARGET_UNEXPECTED);
                     }
 
-                    return parse_target_validate(parser, (pm_node_t *) multi_target, false);
+                    return result;
                 }
 
                 // If we have a single statement and are ending on a right parenthesis
@@ -18185,6 +18216,17 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
                 }
             }
 
+            // When we're parsing multi targets, we allow them to be followed by
+            // a right parenthesis if they are at the statement level. This is
+            // only possible if they are the final statement in a parentheses.
+            // We need to explicitly reject that here.
+            {
+                const pm_node_t *statement = statements->body.nodes[statements->body.size - 1];
+                if (PM_NODE_TYPE_P(statement, PM_MULTI_TARGET_NODE)) {
+                    pm_parser_err_node(parser, statement, PM_ERR_WRITE_TARGET_UNEXPECTED);
+                }
+            }
+
             context_pop(parser);
             pm_accepts_block_stack_pop(parser);
             expect1(parser, PM_TOKEN_PARENTHESIS_RIGHT, PM_ERR_EXPECT_RPAREN);

test/prism/errors/multi_target_parens.txt

@@ -0,0 +1,19 @@
+( + ( * ) )
+    ^~~~~ unexpected write target
+( a ( * ) )
+    ^~~~~ unexpected write target
+( 1 + ( * ) )
+      ^~~~~ unexpected write target
+( .. ( * ) )
+     ^~~~~ unexpected write target
+( a = ( * ) )
+      ^~~~~ unexpected write target
+( * = ( * ) )
+      ^~~~~ unexpected write target
+( a if ( * ) )
+       ^~~~~ unexpected write target
+( 1; ( * ) )
+     ^~~~~ unexpected write target
+( def f() = ( * ) )
+            ^~~~~ unexpected write target
+

test/prism/errors/multi_target_star.txt

@@ -0,0 +1,17 @@
+[(*),]
+ ^~~ unexpected write target
+[1,(a,*b,(c,d)),1]
+   ^~~~~~~~~~~~ unexpected write target
+{a:(*),}
+   ^~~ unexpected write target
+[1+(*),]
+   ^~~ unexpected write target
+x=(*),1
+  ^~~ unexpected write target
+p((*),)
+  ^~~ unexpected write target
+p (*),1
+  ^~~ unexpected write target
+x = def f = (*),1
+            ^~~ unexpected write target
+