TreeRewriter optimizations and default diagnostics engine

lib/parser/source/tree_rewriter.rb

@@ -83,14 +83,10 @@ module Source
     # @!attribute [r] source_buffer
     #  @return [Source::Buffer]
     #
-    # @!attribute [r] diagnostics
-    #  @return [Diagnostic::Engine]
-    #
     # @api public
     #
     class TreeRewriter
       attr_reader :source_buffer
-      attr_reader :diagnostics
 
       ##
       # @param [Source::Buffer] source_buffer
@@ -99,22 +95,18 @@ def initialize(source_buffer,
                      crossing_deletions: :accept,
                      different_replacements: :accept,
                      swallowed_insertions: :accept)
-        @diagnostics = Diagnostic::Engine.new
-        @diagnostics.consumer = -> diag { $stderr.puts diag.render }
-
+        @diagnostics = nil
         @source_buffer = source_buffer
         @in_transaction = false
 
-        @policy = {crossing_deletions: crossing_deletions,
-                   different_replacements: different_replacements,
-                   swallowed_insertions: swallowed_insertions}.freeze
-        check_policy_validity
+        @crossing_deletions = check_policy_value(crossing_deletions)
+        @different_replacements = check_policy_value(different_replacements)
+        @swallowed_insertions = check_policy_value(swallowed_insertions)
 
-        @enforcer = method(:enforce_policy)
         # We need a range that would be jugded as containing all other ranges,
         # including 0...0 and size...size:
         all_encompassing_range = @source_buffer.source_range.adjust(begin_pos: -1, end_pos: +1)
-        @action_root = TreeRewriter::Action.new(all_encompassing_range, @enforcer)
+        @action_root = TreeRewriter::Action.new(all_encompassing_range, self)
       end
 
       ##
@@ -326,6 +318,24 @@ def transaction
         @in_transaction = previous
       end
 
+      ##
+      # Provides access to a diagnostic engine.
+      # By default outputs diagnostic to $stderr
+      #
+      def self.default_diagnostics
+        @default_diagnostics ||= Diagnostic::Engine.new.tap do |engine|
+          engine.consumer = -> diag { $stderr.puts diag.render }
+        end
+      end
+
+      ##
+      # Provides access to a diagnostic engine.
+      # By default: self.class.default_diagnostics
+      #
+      def diagnostics
+        @diagnostics ||= self.class.default_diagnostics.dup
+      end
+
       def in_transaction?
         @in_transaction
       end
@@ -355,21 +365,32 @@ def insert_after_multi(range, text)
 
       extend Deprecation
 
+      ##
+      # @api private
+      # reserved for TreeAction
+      #
+      def enforce_policy(event)
+        return if policy(event) == :accept
+        return unless (values = yield)
+        trigger_policy(event, **values)
+      end
+
       protected
 
       attr_reader :action_root
 
       private
 
-      ACTIONS = %i[accept warn raise].freeze
-      def check_policy_validity
-        invalid = @policy.values - ACTIONS
-        raise ArgumentError, "Invalid policy: #{invalid.join(', ')}" unless invalid.empty?
+      ACTIONS = %i[accept warn raise].to_set.freeze
+      def check_policy_value(value)
+        raise ArgumentError, "Invalid policy value: #{value}" unless ACTIONS.include?(value)
+
+        value
       end
 
       def combine(range, attributes)
         range = check_range_validity(range)
-        action = TreeRewriter::Action.new(range, @enforcer, **attributes)
+        action = TreeRewriter::Action.new(range, self, **attributes)
         @action_root = @action_root.combine(action)
         self
       end
@@ -381,21 +402,28 @@ def check_range_validity(range)
         range
       end
 
-      def enforce_policy(event)
-        return if @policy[event] == :accept
-        return unless (values = yield)
-        trigger_policy(event, **values)
+      EVENT_TO_POLICY = {
+        crossing_deletions:     :@crossing_deletions,
+        different_replacements: :@different_replacements,
+        swallowed_insertions:   :@swallowed_insertions,
+      }.freeze
+
+      def policy(event)
+        return :raise if event == :crossing_insertions
+
+        instance_variable_get(EVENT_TO_POLICY.fetch(event))
       end
 
       POLICY_TO_LEVEL = {warn: :warning, raise: :error}.freeze
       def trigger_policy(event, range: raise, conflict: nil, **arguments)
-        action = @policy[event] || :raise
+        action = policy(event)
         diag = Parser::Diagnostic.new(POLICY_TO_LEVEL[action], event, arguments, range)
-        @diagnostics.process(diag)
+        engine = @diagnostics || self.class.default_diagnostics
+        engine.process(diag)
         if conflict
           range, *highlights = conflict
           diag = Parser::Diagnostic.new(POLICY_TO_LEVEL[action], :"#{event}_conflict", arguments, range, highlights)
-          @diagnostics.process(diag)
+          engine.process(diag)
         end
         raise Parser::ClobberingError, "Parser::Source::TreeRewriter detected clobbering" if action == :raise
       end

lib/parser/source/tree_rewriter/action.rb

@@ -206,7 +206,7 @@ def check_fusible(action, *fusible)
         return if fusible.empty?
         fusible.each do |child|
           kind = action.insertion? || child.insertion? ? :crossing_insertions : :crossing_deletions
-          @enforcer.call(kind) { {range: action.range, conflict: child.range} }
+          @enforcer.enforce_policy(kind) { {range: action.range, conflict: child.range} }
         end
         fusible
       end
@@ -222,15 +222,15 @@ def merge(action)
       end
 
       def call_enforcer_for_merge(action)
-        @enforcer.call(:different_replacements) do
+        @enforcer.enforce_policy(:different_replacements) do
           if @replacement && action.replacement && @replacement != action.replacement
             {range: @range, replacement: action.replacement, other_replacement: @replacement}
           end
         end
       end
 
       def swallow(children)
-        @enforcer.call(:swallowed_insertions) do
+        @enforcer.enforce_policy(:swallowed_insertions) do
           insertions = children.select(&:insertion?)
 
           {range: @range, conflict: insertions.map(&:range)} unless insertions.empty?

test/test_source_tree_rewriter.rb

@@ -298,6 +298,19 @@ def test_ordered_replacements
                   result.map {|r, s| [r.to_range, s]}
                 )
   end
+
+  def test_default_diagnostics
+    default = Parser::Source::TreeRewriter.default_diagnostics
+    before = default.consumer
+    diagnostics = []
+    default.consumer = -> diag { diagnostics << diag.render }
+    rewriter = Parser::Source::TreeRewriter.new(@buf, swallowed_insertions: :warn)
+    rewriter.replace(@ll, 'xx')
+    rewriter.remove(@hello)
+    assert_equal(2, diagnostics.size)
+  ensure
+    default.consumer = before
+  end
 end
 
 class TestSourceTreeRewriterImport < Minitest::Test