From b4df6cf7575957799beb5f954f2c74bf65417382 Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Wed, 19 Jul 2023 18:00:46 +0200
Subject: [PATCH 1/8] Set newline flag using a visitor for StatementsNode,
 IfNode and UnlessNode

* These 3 places is what the CRuby and JRuby parser use.
---
 bin/parse                               |  7 ++-
 config.yml                              | 10 ----
 java/org/yarp/MarkNewlinesVisitor.java  | 61 +++++++++++++++++++
 lib/yarp.rb                             | 78 +++++++++++++++++++++++++
 tasks/check_manifest.rake               |  5 +-
 templates/java/org/yarp/Loader.java.erb |  6 +-
 templates/java/org/yarp/Nodes.java.erb  | 30 +++++-----
 templates/lib/yarp/node.rb.erb          | 18 ------
 templates/template.rb                   |  3 +-
 test/parse_test.rb                      | 68 +++++++++++++++++++++
 10 files changed, 232 insertions(+), 54 deletions(-)
 create mode 100644 java/org/yarp/MarkNewlinesVisitor.java

diff --git a/bin/parse b/bin/parse
index 4b90e0ec7e5..10b9e868f6a 100755
--- a/bin/parse
+++ b/bin/parse
@@ -9,7 +9,10 @@ $:.unshift(File.expand_path("../lib", __dir__))
 require "yarp"
 
 if ARGV[0] == "-e"
-  pp YARP.parse(ARGV[1])
+  result = YARP.parse(ARGV[1])
 else
-  pp YARP.parse_file(ARGV[0])
+  result = YARP.parse_file(ARGV[0])
 end
+
+result.mark_newlines if ENV['MARK_NEWLINES']
+pp result
diff --git a/config.yml b/config.yml
index 0997663b7f5..1f8437bb6c1 100644
--- a/config.yml
+++ b/config.yml
@@ -940,7 +940,6 @@ nodes:
         kind: StatementsNode
       - name: end_keyword_loc
         type: location?
-    newline: false
     comment: |
       Represents an `else` clause in a `case`, `if`, or `unless` statement.
 
@@ -1179,7 +1178,6 @@ nodes:
         type: node?
       - name: end_keyword_loc
         type: location?
-    newline: false
     comment: |
       Represents the use of the `if` keyword, either in the block form or the modifier form.
 
@@ -1289,7 +1287,6 @@ nodes:
         type: location
       - name: flags
         type: uint32
-    newline: false
     comment: |
       Represents a regular expression literal that contains interpolation.
 
@@ -1303,7 +1300,6 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location?
-    newline: false
     comment: |
       Represents a string literal that contains interpolation.
 
@@ -1317,7 +1313,6 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location?
-    newline: false
     comment: |
       Represents a symbol literal that contains interpolation.
 
@@ -1331,7 +1326,6 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location
-    newline: false
     comment: |
       Represents an xstring literal that contains interpolation.
 
@@ -1628,7 +1622,6 @@ nodes:
         type: location
       - name: closing_loc
         type: location
-    newline: false
     comment: |
       Represents a parenthesized expression
 
@@ -1701,7 +1694,6 @@ nodes:
       - name: statements
         type: node
         kind: StatementsNode
-    newline: false
     comment: The top level node of any parse tree.
   - name: RangeNode
     child_nodes:
@@ -1909,7 +1901,6 @@ nodes:
     child_nodes:
       - name: body
         type: node[]
-    newline: false
     comment: |
       Represents a set of statements contained within some scope.
 
@@ -2019,7 +2010,6 @@ nodes:
         kind: ElseNode
       - name: end_keyword_loc
         type: location?
-    newline: false
     comment: |
       Represents the use of the `unless` keyword, either in the block form or the modifier form.
 
