Repository: ruby/yarp
Pull request #1454 (2023-09-12 18:36:29)

Improve #toString() for Java nodes by showing field names and field values
* Locations are excluded because those will be removed soon.

Before:
```
 /home/runner/work/yarp/yarp/test/yarp/fixtures/xstring.txt
ProgramNode
  StatementsNode
    XStringNode[Li]
    InterpolatedXStringNode
      StringNode[Li]
      EmbeddedStatementsNode
        StatementsNode
          CallNode
      StringNode
    XStringNode[Li]
    XStringNode[Li]
```
After:
```
/home/eregon/code/yarp/test/yarp/fixtures/xstring.txt
ProgramNode
  locals: 
  statements: StatementsNode
    body: 
      XStringNode[Li]
        unescaped: "foo"
      InterpolatedXStringNode
        parts: 
          StringNode[Li]
            unescaped: "foo "
          EmbeddedStatementsNode
            statements: StatementsNode
              body: 
                CallNode
                  receiver: null
                  arguments: null
                  block: null
                  flags: 2
                  name: "bar"
          StringNode
            unescaped: " baz"
      XStringNode[Li]
        unescaped: "foo"
      XStringNode[Li]
        unescaped: "foo"
```

cc @enebo 

eregon: It seems better to add `"` around string values to clarify whitespace, I'll add that.
eregon: Done.
eregon: This should make it easier to investigate this bug: https://github.com/ruby/yarp/pull/1413#discussion_r1322799294

Patch:
diff --git a/config.yml b/config.yml
index 7210ad79..411c1154 100644
--- a/config.yml
+++ b/config.yml
@@ -370,7 +370,7 @@ flags:
       - name: ONCE
         comment: "o - only interpolates values into the regular expression once"
 nodes:
-  - name: AliasGlobalVariableNode
+  - name: AliasNode
     fields:
       - name: new_name
         type: node
@@ -379,20 +379,7 @@ nodes:
       - name: keyword_loc
         type: location
     comment: |
-      Represents the use of the `alias` keyword to alias a global variable.
-
-          alias $foo $bar
-          ^^^^^^^^^^^^^^^
-  - name: AliasMethodNode
-    fields:
-      - name: new_name
-        type: node
-      - name: old_name
-        type: node
-      - name: keyword_loc
-        type: location
-    comment: |
-      Represents the use of the `alias` keyword to alias a method.
+      Represents the use of the `alias` keyword.
 
           alias foo bar
           ^^^^^^^^^^^^^
diff --git a/include/yarp.h b/include/yarp.h
index 6e1db007..378efe0c 100644
--- a/include/yarp.h
+++ b/include/yarp.h
@@ -28,10 +28,6 @@
 #include <strings.h>
 #endif
 
-// These aliases are for migration purposes.
-#define YP_ALIAS_NODE YP_ALIAS_METHOD_NODE
-#define yp_alias_node_t yp_alias_method_node_t
-
 void yp_serialize_content(yp_parser_t *parser, yp_node_t *node, yp_buffer_t *buffer);
 
 void yp_print_node(yp_parser_t *parser, yp_node_t *node);
diff --git a/rust/yarp/build.rs b/rust/yarp/build.rs
index 0b739c46..7944bdfd 100644
--- a/rust/yarp/build.rs
+++ b/rust/yarp/build.rs
@@ -519,21 +519,12 @@ impl<'pr> ConstantId<'pr> {{
     }}
 
     /// Returns a byte slice for the constant ID.
-    ///
-    /// # Panics
-    ///
-    /// Panics if the constant ID is not found in the constant pool.
     #[must_use]
     pub fn as_slice(&self) -> &'pr [u8] {{
         unsafe {{
             let pool = &(*self.parser.as_ptr()).constant_pool;
-            for i in 0..pool.capacity {{
-                let constant = &(*pool.constants.add(i));
-                if constant.id() == self.id {{
-                    return std::slice::from_raw_parts(constant.start, constant.length);
-                }}
-            }}
-            panic!("Unable to locate constant id");
+            let constant = &(*pool.constants.offset(isize::try_from(self.id).expect("id should be in range")));
+            std::slice::from_raw_parts(constant.start, constant.length)
         }}
     }}
 }}
