+ Add Source::TreeRewriter::Enforcer as base class to handle rewriting conflicts

Author: marcandre

lib/parser.rb

@@ -38,6 +38,7 @@ module Source
     require 'parser/source/rewriter/action'
     require 'parser/source/tree_rewriter'
     require 'parser/source/tree_rewriter/action'
+    require 'parser/source/tree_rewriter/enforcer'
 
     require 'parser/source/map'
     require 'parser/source/map/operator'

lib/parser/source/tree_rewriter.rb

@@ -90,27 +90,19 @@ module Source
     #
     class TreeRewriter
       attr_reader :source_buffer
-      attr_reader :diagnostics
 
+      attr_reader :enforcer
       ##
       # @param [Source::Buffer] source_buffer
       #
       def initialize(source_buffer,
-                     crossing_deletions: :accept,
-                     different_replacements: :accept,
-                     swallowed_insertions: :accept)
-        @diagnostics = Diagnostic::Engine.new
-        @diagnostics.consumer = -> diag { $stderr.puts diag.render }
+                     enforcer: nil,
+                     **policy)
+        @enforcer = enforcer || Enforcer::WithPolicy.new(**policy)
 
         @source_buffer = source_buffer
         @in_transaction = false
 
-        @policy = {crossing_deletions: crossing_deletions,
-                   different_replacements: different_replacements,
-                   swallowed_insertions: swallowed_insertions}.freeze
-        check_policy_validity
-
-        @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)
@@ -330,6 +322,13 @@ def in_transaction?
         @in_transaction
       end
 
+      ##
+      # @deprecated Provide different enforcer
+      #
+      def diagnostics
+        enforcer.diagnostics
+      end
+
       ##
       # @api private
       # @deprecated Use insert_after or wrap
@@ -361,12 +360,6 @@ def insert_after_multi(range, text)
 
       private
 
-      ACTIONS = %i[accept warn raise].freeze
-      def check_policy_validity
-        invalid = @policy.values - ACTIONS
-        raise ArgumentError, "Invalid policy: #{invalid.join(', ')}" unless invalid.empty?
-      end
-
       def combine(range, attributes)
         range = check_range_validity(range)
         action = TreeRewriter::Action.new(range, @enforcer, **attributes)
@@ -380,25 +373,6 @@ def check_range_validity(range)
         end
         range
       end
-
-      def enforce_policy(event)
-        return if @policy[event] == :accept
-        return unless (values = yield)
-        trigger_policy(event, **values)
-      end
-
-      POLICY_TO_LEVEL = {warn: :warning, raise: :error}.freeze
-      def trigger_policy(event, range: raise, conflict: nil, **arguments)
-        action = @policy[event] || :raise
-        diag = Parser::Diagnostic.new(POLICY_TO_LEVEL[action], event, arguments, range)
-        @diagnostics.process(diag)
-        if conflict
-          range, *highlights = conflict
-          diag = Parser::Diagnostic.new(POLICY_TO_LEVEL[action], :"#{event}_conflict", arguments, range, highlights)
-          @diagnostics.process(diag)
-        end
-        raise Parser::ClobberingError, "Parser::Source::TreeRewriter detected clobbering" if action == :raise
-      end
     end
   end
 end

lib/parser/source/tree_rewriter/action.rb

@@ -205,15 +205,19 @@ def check_fusible(action, *fusible)
         fusible.compact!
         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} }
+          if action.insertion? || child.insertion?
+            @enforcer.on_crossing_insertions(action.range, child.range) unless @enforcer.ignore?(:crossing_insertions)
+          else
+            @enforcer.on_crossing_deletions(action.range, child.range) unless @enforcer.ignore?(:crossing_deletions)
+          end
         end
         fusible
       end
 
       # Assumes action.range == range && action.children.empty?
       def merge(action)
         call_enforcer_for_merge(action)
