Merge pull request #1156 from eregon/more-newline-flags-visitor

Set newline flag using a visitor for both Ruby and Java nodes

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

config.yml

@@ -485,6 +485,7 @@ nodes:
         kind: EnsureNode
       - name: end_keyword_loc
         type: location?
+    newline: false
     comment: |
       Represents a begin statement.
 
@@ -940,7 +941,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 +1179,7 @@ nodes:
         type: node?
       - name: end_keyword_loc
         type: location?
-    newline: false
+    newline: predicate
     comment: |
       Represents the use of the `if` keyword, either in the block form or the modifier form.
 
@@ -1289,7 +1289,7 @@ nodes:
         type: location
       - name: flags
         type: uint32
-    newline: false
+    newline: parts
     comment: |
       Represents a regular expression literal that contains interpolation.
 
@@ -1303,7 +1303,7 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location?
-    newline: false
+    newline: parts
     comment: |
       Represents a string literal that contains interpolation.
 
@@ -1317,7 +1317,7 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location?
-    newline: false
+    newline: parts
     comment: |
       Represents a symbol literal that contains interpolation.
 
@@ -1331,7 +1331,7 @@ nodes:
         type: node[]
       - name: closing_loc
         type: location
-    newline: false
+    newline: parts
     comment: |
       Represents an xstring literal that contains interpolation.
 
@@ -1701,7 +1701,6 @@ nodes:
       - name: statements
         type: node
         kind: StatementsNode
-    newline: false
     comment: The top level node of any parse tree.
   - name: RangeNode
     child_nodes:
@@ -1785,6 +1784,7 @@ nodes:
         type: location
       - name: rescue_expression
         type: node
+    newline: expression
     comment: |
       Represents an expression modified with a rescue.
 
@@ -1909,7 +1909,6 @@ nodes:
     child_nodes:
       - name: body
         type: node[]
-    newline: false
     comment: |
       Represents a set of statements contained within some scope.
 
@@ -2019,7 +2018,7 @@ nodes:
         kind: ElseNode
       - name: end_keyword_loc
         type: location?
-    newline: false
+    newline: predicate
     comment: |
       Represents the use of the `unless` keyword, either in the block form or the modifier form.
 
@@ -2037,6 +2036,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.
 
@@ -2068,6 +2068,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.
 

java/org/yarp/MarkNewlinesVisitor.java

@@ -0,0 +1,62 @@
+package org.yarp;
+
+// Keep in sync with Ruby MarkNewlinesVisitor
+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;
+    }
+
+}

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,45 @@ def success?
     def failure?
       !success?
     end
+
+    # Keep in sync with Java MarkNewlinesVisitor
+    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 +267,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([])

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}
   ]
 

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;
     }