diff --git a/src/yarp.c b/src/yarp.c
index 6379ee50..28d71f69 100644
--- a/src/yarp.c
+++ b/src/yarp.c
@@ -735,37 +735,15 @@ yp_missing_node_create(yp_parser_t *parser, const uint8_t *start, const uint8_t
     return node;
 }
 
-// Allocate and initialize a new AliasGlobalVariableNode node.
-static yp_alias_global_variable_node_t *
-yp_alias_global_variable_node_create(yp_parser_t *parser, const yp_token_t *keyword, yp_node_t *new_name, yp_node_t *old_name) {
+// Allocate and initialize a new alias node.
+static yp_alias_node_t *
+yp_alias_node_create(yp_parser_t *parser, const yp_token_t *keyword, yp_node_t *new_name, yp_node_t *old_name) {
     assert(keyword->type == YP_TOKEN_KEYWORD_ALIAS);
-    yp_alias_global_variable_node_t *node = YP_ALLOC_NODE(parser, yp_alias_global_variable_node_t);
+    yp_alias_node_t *node = YP_ALLOC_NODE(parser, yp_alias_node_t);
 
-    *node = (yp_alias_global_variable_node_t) {
+    *node = (yp_alias_node_t) {
         {
-            .type = YP_ALIAS_GLOBAL_VARIABLE_NODE,
-            .location = {
-                .start = keyword->start,
-                .end = old_name->location.end
-            },
-        },
-        .new_name = new_name,
-        .old_name = old_name,
-        .keyword_loc = YP_LOCATION_TOKEN_VALUE(keyword)
-    };
-
-    return node;
-}
-
-// Allocate and initialize a new AliasMethodNode node.
-static yp_alias_method_node_t *
-yp_alias_method_node_create(yp_parser_t *parser, const yp_token_t *keyword, yp_node_t *new_name, yp_node_t *old_name) {
-    assert(keyword->type == YP_TOKEN_KEYWORD_ALIAS);
-    yp_alias_method_node_t *node = YP_ALLOC_NODE(parser, yp_alias_method_node_t);
-
-    *node = (yp_alias_method_node_t) {
-        {
-            .type = YP_ALIAS_METHOD_NODE,
+            .type = YP_ALIAS_NODE,
             .location = {
                 .start = keyword->start,
                 .end = old_name->location.end
@@ -11435,6 +11413,13 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
             yp_node_t *old_name = parse_alias_argument(parser, false);
 
             switch (YP_NODE_TYPE(new_name)) {
+                case YP_SYMBOL_NODE:
+                case YP_INTERPOLATED_SYMBOL_NODE: {
+                    if (!YP_NODE_TYPE_P(old_name, YP_SYMBOL_NODE) && !YP_NODE_TYPE_P(old_name, YP_INTERPOLATED_SYMBOL_NODE)) {
+                        yp_diagnostic_list_append(&parser->error_list, old_name->location.start, old_name->location.end, YP_ERR_ALIAS_ARGUMENT);
+                    }
+                    break;
+                }
                 case YP_BACK_REFERENCE_READ_NODE:
                 case YP_NUMBERED_REFERENCE_READ_NODE:
                 case YP_GLOBAL_VARIABLE_READ_NODE: {
@@ -11445,19 +11430,13 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) {
                     } else {
                         yp_diagnostic_list_append(&parser->error_list, old_name->location.start, old_name->location.end, YP_ERR_ALIAS_ARGUMENT);
                     }
-
-                    return (yp_node_t *) yp_alias_global_variable_node_create(parser, &keyword, new_name, old_name);
-                }
-                case YP_SYMBOL_NODE:
-                case YP_INTERPOLATED_SYMBOL_NODE: {
-                    if (!YP_NODE_TYPE_P(old_name, YP_SYMBOL_NODE) && !YP_NODE_TYPE_P(old_name, YP_INTERPOLATED_SYMBOL_NODE)) {
-                        yp_diagnostic_list_append(&parser->error_list, old_name->location.start, old_name->location.end, YP_ERR_ALIAS_ARGUMENT);
-                    }
+                    break;
                 }
-                /* fallthrough */
                 default:
-                    return (yp_node_t *) yp_alias_method_node_create(parser, &keyword, new_name, old_name);
+                    break;
             }
+
+            return (yp_node_t *) yp_alias_node_create(parser, &keyword, new_name, old_name);
         }
         case YP_TOKEN_KEYWORD_CASE: {
             parser_lex(parser);
diff --git a/templates/java/org/yarp/Nodes.java.erb b/templates/java/org/yarp/Nodes.java.erb
index 16d5281d..abcef11c 100644
--- a/templates/java/org/yarp/Nodes.java.erb
+++ b/templates/java/org/yarp/Nodes.java.erb
@@ -3,6 +3,7 @@ package org.yarp;
 import java.lang.Override;
 import java.lang.String;
 import java.lang.StringBuilder;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 
@@ -117,20 +118,7 @@ public abstract class Nodes {
             return toString("");
         }
 
-        private String toString(String indent) {
-            StringBuilder builder = new StringBuilder();
-            builder.append(indent).append(this.getClass().getSimpleName());
-            if (hasNewLineFlag()) {
-                builder.append("[Li]");
-            }
-            builder.append('\n');
-            for (Node child : childNodes()) {
-                if (child != null) {
-                    builder.append(child.toString(indent + "  "));
-                }
-            }
-            return builder.toString();
-        }
+        protected abstract String toString(String indent);
     }
 <%# FLAGS -%>
     <%- flags.each do |group| -%>
@@ -287,6 +275,52 @@ public abstract class Nodes {
         public <T> T accept(AbstractNodeVisitor<T> visitor) {
             return visitor.visit<%= node.name -%>(this);
         }
+
+        @Override
+        protected String toString(String indent) {
+            StringBuilder builder = new StringBuilder();
+            builder.append(this.getClass().getSimpleName());
+            if (hasNewLineFlag()) {
+                builder.append("[Li]");
+            }
+            builder.append('\n');
+            String nextIndent = indent + "  ";
+            <%- if node.fields.any?(YARP::NodeListField) or node.fields.any?(YARP::ConstantListField) -%>
+            String nextNextIndent = nextIndent + "  ";
+            <%- end -%>
+            <%- node.fields.grep_v(YARP::LocationField).grep_v(YARP::OptionalLocationField).each do |field| -%>
+            builder.append(nextIndent);
+            builder.append("<%= field.name %>: ");
+            <%- case field -%>
+            <%- when YARP::NodeField -%>
+            builder.append(this.<%= field.name %>.toString(nextIndent));
+            <%- when YARP::OptionalNodeField -%>
+            builder.append(this.<%= field.name %> == null ? "null\n" : this.<%= field.name %>.toString(nextIndent));
+            <%- when YARP::NodeListField -%>
+            builder.append('\n');
+            for (Node child : this.<%= field.name %>) {
+                builder.append(nextNextIndent).append(child.toString(nextNextIndent));
+            }
+            <%- when YARP::StringField, YARP::ConstantField -%>
+            builder.append('"' + new String(this.<%= field.name %>, StandardCharsets.UTF_8) + '"');
+            builder.append('\n');
+            <%- when YARP::OptionalConstantField -%>
+            builder.append(this.<%= field.name %> == null ? "null\n" : '"' + new String(this.<%= field.name %>, StandardCharsets.UTF_8) + '"');
+            builder.append('\n');
+            <%- when YARP::ConstantListField -%>
+            builder.append('\n');
+            for (byte[] constant : this.<%= field.name %>) {
+                builder.append(nextNextIndent).append('"' + new String(constant, StandardCharsets.UTF_8) + '"').append('\n');
+            }
+            <%- when YARP::UInt32Field, YARP::FlagsField -%>
+            builder.append(this.<%= field.name %>);
+            builder.append('\n');
+            <%- else -%>
+            <%- raise field.class.name -%>
+            <%- end -%>
+            <%- end -%>
+            return builder.toString();
+        }
     }
     <%- end -%>
 


Test patch:
diff --git a/test/yarp/location_test.rb b/test/yarp/location_test.rb
index a452dfad..253d6160 100644
--- a/test/yarp/location_test.rb
+++ b/test/yarp/location_test.rb
@@ -4,12 +4,8 @@ require_relative "test_helper"
 
 module YARP
   class LocationTest < TestCase
-    def test_AliasGlobalVariableNode
-      assert_location(AliasGlobalVariableNode, "alias $foo $bar")
-    end
-
-    def test_AliasMethodNode
-      assert_location(AliasMethodNode, "alias foo bar")
+    def test_AliasNode
+      assert_location(AliasNode, "alias foo bar")
     end
 
     def test_AlternationPatternNode
diff --git a/test/yarp/snapshots/alias.txt b/test/yarp/snapshots/alias.txt
index 8709a213..2074f3e4 100644
--- a/test/yarp/snapshots/alias.txt
+++ b/test/yarp/snapshots/alias.txt
@@ -3,7 +3,7 @@
 └── statements:
     @ StatementsNode (location: (0...199))
     └── body: (length: 12)
-        ├── @ AliasMethodNode (location: (0...15))
+        ├── @ AliasNode (location: (0...15))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (6...10))
         │   │   ├── opening_loc: (6...7) = ":"
@@ -17,7 +17,7 @@
         │   │   ├── closing_loc: ∅
         │   │   └── unescaped: "bar"
         │   └── keyword_loc: (0...5) = "alias"
-        ├── @ AliasMethodNode (location: (17...38))
+        ├── @ AliasNode (location: (17...38))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (23...30))
         │   │   ├── opening_loc: (23...26) = "%s["
@@ -31,7 +31,7 @@
         │   │   ├── closing_loc: (37...38) = "]"
         │   │   └── unescaped: "def"
         │   └── keyword_loc: (17...22) = "alias"
-        ├── @ AliasMethodNode (location: (40...59))
+        ├── @ AliasNode (location: (40...59))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (46...52))
         │   │   ├── opening_loc: (46...48) = ":'"
@@ -45,7 +45,7 @@
         │   │   ├── closing_loc: (58...59) = "'"
         │   │   └── unescaped: "def"
         │   └── keyword_loc: (40...45) = "alias"
-        ├── @ AliasMethodNode (location: (61...84))
+        ├── @ AliasNode (location: (61...84))
         │   ├── new_name:
         │   │   @ InterpolatedSymbolNode (location: (67...77))
         │   │   ├── opening_loc: (67...69) = ":\""
@@ -71,14 +71,14 @@
         │   │   ├── closing_loc: (83...84) = "'"
         │   │   └── unescaped: "def"
         │   └── keyword_loc: (61...66) = "alias"
-        ├── @ AliasGlobalVariableNode (location: (86...97))
+        ├── @ AliasNode (location: (86...97))
         │   ├── new_name:
         │   │   @ GlobalVariableReadNode (location: (92...94))
         │   │   └── name: :$a
         │   ├── old_name:
         │   │   @ BackReferenceReadNode (location: (95...97))
         │   └── keyword_loc: (86...91) = "alias"
-        ├── @ AliasMethodNode (location: (99...112))
+        ├── @ AliasNode (location: (99...112))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (105...108))
         │   │   ├── opening_loc: ∅