diff --git a/java/org/yarp/MarkNewlinesVisitor.java b/java/org/yarp/MarkNewlinesVisitor.java
new file mode 100644
index 00000000000..dc9912f4e96
--- /dev/null
+++ b/java/org/yarp/MarkNewlinesVisitor.java
@@ -0,0 +1,61 @@
+package org.yarp;
+
+final class MarkNewlinesVisitor extends AbstractNodeVisitor<Void> {
+
+    private final Nodes.Source source;
+    private boolean[] newlineMarked;
+
+    MarkNewlinesVisitor(Nodes.Source source, boolean[] newlineMarked) {
+        this.source = source;
+        this.newlineMarked = newlineMarked;
+    }
+
+    @Override
+    public Void visitBlockNode(Nodes.BlockNode node) {
+        boolean[] oldNewlineMarked = this.newlineMarked;
+        this.newlineMarked = new boolean[oldNewlineMarked.length];
+        try {
+            return super.visitBlockNode(node);
+        } finally {
+            this.newlineMarked = oldNewlineMarked;
+        }
+    }
+
+    @Override
+    public Void visitLambdaNode(Nodes.LambdaNode node) {
+        boolean[] oldNewlineMarked = this.newlineMarked;
+        this.newlineMarked = new boolean[oldNewlineMarked.length];
+        try {
+            return super.visitLambdaNode(node);
+        } finally {
+            this.newlineMarked = oldNewlineMarked;
+        }
+    }
+
+    @Override
+    public Void visitIfNode(Nodes.IfNode node) {
+        node.setNewLineFlag(this.source, this.newlineMarked);
+        return super.visitIfNode(node);
+    }
+
+    @Override
+    public Void visitUnlessNode(Nodes.UnlessNode node) {
+        node.setNewLineFlag(this.source, this.newlineMarked);
+        return super.visitUnlessNode(node);
+    }
+
+    @Override
+    public Void visitStatementsNode(Nodes.StatementsNode node) {
+        for (Nodes.Node child : node.body) {
+            child.setNewLineFlag(this.source, this.newlineMarked);
+        }
+        return super.visitStatementsNode(node);
+    }
+
+    @Override
+    protected Void defaultVisit(Nodes.Node node) {
+        node.visitChildNodes(this);
+        return null;
+    }
+
+}
diff --git a/lib/yarp.rb b/lib/yarp.rb
index 8730ef6f0f3..b7be9bc8dc3 100644
--- a/lib/yarp.rb
+++ b/lib/yarp.rb
@@ -141,6 +141,27 @@ def deconstruct_keys(keys)
     end
   end
 
+  # A class that knows how to walk down the tree. None of the individual visit
+  # methods are implemented on this visitor, so it forces the consumer to
+  # implement each one that they need. For a default implementation that
+  # continues walking the tree, see the Visitor class.
+  class BasicVisitor
+    def visit(node)
+      node&.accept(self)
+    end
+
+    def visit_all(nodes)
+      nodes.map { |node| visit(node) }
+    end
+
+    def visit_child_nodes(node)
+      visit_all(node.child_nodes)
+    end
+  end
+
+  class Visitor < BasicVisitor
+  end
+
   # This represents the result of a call to ::parse or ::parse_file. It contains
   # the AST, any comments that were encounters, and any errors that were
   # encountered.
@@ -166,6 +187,44 @@ def success?
     def failure?
       !success?
     end
+
+    class MarkNewlinesVisitor < YARP::Visitor
+      def initialize(newline_marked)
+        @newline_marked = newline_marked
+      end
+
+      def visit_block_node(node)
+        old_newline_marked = @newline_marked
+        @newline_marked = Array.new(old_newline_marked.size, false)
+        begin
+          super(node)
+        ensure
+          @newline_marked = old_newline_marked
+        end
+      end
+      alias_method :visit_lambda_node, :visit_block_node
+
+      def visit_if_node(node)
+        node.set_newline_flag(@newline_marked)
+        super(node)
+      end
+      alias_method :visit_unless_node, :visit_if_node
+
+      def visit_statements_node(node)
+        node.body.each do |child|
+          child.set_newline_flag(@newline_marked)
+        end
+        super(node)
+      end
+    end
+    private_constant :MarkNewlinesVisitor
+
+    def mark_newlines
+      newline_marked = Array.new(1 + @source.offsets.size, false)
+      visitor = MarkNewlinesVisitor.new(newline_marked)
+      value.accept(visitor)
+      value
+    end
   end
 
   # This represents a token from the Ruby source.
