+ Source::TreeRewriter: Add #merge, #merge! and #empty? (#674)

lib/parser/source/tree_rewriter.rb

@@ -117,6 +117,43 @@ def initialize(source_buffer,
         @action_root = TreeRewriter::Action.new(all_encompassing_range, @enforcer)
       end
 
+      ##
+      # Returns true iff no (non trivial) update has been recorded
+      #
+      # @return [Boolean]
+      #
+      def empty?
+        @action_root.empty?
+      end
+
+      ##
+      # Merges the updates of argument with the receiver.
+      # Policies of the receiver are used.
+      #
+      # @param [Rewriter] with
+      # @return [Rewriter] self
+      # @raise [ClobberingError] when clobbering is detected
+      #
+      def merge!(with)
+        raise 'TreeRewriter are not for the same source_buffer' unless
+          source_buffer == with.source_buffer
+
+        @action_root = @action_root.combine(with.action_root)
+        self
+      end
+
+      ##
+      # Returns a new rewriter that consists of the updates of the received
+      # and the given argument. Policies of the receiver are used.
+      #
+      # @param [Rewriter] with
+      # @return [Rewriter] merge of receiver and argument
+      # @raise [ClobberingError] when clobbering is detected
+      #
+      def merge(with)
+        dup.merge!(with)
+      end
+
       ##
       # Replaces the code of the source range `range` with `content`.
       #
@@ -255,6 +292,10 @@ def insert_after_multi(range, text)
 
       extend Deprecation
 
+      protected
+
+      attr_reader :action_root
+
       private
 
       ACTIONS = %i[accept warn raise].freeze

lib/parser/source/tree_rewriter/action.rb

@@ -25,12 +25,18 @@ def initialize(range, enforcer,
         freeze
       end
 
-      # Assumes action.children.empty?
       def combine(action)
-        return self unless action.insertion? || action.replacement # Ignore empty action
+        return self if action.empty? # Ignore empty action
         do_combine(action)
       end
 
+      def empty?
+        @insert_before.empty? &&
+          @insert_after.empty? &&
+          @children.empty? &&
+          (@replacement == nil || (@replacement.empty? && @range.empty?))
+      end
+
       def ordered_replacements
         reps = []
         reps << [@range.begin, @insert_before] unless @insert_before.empty?
@@ -46,9 +52,11 @@ def insertion?
 
       protected
 
-      def with(range: @range, children: @children, insert_before: @insert_before, replacement: @replacement, insert_after: @insert_after)
+      attr_reader :children
+
+      def with(range: @range, enforcer: @enforcer, children: @children, insert_before: @insert_before, replacement: @replacement, insert_after: @insert_after)
         children = swallow(children) if replacement
-        self.class.new(range, @enforcer, children: children, insert_before: insert_before, replacement: replacement, insert_after: insert_after)
+        self.class.new(range, enforcer, children: children, insert_before: insert_before, replacement: replacement, insert_after: insert_after)
       end
 
       # Assumes range.contains?(action.range) && action.children.empty?
@@ -69,14 +77,22 @@ def place_in_hierarchy(action)
           extra_sibbling = if family[:parent]  # action should be a descendant of one of the children
             family[:parent][0].do_combine(action)
           elsif family[:child]                 # or it should become the parent of some of the children,
-            action.with(children: family[:child])
+            action.with(children: family[:child], enforcer: @enforcer)
+              .combine_children(action.children)
           else                                 # or else it should become an additional child
             action
           end
           with(children: [*family[:sibbling], extra_sibbling])
         end
       end
 
+      # Assumes more_children all contained within @range
+      def combine_children(more_children)
+        more_children.inject(self) do |parent, new_child|
+          parent.place_in_hierarchy(new_child)
+        end
+      end
+
       def fuse_deletions(action, fusible, other_sibblings)
         without_fusible = with(children: other_sibblings)
         fused_range = [action, *fusible].map(&:range).inject(:join)
@@ -109,7 +125,7 @@ def merge(action)
           insert_before: "#{action.insert_before}#{insert_before}",
           replacement: action.replacement || @replacement,
           insert_after: "#{insert_after}#{action.insert_after}",
-        )
+        ).combine_children(action.children)
       end
 
       def call_enforcer_for_merge(action)

test/test_source_tree_rewriter.rb