@@ -92,7 +92,7 @@
         │   │   ├── closing_loc: ∅
         │   │   └── unescaped: "bar"
         │   └── keyword_loc: (99...104) = "alias"
-        ├── @ AliasGlobalVariableNode (location: (114...129))
+        ├── @ AliasNode (location: (114...129))
         │   ├── new_name:
         │   │   @ GlobalVariableReadNode (location: (120...124))
         │   │   └── name: :$foo
@@ -100,7 +100,7 @@
         │   │   @ GlobalVariableReadNode (location: (125...129))
         │   │   └── name: :$bar
         │   └── keyword_loc: (114...119) = "alias"
-        ├── @ AliasMethodNode (location: (131...143))
+        ├── @ AliasNode (location: (131...143))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (137...140))
         │   │   ├── opening_loc: ∅
@@ -114,7 +114,7 @@
         │   │   ├── closing_loc: ∅
         │   │   └── unescaped: "if"
         │   └── keyword_loc: (131...136) = "alias"
-        ├── @ AliasMethodNode (location: (145...158))
+        ├── @ AliasNode (location: (145...158))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (151...154))
         │   │   ├── opening_loc: ∅
@@ -128,7 +128,7 @@
         │   │   ├── closing_loc: ∅
         │   │   └── unescaped: "<=>"
         │   └── keyword_loc: (145...150) = "alias"
