Repository: ruby/yarp
Pull request #1490 (2023-09-14 15:19:31)

Fix multi target parentheses locations


Patch:
diff --git a/src/yarp.c b/src/yarp.c
index 24a5d987..0f83d864 100644
--- a/src/yarp.c
+++ b/src/yarp.c
@@ -11193,28 +11193,27 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
                 parser_lex(parser);
                 yp_accepts_block_stack_pop(parser);
 
-                // If we have a single statement and are ending on a right parenthesis,
-                // then we need to check if this is possibly a multiple target node.
+                // If we have a single statement and are ending on a right
+                // parenthesis, then we need to check if this is possibly a
+                // multiple target node.
                 if (binding_power == YP_BINDING_POWER_STATEMENT && YP_NODE_TYPE_P(statement, YP_MULTI_TARGET_NODE)) {
-                    yp_node_t *target;
-                    yp_multi_target_node_t *multi_target = (yp_multi_target_node_t *) statement;
+                    yp_multi_target_node_t *multi_target;
+                    if (((yp_multi_target_node_t *) statement)->lparen_loc.start == NULL) {
+                        multi_target = (yp_multi_target_node_t *) statement;
+                    } else {
+                        multi_target = yp_multi_target_node_create(parser);
+                        yp_multi_target_node_targets_append(multi_target, statement);
+                    }
 
                     yp_location_t lparen_loc = YP_LOCATION_TOKEN_VALUE(&opening);
                     yp_location_t rparen_loc = YP_LOCATION_TOKEN_VALUE(&parser->previous);
 
-                    if (multi_target->lparen_loc.start == NULL) {
-                        multi_target->base.location.start = lparen_loc.start;
-                        multi_target->base.location.end = rparen_loc.end;
-                        multi_target->lparen_loc = lparen_loc;
-                        multi_target->rparen_loc = rparen_loc;
-                        target = (yp_node_t *) multi_target;
-                    } else {
-                        yp_multi_target_node_t *parent_target = yp_multi_target_node_create(parser);
-                        yp_multi_target_node_targets_append(parent_target, (yp_node_t *) multi_target);
-                        target = (yp_node_t *) parent_target;
-                    }
+                    multi_target->lparen_loc = lparen_loc;
+                    multi_target->rparen_loc = rparen_loc;
+                    multi_target->base.location.start = lparen_loc.start;
+                    multi_target->base.location.end = rparen_loc.end;
 
-                    return parse_targets(parser, target, YP_BINDING_POWER_INDEX);
+                    return parse_targets(parser, (yp_node_t *) multi_target, YP_BINDING_POWER_INDEX);
                 }
 
                 // If we have a single statement and are ending on a right parenthesis
@@ -11226,9 +11225,9 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
                 return (yp_node_t *) yp_parentheses_node_create(parser, &opening, (yp_node_t *) statements, &parser->previous);
             }
 
-            // If we have more than one statement in the set of parentheses, then we
-            // are going to parse all of them as a list of statements. We'll do that
-            // here.
+            // If we have more than one statement in the set of parentheses,
+            // then we are going to parse all of them as a list of statements.
+            // We'll do that here.
             context_push(parser, YP_CONTEXT_PARENS);
             yp_statements_node_t *statements = yp_statements_node_create(parser);
             yp_statements_node_body_append(statements, statement);
@@ -11240,11 +11239,11 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
                 yp_node_t *node = parse_expression(parser, YP_BINDING_POWER_STATEMENT, YP_ERR_CANNOT_PARSE_EXPRESSION);
                 yp_statements_node_body_append(statements, node);
 
-                // If we're recovering from a syntax error, then we need to stop parsing the
-                // statements now.
+                // If we're recovering from a syntax error, then we need to stop
+                // parsing the statements now.
                 if (parser->recovering) {
-                    // If this is the level of context where the recovery has happened, then
-                    // we can mark the parser as done recovering.
+                    // If this is the level of context where the recovery has
+                    // happened, then we can mark the parser as done recovering.
                     if (match1(parser, YP_TOKEN_PARENTHESIS_RIGHT)) parser->recovering = false;
                     break;
                 }


Test patch:
diff --git a/test/yarp/location_test.rb b/test/yarp/location_test.rb
index a452dfad..d76fac5a 100644
--- a/test/yarp/location_test.rb
+++ b/test/yarp/location_test.rb
@@ -556,6 +556,8 @@ module YARP
 
     def test_MultiWriteNode
       assert_location(MultiWriteNode, "foo, bar = baz")
+      assert_location(MultiWriteNode, "(foo, bar) = baz")
+      assert_location(MultiWriteNode, "((foo, bar)) = baz")
     end
 
     def test_NextNode
diff --git a/test/yarp/snapshots/seattlerb/masgn_double_paren.txt b/test/yarp/snapshots/seattlerb/masgn_double_paren.txt
index 3e4fc9af..d2f8b358 100644
--- a/test/yarp/snapshots/seattlerb/masgn_double_paren.txt
+++ b/test/yarp/snapshots/seattlerb/masgn_double_paren.txt
@@ -1,9 +1,9 @@
-@ ProgramNode (location: (1...9))
+@ ProgramNode (location: (0...9))
 ├── locals: [:a, :b]
 └── statements:
-    @ StatementsNode (location: (1...9))
+    @ StatementsNode (location: (0...9))
     └── body: (length: 1)
-        └── @ MultiWriteNode (location: (1...9))
+        └── @ MultiWriteNode (location: (0...9))
             ├── targets: (length: 1)
             │   └── @ MultiTargetNode (location: (1...6))
             │       ├── targets: (length: 2)
@@ -15,8 +15,8 @@
             │       │       └── depth: 0
             │       ├── lparen_loc: (1...2) = "("
             │       └── rparen_loc: (5...6) = ")"
-            ├── lparen_loc: ∅
-            ├── rparen_loc: ∅
+            ├── lparen_loc: (0...1) = "("
+            ├── rparen_loc: (6...7) = ")"
             ├── operator_loc: (7...8) = "="
             └── value:
                 @ CallNode (location: (8...9))
diff --git a/test/yarp/snapshots/whitequark/masgn_nested.txt b/test/yarp/snapshots/whitequark/masgn_nested.txt
index 30982bf9..3a3f07d0 100644
--- a/test/yarp/snapshots/whitequark/masgn_nested.txt
+++ b/test/yarp/snapshots/whitequark/masgn_nested.txt
@@ -1,9 +1,9 @@
-@ ProgramNode (location: (1...30))
+@ ProgramNode (location: (0...30))
 ├── locals: [:b, :a, :c]
 └── statements:
-    @ StatementsNode (location: (1...30))
+    @ StatementsNode (location: (0...30))
     └── body: (length: 2)
-        ├── @ MultiWriteNode (location: (1...13))
+        ├── @ MultiWriteNode (location: (0...13))
         │   ├── targets: (length: 1)
         │   │   └── @ MultiTargetNode (location: (1...6))
         │   │       ├── targets: (length: 2)
@@ -15,8 +15,8 @@
         │   │       │       └── expression: ∅
         │   │       ├── lparen_loc: (1...2) = "("
         │   │       └── rparen_loc: (5...6) = ")"
-        │   ├── lparen_loc: ∅
-        │   ├── rparen_loc: ∅
+        │   ├── lparen_loc: (0...1) = "("
+        │   ├── rparen_loc: (6...7) = ")"
         │   ├── operator_loc: (8...9) = "="
         │   └── value:
         │       @ CallNode (location: (10...13))