+
         with(
           insert_before: "#{action.insert_before}#{insert_before}",
           replacement: action.replacement || @replacement,
@@ -222,19 +226,20 @@ def merge(action)
       end
 
       def call_enforcer_for_merge(action)
-        @enforcer.call(:different_replacements) do
-          if @replacement && action.replacement && @replacement != action.replacement
-            {range: @range, replacement: action.replacement, other_replacement: @replacement}
-          end
+        if @replacement && action.replacement && !@enforcer.ignore?(:different_replacements) &&
+           @replacement != action.replacement
+          @enforcer.on_different_replacements(@range, action.replacement, @replacement)
         end
       end
 
       def swallow(children)
-        @enforcer.call(:swallowed_insertions) do
+        unless @enforcer.ignore?(:swallowed_insertions)
           insertions = children.select(&:insertion?)
-
-          {range: @range, conflict: insertions.map(&:range)} unless insertions.empty?
+          unless insertions.empty?
+            @enforcer.on_swallowed_insertions @range, insertions.map(&:range)
+          end
         end
+
         []
       end
     end

lib/parser/source/tree_rewriter/enforcer.rb

@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+module Parser
+  module Source
+    class TreeRewriter
+      ##
+      # Base class for objects responsible to handle rewriting conflicts
+      # At a minimum, `ignore?` must be defined.
+      # Other methods may be overwritten.
+      #
+      class Enforcer
+        #
+        # @param [Symbol] one of :crossing_deletions, :different_replacements, :swallowed_insertions,
+        # For future-proofing, `ignore?` must accept other events and return `true`.
+        def ignore?(event)
+          raise NotImplementedError
+        end
+
+        def on_crossing_insertions(range, conflict_range)
+          on(:crossing_insertions, range, conflict: conflict_range)
+        end
+
+        def on_crossing_deletions(range, conflict_range)
+          on(:crossing_deletions, range, conflict: conflict_range)
+        end
+
+        def on_swallowed_insertions(range, conflict_range)
+          on(:swallowed_insertions, range, conflict: conflict_range)
+        end
+
+        def on_different_replacements(range, replacement, other_replacement)
+          on(:different_replacements, range, replacement: replacement, other_replacement: other_replacement)
+        end
+
+        protected
+
+        def on(_event, _range, **_args)
+          reject
+        end
+
+        def reject
+          raise Parser::ClobberingError, "Parser::Source::TreeRewriter detected clobbering"
+        end
+
+        class WithPolicy < Enforcer
+          ACTIONS = %i[accept warn raise].freeze
+
+          def initialize(
+            crossing_deletions: :accept,
+            different_replacements: :accept,
+            swallowed_insertions: :accept
+          )
+            @crossing_deletions     = validate_policy(crossing_deletions    )
+            @different_replacements = validate_policy(different_replacements)
+            @swallowed_insertions   = validate_policy(swallowed_insertions  )
+          end
+
+          def ignore?(event)
+            policy(event) == :accept
+          end
+
+          def diagnostics
+            @diagnostics ||= Diagnostic::Engine.new(-> diag { $stderr.puts diag.render })
+          end
+
+          protected
+
+          def validate_policy(action)
+            raise ArgumentError, "Invalid policy value: #{action}" unless ACTIONS.include?(action)
+
+            action
+          end
+
+          def policy(event)
+            case event
+            when :crossing_insertions    then :raise
+            when :crossing_deletions     then @crossing_deletions
+            when :different_replacements then @different_replacements
+            when :swallowed_insertions   then @swallowed_insertions
+            else :accept # Example of future proofing
+            end
+          end
+
+          POLICY_TO_LEVEL = {warn: :warning, raise: :error}.freeze
+          def on(event, range, conflict: nil, **arguments)
+            action = policy(event)
+            severity = POLICY_TO_LEVEL.fetch(action)
+            diag = Parser::Diagnostic.new(severity, event, arguments, range)
+            diagnostics.process(diag)
+            if conflict
+              range, *highlights = conflict
+              diag = Parser::Diagnostic.new(severity, :"#{event}_conflict", arguments, range, highlights)
+              diagnostics.process(diag)
+            end
+            raise Parser::ClobberingError, "Parser::Source::TreeRewriter detected clobbering" if action == :raise
+          end
+        end
+
+        DEFAULT = WithPolicy.new.freeze
+      end
+    end
+  end
+end