-        ├── @ AliasMethodNode (location: (160...175))
+        ├── @ AliasNode (location: (160...175))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (166...169))
         │   │   ├── opening_loc: (166...167) = ":"
@@ -142,7 +142,7 @@
         │   │   ├── closing_loc: ∅
         │   │   └── unescaped: "eql?"
         │   └── keyword_loc: (160...165) = "alias"
-        ├── @ AliasMethodNode (location: (177...186))
+        ├── @ AliasNode (location: (177...186))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (183...184))
         │   │   ├── opening_loc: ∅
@@ -156,7 +156,7 @@
         │   │   ├── closing_loc: ∅
         │   │   └── unescaped: "B"
         │   └── keyword_loc: (177...182) = "alias"
-        └── @ AliasMethodNode (location: (188...199))
+        └── @ AliasNode (location: (188...199))
             ├── new_name:
             │   @ SymbolNode (location: (194...196))
             │   ├── opening_loc: (194...195) = ":"
diff --git a/test/yarp/snapshots/seattlerb/alias_gvar_backref.txt b/test/yarp/snapshots/seattlerb/alias_gvar_backref.txt
index 83406fe8..c667bbfc 100644
--- a/test/yarp/snapshots/seattlerb/alias_gvar_backref.txt
+++ b/test/yarp/snapshots/seattlerb/alias_gvar_backref.txt
@@ -3,7 +3,7 @@
 └── statements:
     @ StatementsNode (location: (0...15))
     └── body: (length: 1)