@@ -207,10 +266,23 @@ def ==(other)
   class Node
     attr_reader :location
 
+    def newline?
+      @newline ? true : false
+    end
+
+    def set_newline_flag(newline_marked)
+      line = location.start_line
+      unless newline_marked[line]
+        newline_marked[line] = true
+        @newline = true
+      end
+    end
+
     def pretty_print(q)
       q.group do
         q.text(self.class.name.split("::").last)
         location.pretty_print(q)
+        q.text("[Li:#{location.start_line}]") if newline?
         q.text("(")
         q.nest(2) do
           deconstructed = deconstruct_keys([])
@@ -225,6 +297,12 @@ def pretty_print(q)
     end
   end
 
+  class BeginNode < Node
+    def set_newline_flag(newline_marked)
+      # Never marking BeginNode, mark the StatementsNode statements instead
+    end
+  end
+
   # Load the serialized AST using the source as a reference into a tree.
   def self.load(source, serialized)
     Serialize.load(source, serialized)
diff --git a/tasks/check_manifest.rake b/tasks/check_manifest.rake
index b0d3c19f833..fe3fbe89553 100644
--- a/tasks/check_manifest.rake
+++ b/tasks/check_manifest.rake
@@ -11,6 +11,7 @@ task :check_manifest => [:templates, "configure"] do
     bin
     build
     fuzz
+    java
     pkg
     tasks
     templates
@@ -34,10 +35,6 @@ task :check_manifest => [:templates, "configure"] do
     config.status
     configure.ac
     include/yarp/config.h
-    java/org/yarp/AbstractNodeVisitor.java
-    java/org/yarp/Loader.java
-    java/org/yarp/Nodes.java
-    java/org/yarp/Parser.java
     lib/yarp.{jar,so,bundle}
   ]
 