@@ -8,20 +8,22 @@ def setup
     @buf.source = 'puts(:hello, :world)'
 
     @hello = range(5, 6)
+    @ll = range(7, 2)
     @comma_space = range(11,2)
     @world = range(13,6)
+    @whole = range(0, @buf.source.length)
   end
 
   def range(from, len)
     Parser::Source::Range.new(@buf, from, from + len)
   end
 
   # Returns either:
-  #  - String (Normal operation)
+  #  - yield rewriter
   #  - [diagnostic, ...] (Diagnostics)
   #  - Parser::ClobberingError
   #
-  def apply(actions, **policy)
+  def build(actions, **policy)
     diagnostics = []
     diags = -> { diagnostics.flatten.map(&:strip).join("\n") }
     rewriter = Parser::Source::TreeRewriter.new(@buf, **policy)
@@ -30,14 +32,23 @@ def apply(actions, **policy)
       rewriter.public_send(action, range, *args)
     end
     if diagnostics.empty?
-      rewriter.process
+      yield rewriter
     else
       diags.call
     end
   rescue ::Parser::ClobberingError => e
     [::Parser::ClobberingError, diags.call]
   end
 
+  # Returns either:
+  #  - String (Normal operation)
+  #  - [diagnostic, ...] (Diagnostics)
+  #  - Parser::ClobberingError
+  #
+  def apply(actions, **policy)
+    build(actions, **policy) { |rewriter| rewriter.process }
+  end
+
   # Expects ordered actions to be grouped together
   def check_actions(expected, grouped_actions, **policy)
     grouped_actions.permutation do |sequence|
@@ -170,4 +181,73 @@ def test_out_of_range_ranges
     rewriter = Parser::Source::TreeRewriter.new(@buf)
     assert_raises(IndexError) { rewriter.insert_before(range(0, 100), 'hola') }
   end
+
+  def test_empty
+    rewriter = Parser::Source::TreeRewriter.new(@buf)
+    assert_equal true, rewriter.empty?
+
+    # This is a trivial wrap
+    rewriter.wrap(range(2,3), '', '')
+    assert_equal true, rewriter.empty?
+
+    # This is a trivial deletion
+    rewriter.remove(range(2,0))
+    assert_equal true, rewriter.empty?
+
+    rewriter.remove(range(2,3))
+    assert_equal false, rewriter.empty?
+  end
+
+  # splits array into two groups, yield all such possible pairs of groups
+  # each_split([1, 2, 3, 4]) yields [1, 2], [3, 4];
+  #                            then [1, 3], [2, 4]
+  #                                 ...
+  #                     and finally [3, 4], [1, 2]
+  def each_split(array)
+    n = array.size
+    first_split_size = n.div(2)
+    splitting = (0...n).to_set
+    splitting.to_a.combination(first_split_size) do |indices|
+      yield array.values_at(*indices),
+            array.values_at(*(splitting - indices))
+    end
+  end
+
+  # Checks that `actions+extra` give the same result when
+  # made in order or from subgroups that are later merged.
+  # The `extra` actions are always added at the end of the second group.
+  #
+  def check_all_merge_possibilities(actions, extra, **policy)
+    expected = apply(actions + extra, **policy)
+
+    each_split(actions) do |actions_1, actions_2|
+      build(actions_1, **policy) do |rewriter_1|
+        build(actions_2 + extra, **policy) do |rewriter_2|
+          result = rewriter_1.merge(rewriter_2).process
+          assert_equal(expected, result,
+            "Group 1: #{actions_1.inspect}\n\n" +
+            "Group 2: #{(actions_2 + extra).inspect}"
+          )
+        end
+      end
+    end
+  end
+
+  def test_merge
+    check_all_merge_possibilities([
+      [:wrap, @whole, '<', '>'],
+      [:replace, @comma_space, ' => '],
+      [:wrap, @hello, '!', '!'],
+      # Following two wraps must have same value as they
+      # will be applied in different orders...
+      [:wrap, @hello.join(@world), '{', '}'],
+      [:wrap, @hello.join(@world), '{', '}'],
+      [:remove, @ll],
+      [:replace, @world, ':everybody'],
+      [:wrap, @world, '[', ']']
+    ],
+    [ # ... but this one is always going to be applied last (extra)
+      [:wrap, @hello.join(@world), '@', '@'],
+    ])
+  end
 end