-        └── @ AliasGlobalVariableNode (location: (0...15))
+        └── @ AliasNode (location: (0...15))
             ├── new_name:
             │   @ GlobalVariableReadNode (location: (6...12))
             │   └── name: :$MATCH
diff --git a/test/yarp/snapshots/seattlerb/alias_resword.txt b/test/yarp/snapshots/seattlerb/alias_resword.txt
index 418af5d9..0a0b7b18 100644
--- a/test/yarp/snapshots/seattlerb/alias_resword.txt
+++ b/test/yarp/snapshots/seattlerb/alias_resword.txt
@@ -3,7 +3,7 @@
 └── statements:
     @ StatementsNode (location: (0...12))
     └── body: (length: 1)
-        └── @ AliasMethodNode (location: (0...12))
+        └── @ AliasNode (location: (0...12))
             ├── new_name:
             │   @ SymbolNode (location: (6...8))
             │   ├── opening_loc: ∅
diff --git a/test/yarp/snapshots/seattlerb/dsym_to_sym.txt b/test/yarp/snapshots/seattlerb/dsym_to_sym.txt
index d4187447..8c79a29d 100644
--- a/test/yarp/snapshots/seattlerb/dsym_to_sym.txt
+++ b/test/yarp/snapshots/seattlerb/dsym_to_sym.txt
@@ -3,7 +3,7 @@
 └── statements:
     @ StatementsNode (location: (0...32))
     └── body: (length: 2)