diff --git a/templates/java/org/yarp/Loader.java.erb b/templates/java/org/yarp/Loader.java.erb
index 66fc9419602..e5270635dc1 100644
--- a/templates/java/org/yarp/Loader.java.erb
+++ b/templates/java/org/yarp/Loader.java.erb
@@ -43,12 +43,10 @@ public class Loader {
     private final ByteBuffer buffer;
     private ConstantPool constantPool;
     private final Nodes.Source source;
-    private final boolean[] newlineMarked;
 
     private Loader(byte[] serialized, Nodes.Source source) {
         this.buffer = ByteBuffer.wrap(serialized).order(ByteOrder.nativeOrder());
         this.source = source;
-        this.newlineMarked = new boolean[1 + source.getLineCount()];
     }
 
     private Nodes.Node load() {
@@ -78,7 +76,9 @@ public class Loader {
             throw new Error("Expected to consume all bytes while deserializing but there were " + left + " bytes left");
         }
 
-        node.setNewLineFlag(this.source, newlineMarked);
+        boolean[] newlineMarked = new boolean[1 + source.getLineCount()];
+        MarkNewlinesVisitor visitor = new MarkNewlinesVisitor(source, newlineMarked);
+        node.accept(visitor);
 
         return node;
     }
diff --git a/templates/java/org/yarp/Nodes.java.erb b/templates/java/org/yarp/Nodes.java.erb
index 0625a1a77ef..eab19431c34 100644
--- a/templates/java/org/yarp/Nodes.java.erb
+++ b/templates/java/org/yarp/Nodes.java.erb
@@ -123,15 +123,11 @@ public abstract class Nodes {
             this.length = length;
         }
 
-        public int length() {
-            return length;
-        }
-
-        public int endOffset() {
+        public final int endOffset() {
             return startOffset + length;
         }
 
-        public boolean hasNewLineFlag() {
+        public final boolean hasNewLineFlag() {
             return newLineFlag;
         }
 
@@ -145,6 +141,8 @@ public abstract class Nodes {
 
         public abstract <T> T accept(AbstractNodeVisitor<T> visitor);
 
+        public abstract <T> void visitChildNodes(AbstractNodeVisitor<T> visitor);
+
         public abstract Node[] childNodes();
 
         @Override
@@ -194,27 +192,29 @@ public abstract class Nodes {
         <%- end -%>
         }
 
-        <%- unless node.newline -%>
+        <%- if node.name == 'BeginNode' -%>
+        @Override
         public void setNewLineFlag(Source source, boolean[] newlineMarked) {
+            // Never marking BeginNode, mark the StatementsNode statements instead
+        }
+        <%- end -%>
+
+        public <T> void visitChildNodes(AbstractNodeVisitor<T> visitor) {
           <%- node.params.each do |param| -%>
             <%- case param -%>
             <%- when NodeListParam -%>
-            for (Nodes.Node node : this.<%= param.name %>) {
-                node.setNewLineFlag(source, newlineMarked);
+            for (Nodes.Node child : this.<%= param.name %>) {
+                child.accept(visitor);
             }
             <%- when NodeParam -%>
-            this.<%= param.name %>.setNewLineFlag(source, newlineMarked);
+            this.<%= param.name %>.accept(visitor);
             <%- when OptionalNodeParam -%>
             if (this.<%= param.name %> != null) {
-                this.<%= param.name %>.setNewLineFlag(source, newlineMarked);
+                this.<%= param.name %>.accept(visitor);
             }
-            <%- when -> _ { !param.java_type.include?("Node") } -%>
-            <%- else -%>
-            <%- raise "Param type #{param.class} not handled in #{__FILE__ }" -%>
             <%- end -%>
           <%- end -%>
         }
-        <%- end -%>
 
         public Node[] childNodes() {
           <%- if node.params.none?(NodeListParam) and node.params.none?(SingleNodeParam) -%>
diff --git a/templates/lib/yarp/node.rb.erb b/templates/lib/yarp/node.rb.erb
index a0d05bba63d..a235970ef33 100644
--- a/templates/lib/yarp/node.rb.erb
+++ b/templates/lib/yarp/node.rb.erb
@@ -69,24 +69,6 @@ module YARP
   end
 
   <%- end -%>
-  # A class that knows how to walk down the tree. None of the individual visit
-  # methods are implemented on this visitor, so it forces the consumer to
-  # implement each one that they need. For a default implementation that
-  # continues walking the tree, see the Visitor class.
-  class BasicVisitor
-    def visit(node)
-      node&.accept(self)
-    end
-
-    def visit_all(nodes)
-      nodes.map { |node| visit(node) }
-    end
-
-    def visit_child_nodes(node)
-      visit_all(node.child_nodes)
-    end
-  end
-
   class Visitor < BasicVisitor
     <%- nodes.each do |node| -%>
     # Visit a <%= node.name %> node
diff --git a/templates/template.rb b/templates/template.rb
index 1bca92c16eb..1e39efd2884 100755
--- a/templates/template.rb
+++ b/templates/template.rb
@@ -118,7 +118,7 @@ def java_type = "int"
 # YAML format. It contains information about the name of the node and the
 # various child nodes it contains.
 class NodeType
-  attr_reader :name, :type, :human, :params, :newline, :comment
+  attr_reader :name, :type, :human, :params, :comment
 
   def initialize(config)
     @name = config.fetch("name")
@@ -131,7 +131,6 @@ def initialize(config)
                    raise("Unknown param type: #{param["type"].inspect}")
       param_type.new(**param.transform_keys(&:to_sym))
     end
-    @newline = config.fetch("newline", true)
     @comment = config.fetch("comment")
   end
 
diff --git a/test/parse_test.rb b/test/parse_test.rb
index d0cdfe2c267..792cdbd6f68 100644
--- a/test/parse_test.rb
+++ b/test/parse_test.rb
@@ -2,6 +2,9 @@
 
 require "yarp_test_helper"
 
+# It is useful to have a diff even if the strings to compare are big
+Test::Unit::Assertions::AssertionMessage.max_diff_target_string_size = 5000
+
 class ParseTest < Test::Unit::TestCase
   # When we pretty-print the trees to compare against the snapshots, we want to
   # be certain that we print with the same external encoding. This is because
@@ -57,6 +60,71 @@ def test_parse_takes_file_path
     assert_equal filepath, find_source_file_node(parsed_result.value).filepath
   end
 
+  root = File.dirname(__dir__)
+  # We need valid Ruby files for this test and no "void expression"
+  # as that would count as a line in YARP but not with RubyVM::InstructionSequence
+  Dir["{lib,test}/**/*.rb", base: root].each do |relative|
+    filepath = File.join(root, relative)
+
+    define_method "test_newline_flags_#{relative}" do
+      # puts relative
+
+      source = File.read(filepath, binmode: true, external_encoding: Encoding::UTF_8)
+      verbose, $VERBOSE = $VERBOSE, nil
+      begin
+        insns = RubyVM::InstructionSequence.compile(source)
+      ensure
+        $VERBOSE = verbose
+      end
+
+      queue = [insns]
+      cruby_lines = []
+      while iseq = queue.shift
+        iseq.trace_points.each do |line, event|
+          cruby_lines << line if event == :line
+        end
+        iseq.each_child do |insn|
+          queue << insn unless insn.label.start_with?('ensure in ')
+        end
+      end
+      cruby_lines.sort!
+
+      result = YARP.parse(source, relative)
+      assert_empty result.errors
+
+      result.mark_newlines
+      ast = result.value
+      yarp_lines = []
+      visitor = Class.new(YARP::Visitor) do
+        define_method(:visit) do |node|
+          if node and node.newline?
+            yarp_lines << result.source.line(node.location.start_offset)
+          end
+          super(node)
+        end
+      end
+      ast.accept(visitor.new)
+
+      if relative == 'lib/yarp/serialize.rb'
+        # while (b = io.getbyte) >= 128 has 2 newline flags
+        cruby_lines.delete_at yarp_lines.index(62)
+      elsif relative == 'lib/yarp/lex_compat.rb'
+        # extra flag for: dedent_next =\n  ((token.event: due to bytecode order
+        yarp_lines.delete(514)
+        # different line for: token =\n  case event: due to bytecode order
+        yarp_lines.delete(571)
+        cruby_lines.delete(572)
+        # extra flag for: lex_state =\n  if RIPPER: due to bytecode order
+        yarp_lines.delete(604)
+        # extra flag for: (token[2].start_with?("\#$") || token[2].start_with?("\#@"))
+        # unclear when ParenthesesNode should allow a second flag on the same line or not
+        yarp_lines.delete(731)
+      end
+
+      assert_equal cruby_lines, yarp_lines
+    end
+  end
+
   base = File.join(__dir__, "fixtures")
   Dir["**/*.txt", base: base].each do |relative|
     next if known_failures.include?(relative)

From d9fa766377140555dce4a9007f57254d1ba26657 Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Mon, 24 Jul 2023 20:49:26 +0200
Subject: [PATCH 2/8] Mark predicate of if/unless and first part of
 Interpolated*Node

* Consistent with where CRuby places the flag after bytecode compilation.
---
 config.yml                             |  7 +++++++
 lib/yarp.rb                            |  6 ------
 templates/java/org/yarp/Nodes.java.erb | 18 ++++++++++++++++--
 templates/lib/yarp/node.rb.erb         | 17 +++++++++++++++++
 templates/template.rb                  |  3 ++-
 5 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/config.yml b/config.yml
index 1f8437bb6c1..8b4081b6639 100644
--- a/config.yml
+++ b/config.yml
@@ -485,6 +485,7 @@ nodes:
         kind: EnsureNode
       - name: end_keyword_loc
         type: location?
+    newline: false
     comment: |
       Represents a begin statement.
 
@@ -1178,6 +1179,7 @@ nodes:
         type: node?
       - name: end_keyword_loc
         type: location?
+    newline: predicate
     comment: |
       Represents the use of the `if` keyword, either in the block form or the modifier form.
 
@@ -1287,6 +1289,7 @@ nodes:
         type: location
       - name: flags
         type: uint32
+    newline: parts
     comment: |
       Represents a regular expression literal that contains interpolation.
 
@@ -1300,6 +1303,7 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location?
+    newline: parts
     comment: |
       Represents a string literal that contains interpolation.
 
@@ -1313,6 +1317,7 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location?
+    newline: parts
     comment: |
       Represents a symbol literal that contains interpolation.
 
@@ -1326,6 +1331,7 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location
+    newline: parts
     comment: |
       Represents an xstring literal that contains interpolation.
 
@@ -2010,6 +2016,7 @@ nodes:
         kind: ElseNode
       - name: end_keyword_loc
         type: location?
+    newline: predicate
     comment: |
       Represents the use of the `unless` keyword, either in the block form or the modifier form.
 
diff --git a/lib/yarp.rb b/lib/yarp.rb
index b7be9bc8dc3..9be869d5f84 100644
--- a/lib/yarp.rb
+++ b/lib/yarp.rb
@@ -297,12 +297,6 @@ def pretty_print(q)
     end
   end
 
-  class BeginNode < Node
-    def set_newline_flag(newline_marked)
-      # Never marking BeginNode, mark the StatementsNode statements instead
-    end
-  end
-
   # Load the serialized AST using the source as a reference into a tree.
   def self.load(source, serialized)
     Serialize.load(source, serialized)
diff --git a/templates/java/org/yarp/Nodes.java.erb b/templates/java/org/yarp/Nodes.java.erb
index eab19431c34..f8c6ce713c6 100644
--- a/templates/java/org/yarp/Nodes.java.erb
+++ b/templates/java/org/yarp/Nodes.java.erb
@@ -192,10 +192,24 @@ public abstract class Nodes {
         <%- end -%>
         }
 
-        <%- if node.name == 'BeginNode' -%>
+        <%- if node.newline == false -%>
         @Override
         public void setNewLineFlag(Source source, boolean[] newlineMarked) {
-            // Never marking BeginNode, mark the StatementsNode statements instead
+            // Never mark <%= node.name %> with a newline flag, mark children instead
+        }
+        <%- elsif node.newline.is_a?(String) -%>
+        @Override
+        public void setNewLineFlag(Source source, boolean[] newlineMarked) {
+          <%- param = node.params.find { |p| p.name == node.newline } or raise node.newline -%>
+          <%- case param -%>
+          <%- in SingleNodeParam -%>
+            <%= param.name %>.setNewLineFlag(source, newlineMarked);
+          <%- in NodeListParam -%>
+            Node first = <%= param.name %>.length > 0 ? <%= param.name %>[0] : null;
+            if (first != null) {
+                first.setNewLineFlag(source, newlineMarked);
+            }
+          <%- end -%>
         }
         <%- end -%>
 
diff --git a/templates/lib/yarp/node.rb.erb b/templates/lib/yarp/node.rb.erb
index a235970ef33..6b8c025be9c 100644
--- a/templates/lib/yarp/node.rb.erb
+++ b/templates/lib/yarp/node.rb.erb
@@ -19,6 +19,23 @@ module YARP
       visitor.visit_<%= node.human %>(self)
     end
 
+    <%- 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.name %>.set_newline_flag(newline_marked)
+    <%- in NodeListParam -%>
+      first = <%= param.name %>.first
+      first.set_newline_flag(newline_marked) if first
+    <%- end -%>
+    end
+    <%- end -%>
+
     # def child_nodes: () -> Array[nil | Node]
     def child_nodes
       [<%= node.params.filter_map { |param|
diff --git a/templates/template.rb b/templates/template.rb
index 1e39efd2884..1bca92c16eb 100755
--- a/templates/template.rb
+++ b/templates/template.rb
@@ -118,7 +118,7 @@ def java_type = "int"
 # YAML format. It contains information about the name of the node and the
 # various child nodes it contains.
 class NodeType
-  attr_reader :name, :type, :human, :params, :comment
+  attr_reader :name, :type, :human, :params, :newline, :comment
 
   def initialize(config)
     @name = config.fetch("name")
@@ -131,6 +131,7 @@ def initialize(config)
                    raise("Unknown param type: #{param["type"].inspect}")
       param_type.new(**param.transform_keys(&:to_sym))
     end
+    @newline = config.fetch("newline", true)
     @comment = config.fetch("comment")
   end
 

From 5b297fe91bf044a0b8b9f5607ce77a137d53203e Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Tue, 25 Jul 2023 11:58:51 +0200
Subject: [PATCH 3/8] Flag the body of modifier rescue not the
 RescueModifierNode itself

* Consistent with BeginNode (used for rescue & ensure).
* Consistent with where CRuby places the flag after bytecode compilation.
---
 config.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.yml b/config.yml
index 8b4081b6639..842dc64cfb5 100644
--- a/config.yml
+++ b/config.yml
@@ -1783,6 +1783,7 @@ nodes:
         type: location
       - name: rescue_expression
         type: node
+    newline: expression
     comment: |
       Represents an expression modified with a rescue.
 

From 56b9e5a8b27dfa96345c2470186e32b47744be3a Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Tue, 25 Jul 2023 12:12:13 +0200
Subject: [PATCH 4/8] Flag the predicate of while/until and not
 WhileNode/UntilNode themselves

* Consistent with IfNode and UnlessNode.
---
 config.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.yml b/config.yml
index 842dc64cfb5..96322516bdb 100644
--- a/config.yml
+++ b/config.yml
@@ -2035,6 +2035,7 @@ nodes:
       - name: statements
         type: node?
         kind: StatementsNode
+    newline: predicate
     comment: |
       Represents the use of the `until` keyword, either in the block form or the modifier form.
 
@@ -2066,6 +2067,7 @@ nodes:
       - name: statements
         type: node?
         kind: StatementsNode
+    newline: predicate
     comment: |
       Represents the use of the `while` keyword, either in the block form or the modifier form.
 

From 1b46ac0306b8b7f8b24853b5b455a22b3ee4f7fb Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Tue, 25 Jul 2023 12:23:30 +0200
Subject: [PATCH 5/8] Never mark a ParenthesesNode with the newline flag

* Consistent with where CRuby places the flag after bytecode compilation.
---
 config.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.yml b/config.yml
index 96322516bdb..c2504130c0e 100644
--- a/config.yml
+++ b/config.yml
@@ -1628,6 +1628,7 @@ nodes:
         type: location
       - name: closing_loc
         type: location
+    newline: false
     comment: |
       Represents a parenthesized expression
 

From aa43e8564017abfc59b58e62c0c8b20e217deacf Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Tue, 25 Jul 2023 12:40:54 +0200
Subject: [PATCH 6/8] Use `this.field` consistently in generated code for Java
 Nodes

---
 templates/java/org/yarp/Nodes.java.erb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/templates/java/org/yarp/Nodes.java.erb b/templates/java/org/yarp/Nodes.java.erb
index f8c6ce713c6..770e8b55c8c 100644
--- a/templates/java/org/yarp/Nodes.java.erb
+++ b/templates/java/org/yarp/Nodes.java.erb
@@ -203,9 +203,9 @@ public abstract class Nodes {
           <%- param = node.params.find { |p| p.name == node.newline } or raise node.newline -%>
           <%- case param -%>
           <%- in SingleNodeParam -%>
-            <%= param.name %>.setNewLineFlag(source, newlineMarked);
+            this.<%= param.name %>.setNewLineFlag(source, newlineMarked);
           <%- in NodeListParam -%>
-            Node first = <%= param.name %>.length > 0 ? <%= param.name %>[0] : null;
+            Node first = this.<%= param.name %>.length > 0 ? this.<%= param.name %>[0] : null;
             if (first != null) {
                 first.setNewLineFlag(source, newlineMarked);
             }
@@ -234,17 +234,17 @@ public abstract class Nodes {
           <%- if node.params.none?(NodeListParam) and node.params.none?(SingleNodeParam) -%>
             return EMPTY_ARRAY;
           <%- elsif node.params.one?(NodeListParam) and node.params.none?(SingleNodeParam) -%>
-            return <%= node.params.grep(NodeListParam).first.name %>;
+            return this.<%= node.params.grep(NodeListParam).first.name %>;
           <%- elsif node.params.none?(NodeListParam) -%>
-            return new Node[] { <%= node.params.grep(SingleNodeParam).map(&:name).join(', ') %> };
+            return new Node[] { <%= node.params.grep(SingleNodeParam).map { "this.#{_1.name}" }.join(', ') %> };
           <%- else -%>
             ArrayList<Node> childNodes = new ArrayList<>();
             <%- node.params.each do |param| -%>
               <%- case param -%>
               <%- when SingleNodeParam -%>
-            childNodes.add(<%= param.name %>);
+            childNodes.add(this.<%= param.name %>);
               <%- when NodeListParam -%>
-            childNodes.addAll(Arrays.asList(<%= param.name %>));
+            childNodes.addAll(Arrays.asList(this.<%= param.name %>));
               <%- end -%>
             <%- end -%>
             return childNodes.toArray(EMPTY_ARRAY);

From 57a8bb3bbc5bd8b865b910765e6cd2732467a0ab Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Tue, 25 Jul 2023 12:42:03 +0200
Subject: [PATCH 7/8] Remove extra length() method

---
 templates/java/org/yarp/Nodes.java.erb | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/templates/java/org/yarp/Nodes.java.erb b/templates/java/org/yarp/Nodes.java.erb
index 770e8b55c8c..37196af09a7 100644
--- a/templates/java/org/yarp/Nodes.java.erb
+++ b/templates/java/org/yarp/Nodes.java.erb
@@ -73,10 +73,6 @@ public abstract class Nodes {
             this.length = length;
         }
 
-        public int length() {
-            return length;
-        }
-
         public int endOffset() {
             return startOffset + length;
         }

From 2ae27833e8e105f48a7bfd7f972d50744366ee17 Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Tue, 25 Jul 2023 15:03:45 +0200
Subject: [PATCH 8/8] Add "keep in sync" comments for MarkNewlinesVisitor

---
 java/org/yarp/MarkNewlinesVisitor.java | 1 +
 lib/yarp.rb                            | 1 +
 2 files changed, 2 insertions(+)

diff --git a/java/org/yarp/MarkNewlinesVisitor.java b/java/org/yarp/MarkNewlinesVisitor.java
index dc9912f4e96..d1d38ea96dd 100644
--- a/java/org/yarp/MarkNewlinesVisitor.java
+++ b/java/org/yarp/MarkNewlinesVisitor.java
@@ -1,5 +1,6 @@
 package org.yarp;
 
+// Keep in sync with Ruby MarkNewlinesVisitor
 final class MarkNewlinesVisitor extends AbstractNodeVisitor<Void> {
 
     private final Nodes.Source source;
diff --git a/lib/yarp.rb b/lib/yarp.rb
index 9be869d5f84..c1c9dd70f8d 100644
--- a/lib/yarp.rb
+++ b/lib/yarp.rb
@@ -188,6 +188,7 @@ def failure?
       !success?
     end
 
+    # Keep in sync with Java MarkNewlinesVisitor
     class MarkNewlinesVisitor < YARP::Visitor
       def initialize(newline_marked)
         @newline_marked = newline_marked