[ruby/prism] Ensure no extra multi-target nodes are created for splats

https://github.com/ruby/prism/commit/e60240d54b

8 files changed, 68 insertions, 117 deletions

diff --git a/prism/prism.c b/prism/prism.c
index 5cb18a7b30..299c8cdb7f 100644
--- a/prism/prism.c
+++ b/prism/prism.c
@@ -9509,10 +9509,7 @@ parse_target(pm_parser_t *parser, pm_node_t *target) {
                 splat->expression = parse_target(parser, splat->expression);
             }
 
-            pm_multi_target_node_t *multi_target = pm_multi_target_node_create(parser);
-            pm_multi_target_node_targets_append(parser, multi_target, (pm_node_t *) splat);
-
-            return (pm_node_t *) multi_target;
+            return (pm_node_t *) splat;
         }
         case PM_CALL_NODE: {
             pm_call_node_t *call = (pm_call_node_t *) target;
@@ -9802,7 +9799,7 @@ parse_targets(pm_parser_t *parser, pm_node_t *first_target, pm_binding_power_t b
             target = parse_target(parser, target);
 
             pm_multi_target_node_targets_append(parser, result, target);
-        } else {
+        } else if (!match1(parser, PM_TOKEN_EOF)) {
             // If we get here, then we have a trailing , in a multi target node.
             // We need to indicate this somehow in the tree, so we'll add an
             // anonymous splat.
@@ -12526,12 +12523,13 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) {
                 parser_lex(parser);
                 pm_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 (PM_NODE_TYPE_P(statement, PM_MULTI_TARGET_NODE)) {
+                if (PM_NODE_TYPE_P(statement, PM_MULTI_TARGET_NODE) || PM_NODE_TYPE_P(statement, PM_SPLAT_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.
                     pm_multi_target_node_t *multi_target;
-                    if (((pm_multi_target_node_t *) statement)->lparen_loc.start == NULL) {
+
+                    if (PM_NODE_TYPE_P(statement, PM_MULTI_TARGET_NODE) && ((pm_multi_target_node_t *) statement)->lparen_loc.start == NULL) {
                         multi_target = (pm_multi_target_node_t *) statement;
                     } else {
                         multi_target = pm_multi_target_node_create(parser);
@@ -14575,18 +14573,13 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
                     return parse_write(parser, node, &token, value);
                 }
                 case PM_SPLAT_NODE: {
-                    pm_splat_node_t *splat_node = (pm_splat_node_t *) node;
+                    pm_multi_target_node_t *multi_target = pm_multi_target_node_create(parser);
+                    pm_multi_target_node_targets_append(parser, multi_target, node);
 
-                    switch (PM_NODE_TYPE(splat_node->expression)) {
-                        case PM_CASE_WRITABLE:
-                            parser_lex(parser);
-                            pm_node_t *value = parse_assignment_value(parser, previous_binding_power, binding_power, PM_ERR_EXPECT_EXPRESSION_AFTER_EQUAL);
-                            return parse_write(parser, (pm_node_t *) splat_node, &token, value);
-                        default:
-                            break;
-                    }
+                    parser_lex(parser);
+                    pm_node_t *value = parse_assignment_value(parser, previous_binding_power, binding_power, PM_ERR_EXPECT_EXPRESSION_AFTER_EQUAL);
+                    return parse_write(parser, (pm_node_t *) multi_target, &token, value);
                 }
-                /* fallthrough */
                 default:
                     parser_lex(parser);
 
diff --git a/test/prism/errors_test.rb b/test/prism/errors_test.rb
index 6cf501c4c1..e08c2f744d 100644
--- a/test/prism/errors_test.rb
+++ b/test/prism/errors_test.rb
@@ -1202,7 +1202,7 @@ module Prism
     def test_invalid_global_variable_write
       assert_errors expression("$',"), "$',", [
         ["Immutable variable as a write target", 0..2],
-        ["Unexpected write target", 0..3]
+        ["Unexpected write target", 0..2]
       ]
     end
 
diff --git a/test/prism/snapshots/seattlerb/masgn_anon_splat_arg.txt b/test/prism/snapshots/seattlerb/masgn_anon_splat_arg.txt
index bac1576259..53d6ebf340 100644
--- a/test/prism/snapshots/seattlerb/masgn_anon_splat_arg.txt
+++ b/test/prism/snapshots/seattlerb/masgn_anon_splat_arg.txt
@@ -4,21 +4,15 @@
     @ StatementsNode (location: (1,0)-(1,8))
     └── body: (length: 1)
         └── @ MultiWriteNode (location: (1,0)-(1,8))
-            ├── requireds: (length: 2)
-            │   ├── @ MultiTargetNode (location: (1,0)-(1,1))
-            │   │   ├── requireds: (length: 0)
-            │   │   ├── rest:
-            │   │   │   @ SplatNode (location: (1,0)-(1,1))
-            │   │   │   ├── operator_loc: (1,0)-(1,1) = "*"
-            │   │   │   └── expression: ∅
-            │   │   ├── posts: (length: 0)
-            │   │   ├── lparen_loc: ∅
-            │   │   └── rparen_loc: ∅
+            ├── requireds: (length: 0)
+            ├── rest:
+            │   @ SplatNode (location: (1,0)-(1,1))
+            │   ├── operator_loc: (1,0)-(1,1) = "*"
+            │   └── expression: ∅
+            ├── posts: (length: 1)
             │   └── @ LocalVariableTargetNode (location: (1,3)-(1,4))
             │       ├── name: :a
             │       └── depth: 0
-            ├── rest: ∅
-            ├── posts: (length: 0)
             ├── lparen_loc: ∅
             ├── rparen_loc: ∅
             ├── operator_loc: (1,5)-(1,6) = "="
diff --git a/test/prism/snapshots/seattlerb/masgn_splat_arg.txt b/test/prism/snapshots/seattlerb/masgn_splat_arg.txt
index 30f451fcb6..fe7698f376 100644
--- a/test/prism/snapshots/seattlerb/masgn_splat_arg.txt
+++ b/test/prism/snapshots/seattlerb/masgn_splat_arg.txt
@@ -4,24 +4,18 @@
     @ StatementsNode (location: (1,0)-(1,9))
     └── body: (length: 1)
         └── @ MultiWriteNode (location: (1,0)-(1,9))
-            ├── requireds: (length: 2)
-            │   ├── @ MultiTargetNode (location: (1,0)-(1,2))
-            │   │   ├── requireds: (length: 0)
-            │   │   ├── rest:
-            │   │   │   @ SplatNode (location: (1,0)-(1,2))
-            │   │   │   ├── operator_loc: (1,0)-(1,1) = "*"
-            │   │   │   └── expression:
-            │   │   │       @ LocalVariableTargetNode (location: (1,1)-(1,2))
-            │   │   │       ├── name: :a
-            │   │   │       └── depth: 0
-            │   │   ├── posts: (length: 0)
-            │   │   ├── lparen_loc: ∅
-            │   │   └── rparen_loc: ∅
+            ├── requireds: (length: 0)
+            ├── rest:
+            │   @ SplatNode (location: (1,0)-(1,2))
+            │   ├── operator_loc: (1,0)-(1,1) = "*"
+            │   └── expression:
+            │       @ LocalVariableTargetNode (location: (1,1)-(1,2))
+            │       ├── name: :a
+            │       └── depth: 0
+            ├── posts: (length: 1)
             │   └── @ LocalVariableTargetNode (location: (1,4)-(1,5))
             │       ├── name: :b
             │       └── depth: 0
-            ├── rest: ∅
-            ├── posts: (length: 0)
             ├── lparen_loc: ∅
             ├── rparen_loc: ∅
             ├── operator_loc: (1,6)-(1,7) = "="
diff --git a/test/prism/snapshots/seattlerb/masgn_splat_arg_arg.txt b/test/prism/snapshots/seattlerb/masgn_splat_arg_arg.txt
index 386ad7b563..231b11f3fe 100644
--- a/test/prism/snapshots/seattlerb/masgn_splat_arg_arg.txt
+++ b/test/prism/snapshots/seattlerb/masgn_splat_arg_arg.txt
@@ -4,27 +4,21 @@
     @ StatementsNode (location: (1,0)-(1,12))
     └── body: (length: 1)
         └── @ MultiWriteNode (location: (1,0)-(1,12))
-            ├── requireds: (length: 3)
-            │   ├── @ MultiTargetNode (location: (1,0)-(1,2))
-            │   │   ├── requireds: (length: 0)
-            │   │   ├── rest:
-            │   │   │   @ SplatNode (location: (1,0)-(1,2))
-            │   │   │   ├── operator_loc: (1,0)-(1,1) = "*"
-            │   │   │   └── expression:
-            │   │   │       @ LocalVariableTargetNode (location: (1,1)-(1,2))
-            │   │   │       ├── name: :a
-            │   │   │       └── depth: 0
-            │   │   ├── posts: (length: 0)
-            │   │   ├── lparen_loc: ∅
-            │   │   └── rparen_loc: ∅
+            ├── requireds: (length: 0)
+            ├── rest:
+            │   @ SplatNode (location: (1,0)-(1,2))
+            │   ├── operator_loc: (1,0)-(1,1) = "*"
+            │   └── expression:
+            │       @ LocalVariableTargetNode (location: (1,1)-(1,2))
+            │       ├── name: :a
+            │       └── depth: 0
+            ├── posts: (length: 2)
             │   ├── @ LocalVariableTargetNode (location: (1,4)-(1,5))
             │   │   ├── name: :b
             │   │   └── depth: 0
             │   └── @ LocalVariableTargetNode (location: (1,7)-(1,8))
             │       ├── name: :c
             │       └── depth: 0
-            ├── rest: ∅
-            ├── posts: (length: 0)
             ├── lparen_loc: ∅
             ├── rparen_loc: ∅
             ├── operator_loc: (1,9)-(1,10) = "="
diff --git a/test/prism/snapshots/seattlerb/mlhs_front_anonsplat.txt b/test/prism/snapshots/seattlerb/mlhs_front_anonsplat.txt
index 04df31deaa..5d919ee689 100644
--- a/test/prism/snapshots/seattlerb/mlhs_front_anonsplat.txt
+++ b/test/prism/snapshots/seattlerb/mlhs_front_anonsplat.txt
@@ -4,16 +4,12 @@
     @ StatementsNode (location: (1,0)-(1,14))
     └── body: (length: 1)
         └── @ MultiWriteNode (location: (1,0)-(1,14))
-            ├── requireds: (length: 4)
-            │   ├── @ MultiTargetNode (location: (1,0)-(1,1))
-            │   │   ├── requireds: (length: 0)
-            │   │   ├── rest:
-            │   │   │   @ SplatNode (location: (1,0)-(1,1))
-            │   │   │   ├── operator_loc: (1,0)-(1,1) = "*"
-            │   │   │   └── expression: ∅
-            │   │   ├── posts: (length: 0)
-            │   │   ├── lparen_loc: ∅
-            │   │   └── rparen_loc: ∅
+            ├── requireds: (length: 0)
+            ├── rest:
+            │   @ SplatNode (location: (1,0)-(1,1))
+            │   ├── operator_loc: (1,0)-(1,1) = "*"
+            │   └── expression: ∅
+            ├── posts: (length: 3)
             │   ├── @ LocalVariableTargetNode (location: (1,3)-(1,4))
             │   │   ├── name: :x
             │   │   └── depth: 0
@@ -23,8 +19,6 @@
             │   └── @ LocalVariableTargetNode (location: (1,9)-(1,10))
             │       ├── name: :z
             │       └── depth: 0
-            ├── rest: ∅
-            ├── posts: (length: 0)
             ├── lparen_loc: ∅
             ├── rparen_loc: ∅
             ├── operator_loc: (1,11)-(1,12) = "="
diff --git a/test/prism/snapshots/seattlerb/mlhs_front_splat.txt b/test/prism/snapshots/seattlerb/mlhs_front_splat.txt
index 51cd7fbfca..dc595542e5 100644
--- a/test/prism/snapshots/seattlerb/mlhs_front_splat.txt
+++ b/test/prism/snapshots/seattlerb/mlhs_front_splat.txt
@@ -4,19 +4,15 @@
     @ StatementsNode (location: (1,0)-(1,15))
     └── body: (length: 1)
         └── @ MultiWriteNode (location: (1,0)-(1,15))
-            ├── requireds: (length: 4)
-            │   ├── @ MultiTargetNode (location: (1,0)-(1,2))
-            │   │   ├── requireds: (length: 0)
-            │   │   ├── rest:
-            │   │   │   @ SplatNode (location: (1,0)-(1,2))
-            │   │   │   ├── operator_loc: (1,0)-(1,1) = "*"
-            │   │   │   └── expression:
-            │   │   │       @ LocalVariableTargetNode (location: (1,1)-(1,2))
-            │   │   │       ├── name: :s
-            │   │   │       └── depth: 0
-            │   │   ├── posts: (length: 0)
-            │   │   ├── lparen_loc: ∅
-            │   │   └── rparen_loc: ∅
+            ├── requireds: (length: 0)
+            ├── rest:
+            │   @ SplatNode (location: (1,0)-(1,2))
+            │   ├── operator_loc: (1,0)-(1,1) = "*"
+            │   └── expression:
+            │       @ LocalVariableTargetNode (location: (1,1)-(1,2))
+            │       ├── name: :s
+            │       └── depth: 0
+            ├── posts: (length: 3)
             │   ├── @ LocalVariableTargetNode (location: (1,4)-(1,5))
             │   │   ├── name: :x
             │   │   └── depth: 0
@@ -26,8 +22,6 @@
             │   └── @ LocalVariableTargetNode (location: (1,10)-(1,11))
             │       ├── name: :z
             │       └── depth: 0
-            ├── rest: ∅
-            ├── posts: (length: 0)
             ├── lparen_loc: ∅
             ├── rparen_loc: ∅
             ├── operator_loc: (1,12)-(1,13) = "="
diff --git a/test/prism/snapshots/whitequark/masgn_splat.txt b/test/prism/snapshots/whitequark/masgn_splat.txt
index f496124b07..9662ef5399 100644
--- a/test/prism/snapshots/whitequark/masgn_splat.txt
+++ b/test/prism/snapshots/whitequark/masgn_splat.txt
@@ -25,24 +25,18 @@
         │       ├── flags: variable_call
         │       └── name: :bar
         ├── @ MultiWriteNode (location: (3,0)-(3,13))
-        │   ├── requireds: (length: 3)
-        │   │   ├── @ MultiTargetNode (location: (3,0)-(3,1))
-        │   │   │   ├── requireds: (length: 0)
-        │   │   │   ├── rest:
-        │   │   │   │   @ SplatNode (location: (3,0)-(3,1))
-        │   │   │   │   ├── operator_loc: (3,0)-(3,1) = "*"
-        │   │   │   │   └── expression: ∅
-        │   │   │   ├── posts: (length: 0)
-        │   │   │   ├── lparen_loc: ∅
-        │   │   │   └── rparen_loc: ∅
+        │   ├── requireds: (length: 0)
+        │   ├── rest:
+        │   │   @ SplatNode (location: (3,0)-(3,1))
+        │   │   ├── operator_loc: (3,0)-(3,1) = "*"
+        │   │   └── expression: ∅
+        │   ├── posts: (length: 2)
         │   │   ├── @ LocalVariableTargetNode (location: (3,3)-(3,4))
         │   │   │   ├── name: :c
         │   │   │   └── depth: 0
         │   │   └── @ LocalVariableTargetNode (location: (3,6)-(3,7))
         │   │       ├── name: :d
         │   │       └── depth: 0
-        │   ├── rest: ∅
-        │   ├── posts: (length: 0)
         │   ├── lparen_loc: ∅
         │   ├── rparen_loc: ∅
         │   ├── operator_loc: (3,8)-(3,9) = "="
@@ -82,24 +76,18 @@
         │       ├── flags: variable_call
         │       └── name: :bar
         ├── @ MultiWriteNode (location: (7,0)-(7,11))
-        │   ├── requireds: (length: 2)
-        │   │   ├── @ MultiTargetNode (location: (7,0)-(7,2))
-        │   │   │   ├── requireds: (length: 0)
-        │   │   │   ├── rest:
-        │   │   │   │   @ SplatNode (location: (7,0)-(7,2))
-        │   │   │   │   ├── operator_loc: (7,0)-(7,1) = "*"
-        │   │   │   │   └── expression:
-        │   │   │   │       @ LocalVariableTargetNode (location: (7,1)-(7,2))
-        │   │   │   │       ├── name: :b
-        │   │   │   │       └── depth: 0
-        │   │   │   ├── posts: (length: 0)
-        │   │   │   ├── lparen_loc: ∅
-        │   │   │   └── rparen_loc: ∅
+        │   ├── requireds: (length: 0)
+        │   ├── rest:
+        │   │   @ SplatNode (location: (7,0)-(7,2))
+        │   │   ├── operator_loc: (7,0)-(7,1) = "*"
+        │   │   └── expression:
+        │   │       @ LocalVariableTargetNode (location: (7,1)-(7,2))
+        │   │       ├── name: :b
+        │   │       └── depth: 0
+        │   ├── posts: (length: 1)
         │   │   └── @ LocalVariableTargetNode (location: (7,4)-(7,5))
         │   │       ├── name: :c
         │   │       └── depth: 0
-        │   ├── rest: ∅
-        │   ├── posts: (length: 0)
         │   ├── lparen_loc: ∅
         │   ├── rparen_loc: ∅
         │   ├── operator_loc: (7,6)-(7,7) = "="