Repository: ruby/yarp (c)

Issue:
FlipFlopNode instead of RangeNode with flags YP_RANGE_NODE_FLAGS_FLIP_FLOP
I think this would be clearer and a lot less error-prone.
Otherwise it sounds likely many usages would incorrectly treat flip flops as Range literals.
Notably, it doesn't make sense to ever create a Range object for a flip-flop.
I.e., they look alike but they have nothing in common in terms of semantics.

Pull request description:
FlipFlopNode as opposed to flip flop flag
Fixes #1238

Patch:
diff --git a/config.yml b/config.yml
index fd4ac706..87647d06 100644
--- a/config.yml
+++ b/config.yml
@@ -337,12 +337,10 @@ flags:
     values:
       - name: BEGIN_MODIFIER
         comment: "a loop after a begin statement, so the body is executed first before the condition"
-  - name: RangeNodeFlags
+  - name: RangeFlags
     values:
       - name: EXCLUDE_END
         comment: "... operator"
-      - name: FLIP_FLOP
-        comment: "a range used as a flip flop"
   - name: RegularExpressionFlags
     values:
       - name: IGNORE_CASE
@@ -1056,6 +1054,22 @@ nodes:
 
           foo in Foo(*bar, baz, *qux)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  - name: FlipFlopNode
+    child_nodes:
+      - name: left
+        type: node?
+      - name: right
+        type: node?
+      - name: operator_loc
+        type: location
+      - name: flags
+        type: flags
+        kind: RangeFlags
+    comment: |
+      Represents the use of the `..` or `...` operators to create flip flops.
+
+          baz if foo .. bar
+                 ^^^^^^^^^^
   - name: FloatNode
     comment: |
       Represents a floating point number literal.
@@ -1749,7 +1763,7 @@ nodes:
         type: location
       - name: flags
         type: flags
-        kind: RangeNodeFlags
+        kind: RangeFlags
     comment: |
       Represents the use of the `..` or `...` operators.
 
diff --git a/src/yarp.c b/src/yarp.c
index 57c4258c..cb1f399b 100644
--- a/src/yarp.c
+++ b/src/yarp.c
@@ -459,7 +459,14 @@ yp_flip_flop(yp_node_t *node) {
         }
         case YP_NODE_RANGE_NODE: {
             yp_range_node_t *cast = (yp_range_node_t *) node;
-            cast->base.flags |= YP_RANGE_NODE_FLAGS_FLIP_FLOP;
+            yp_flip_flop(cast->left);
+            yp_flip_flop(cast->right);
+
+            // Here we change the range node into a flip flop node. We can do
+            // this since the nodes are exactly the same except for the type.
+            assert(sizeof(yp_range_node_t) == sizeof(yp_flip_flop_node_t));
+            node->type = YP_NODE_FLIP_FLOP_NODE;
+
             break;
         }
         default:
@@ -3462,7 +3469,7 @@ yp_range_node_create(yp_parser_t *parser, yp_node_t *left, const yp_token_t *ope
     switch (operator->type) {
         case YP_TOKEN_DOT_DOT_DOT:
         case YP_TOKEN_UDOT_DOT_DOT:
-            node->base.flags |= YP_RANGE_NODE_FLAGS_EXCLUDE_END;
+            node->base.flags |= YP_RANGE_FLAGS_EXCLUDE_END;
             break;
         default:
             break;
diff --git a/templates/lib/yarp/node.rb.erb b/templates/lib/yarp/node.rb.erb
index f95e782d..1024f354 100644
--- a/templates/lib/yarp/node.rb.erb
+++ b/templates/lib/yarp/node.rb.erb
@@ -20,19 +20,21 @@ module YARP
     end
     <%# potential override of set_newline_flag -%>
     <%- if node.newline == false -%>
+
     def set_newline_flag(newline_marked)
       # Never mark <%= node.name %> with a newline flag, mark children instead
     end
     <%- elsif node.newline.is_a?(String) -%>
+
     def set_newline_flag(newline_marked)
-    <%- param = node.params.find { |p| p.name == node.newline } or raise node.newline -%>
-    <%- case param -%>
-    <%- in SingleNodeParam -%>
+      <%- param = node.params.find { |p| p.name == node.newline } or raise node.newline -%>
+      <%- case param -%>
+      <%- in SingleNodeParam -%>
       <%= param.name %>.set_newline_flag(newline_marked)
-    <%- in NodeListParam -%>
+      <%- in NodeListParam -%>
       first = <%= param.name %>.first
       first.set_newline_flag(newline_marked) if first
-    <%- end -%>
+      <%- end -%>
     end
     <%- end -%>
 


Test patch:
diff --git a/test/snapshots/not.txt b/test/snapshots/not.txt
index 6eb1e44f..5a9889bf 100644
--- a/test/snapshots/not.txt
+++ b/test/snapshots/not.txt
@@ -190,7 +190,7 @@ ProgramNode(0...190)(
        "!"
      ),
      CallNode(158...172)(
-       RangeNode(162...172)(
+       FlipFlopNode(162...172)(
          CallNode(162...165)(
            nil,
            nil,
@@ -214,7 +214,7 @@ ProgramNode(0...190)(
            "bar"
          ),
          (166...168),
-         2
+         0
        ),
        nil,
        (158...161),
@@ -228,7 +228,7 @@ ProgramNode(0...190)(
      CallNode(174...190)(
        ParenthesesNode(178...190)(
          StatementsNode(179...189)(
-           [RangeNode(179...189)(
+           [FlipFlopNode(179...189)(
               CallNode(179...182)(
                 nil,
                 nil,
@@ -252,7 +252,7 @@ ProgramNode(0...190)(
                 "bar"
               ),
               (183...185),
-              2
+              0
             )]
          ),
          (178...179),
diff --git a/test/snapshots/seattlerb/flip2_env_lvar.txt b/test/snapshots/seattlerb/flip2_env_lvar.txt
index ca482698..bca1426f 100644
--- a/test/snapshots/seattlerb/flip2_env_lvar.txt
+++ b/test/snapshots/seattlerb/flip2_env_lvar.txt
@@ -3,11 +3,11 @@ ProgramNode(0...16)(
   StatementsNode(0...16)(
     [IfNode(0...16)(
        (0...2),
-       RangeNode(3...7)(
+       FlipFlopNode(3...7)(
          CallNode(3...4)(nil, nil, (3...4), nil, nil, nil, nil, 2, "a"),
          CallNode(6...7)(nil, nil, (6...7), nil, nil, nil, nil, 2, "b"),
          (4...6),
-         2
+         0
        ),
        nil,
        nil,
diff --git a/test/snapshots/unparser/corpus/literal/flipflop.txt b/test/snapshots/unparser/corpus/literal/flipflop.txt
index 9128dd05..b08835a4 100644
--- a/test/snapshots/unparser/corpus/literal/flipflop.txt
+++ b/test/snapshots/unparser/corpus/literal/flipflop.txt
@@ -5,7 +5,7 @@ ProgramNode(0...68)(
        (0...2),
        ParenthesesNode(3...23)(
          StatementsNode(4...22)(
-           [RangeNode(4...22)(
+           [FlipFlopNode(4...22)(
               ParenthesesNode(4...12)(
                 StatementsNode(5...11)(
                   [CallNode(5...11)(
@@ -61,7 +61,7 @@ ProgramNode(0...68)(
                 (21...22)
               ),
               (12...14),
-              2
+              0
             )]
          ),
          (3...4),
@@ -77,7 +77,7 @@ ProgramNode(0...68)(
        (34...36),
        ParenthesesNode(37...58)(
          StatementsNode(38...57)(
-           [RangeNode(38...57)(
+           [FlipFlopNode(38...57)(
               ParenthesesNode(38...46)(
                 StatementsNode(39...45)(
                   [CallNode(39...45)(
@@ -133,7 +133,7 @@ ProgramNode(0...68)(
                 (56...57)
               ),
               (46...49),
-              3
+              1
             )]
          ),
          (37...38),
diff --git a/test/snapshots/unparser/corpus/semantic/and.txt b/test/snapshots/unparser/corpus/semantic/and.txt
index 8b0ba672..de402670 100644
--- a/test/snapshots/unparser/corpus/semantic/and.txt
+++ b/test/snapshots/unparser/corpus/semantic/and.txt
@@ -34,17 +34,17 @@ ProgramNode(0...77)(
      IfNode(32...53)(
        (32...34),
        OrNode(35...49)(
-         RangeNode(35...40)(
+         FlipFlopNode(35...40)(
            CallNode(35...36)(nil, nil, (35...36), nil, nil, nil, nil, 2, "a"),
            CallNode(39...40)(nil, nil, (39...40), nil, nil, nil, nil, 2, "b"),
            (36...39),
-           3
+           1
          ),
-         RangeNode(44...49)(
+         FlipFlopNode(44...49)(
            CallNode(44...45)(nil, nil, (44...45), nil, nil, nil, nil, 2, "c"),
            CallNode(48...49)(nil, nil, (48...49), nil, nil, nil, nil, 2, "d"),
            (45...48),
-           3
+           1
          ),
          (41...43)
        ),
@@ -55,17 +55,17 @@ ProgramNode(0...77)(
      IfNode(55...77)(
        (55...57),
        AndNode(58...73)(
-         RangeNode(58...63)(
+         FlipFlopNode(58...63)(
            CallNode(58...59)(nil, nil, (58...59), nil, nil, nil, nil, 2, "a"),
            CallNode(62...63)(nil, nil, (62...63), nil, nil, nil, nil, 2, "b"),
            (59...62),
-           3
+           1
          ),
-         RangeNode(68...73)(
+         FlipFlopNode(68...73)(
            CallNode(68...69)(nil, nil, (68...69), nil, nil, nil, nil, 2, "c"),
            CallNode(72...73)(nil, nil, (72...73), nil, nil, nil, nil, 2, "d"),
            (69...72),
-           3
+           1
          ),
          (64...67)
        ),
diff --git a/test/snapshots/whitequark/cond_eflipflop.txt b/test/snapshots/whitequark/cond_eflipflop.txt
index a675ad6e..31ef52f1 100644
--- a/test/snapshots/whitequark/cond_eflipflop.txt
+++ b/test/snapshots/whitequark/cond_eflipflop.txt
@@ -4,7 +4,7 @@ ProgramNode(0...31)(
     [CallNode(0...12)(
        ParenthesesNode(1...12)(
          StatementsNode(2...11)(
-           [RangeNode(2...11)(
+           [FlipFlopNode(2...11)(
               CallNode(2...5)(nil, nil, (2...5), nil, nil, nil, nil, 2, "foo"),
               CallNode(8...11)(
                 nil,
@@ -18,7 +18,7 @@ ProgramNode(0...31)(
                 "bar"
               ),
               (5...8),
-              3
+              1
             )]
          ),
          (1...2),
@@ -35,11 +35,11 @@ ProgramNode(0...31)(
      ),
      IfNode(14...31)(
        (14...16),
-       RangeNode(17...26)(
+       FlipFlopNode(17...26)(
          CallNode(17...20)(nil, nil, (17...20), nil, nil, nil, nil, 2, "foo"),
          CallNode(23...26)(nil, nil, (23...26), nil, nil, nil, nil, 2, "bar"),
          (20...23),
-         3
+         1
        ),
        nil,
        nil,
diff --git a/test/snapshots/whitequark/cond_iflipflop.txt b/test/snapshots/whitequark/cond_iflipflop.txt
index 7a164424..4d1d42ed 100644
--- a/test/snapshots/whitequark/cond_iflipflop.txt
+++ b/test/snapshots/whitequark/cond_iflipflop.txt
@@ -4,7 +4,7 @@ ProgramNode(0...29)(
     [CallNode(0...11)(
        ParenthesesNode(1...11)(
          StatementsNode(2...10)(
-           [RangeNode(2...10)(
+           [FlipFlopNode(2...10)(
               CallNode(2...5)(nil, nil, (2...5), nil, nil, nil, nil, 2, "foo"),
               CallNode(7...10)(
                 nil,
@@ -18,7 +18,7 @@ ProgramNode(0...29)(
                 "bar"
               ),
               (5...7),
-              2
+              0
             )]
          ),
          (1...2),
@@ -35,11 +35,11 @@ ProgramNode(0...29)(
      ),
      IfNode(13...29)(
        (13...15),
-       RangeNode(16...24)(
+       FlipFlopNode(16...24)(
          CallNode(16...19)(nil, nil, (16...19), nil, nil, nil, nil, 2, "foo"),
          CallNode(21...24)(nil, nil, (21...24), nil, nil, nil, nil, 2, "bar"),
          (19...21),
-         2
+         0
        ),
        nil,
        nil,