-        ├── @ AliasMethodNode (location: (0...17))
+        ├── @ AliasNode (location: (0...17))
         │   ├── new_name:
         │   │   @ SymbolNode (location: (6...11))
         │   │   ├── opening_loc: (6...8) = ":\""
@@ -17,7 +17,7 @@
         │   │   ├── closing_loc: (16...17) = "\""
         │   │   └── unescaped: ">>"
         │   └── keyword_loc: (0...5) = "alias"
-        └── @ AliasMethodNode (location: (19...32))
+        └── @ AliasNode (location: (19...32))
             ├── new_name:
             │   @ SymbolNode (location: (25...28))
             │   ├── opening_loc: (25...26) = ":"
diff --git a/test/yarp/snapshots/unparser/corpus/literal/alias.txt b/test/yarp/snapshots/unparser/corpus/literal/alias.txt
index 4427cfb3..96a4e6f6 100644
--- a/test/yarp/snapshots/unparser/corpus/literal/alias.txt
+++ b/test/yarp/snapshots/unparser/corpus/literal/alias.txt
@@ -3,7 +3,7 @@
 └── statements:
     @ StatementsNode (location: (0...31))
     └── body: (length: 2)
-        ├── @ AliasGlobalVariableNode (location: (0...15))
+        ├── @ AliasNode (location: (0...15))
         │   ├── new_name:
         │   │   @ GlobalVariableReadNode (location: (6...10))
         │   │   └── name: :$foo
@@ -11,7 +11,7 @@
         │   │   @ GlobalVariableReadNode (location: (11...15))
         │   │   └── name: :$bar
         │   └── keyword_loc: (0...5) = "alias"
-        └── @ AliasMethodNode (location: (16...31))
+        └── @ AliasNode (location: (16...31))
             ├── new_name:
             │   @ SymbolNode (location: (22...26))
             │   ├── opening_loc: (22...23) = ":"
diff --git a/test/yarp/snapshots/whitequark/alias.txt b/test/yarp/snapshots/whitequark/alias.txt
index 9f6a5861..fce4c912 100644
--- a/test/yarp/snapshots/whitequark/alias.txt
+++ b/test/yarp/snapshots/whitequark/alias.txt
@@ -3,7 +3,7 @@
 └── statements:
     @ StatementsNode (location: (0...14))
     └── body: (length: 1)
-        └── @ AliasMethodNode (location: (0...14))
+        └── @ AliasNode (location: (0...14))
             ├── new_name:
             │   @ SymbolNode (location: (6...10))
             │   ├── opening_loc: (6...7) = ":"
diff --git a/test/yarp/snapshots/whitequark/alias_gvar.txt b/test/yarp/snapshots/whitequark/alias_gvar.txt
index 56dc91c5..0d5616e1 100644
--- a/test/yarp/snapshots/whitequark/alias_gvar.txt
+++ b/test/yarp/snapshots/whitequark/alias_gvar.txt
@@ -3,14 +3,14 @@
 └── statements:
     @ StatementsNode (location: (0...24))
     └── body: (length: 2)
-        ├── @ AliasGlobalVariableNode (location: (0...11))
+        ├── @ AliasNode (location: (0...11))
         │   ├── new_name:
         │   │   @ GlobalVariableReadNode (location: (6...8))
         │   │   └── name: :$a
         │   ├── old_name:
         │   │   @ BackReferenceReadNode (location: (9...11))
         │   └── keyword_loc: (0...5) = "alias"
-        └── @ AliasGlobalVariableNode (location: (13...24))
+        └── @ AliasNode (location: (13...24))
             ├── new_name:
             │   @ GlobalVariableReadNode (location: (19...21))
             │   └── name: :$a