Add ability to diff complex arrays

By "complex" data structures we mean arrays within arrays and hashes
within arrays (and everything under the sun inside of them).

In order to diff a complex array, we need to have the ability to show a
diff that reaches into inner arrays or hashes and shows the diff for
them too. That is, we need to compare each item in the array, and if we
have two elements which are different, and they are themselves arrays or
hashes, we need to descend into them and diff each of their elements
as well.

We can already diff two complex hashes in this manner because we have
custom diffing logic for hashes, where an added key is treated as an
insert, a missing key is treated as a delete, and a changed key is
treated as a delete + an insert. To show a diff for two hashes, we
merely go back over the diff information and look for a delete followed
by an insert, treating it as a change, and if the changed value is an
array or hash we know to descend.

However, this approach does not work for arrays because we are using the
patience algorithm to diff their contents. The patience algorithm is
commonly used to diff two text files on a line level, and it is helpful
because it groups together successful additions and deletions so as to
create a cleaner diff. This grouping algorithm is completely useless for
arrays, though, because it does not allow us to detect changed values
effectively.

Another solution which *does* allow us to detect changes, however, is
the diff-lcs gem, and that's exactly what this commit introduces.

Author: mcmire

Gemfile.lock

@@ -31,6 +31,7 @@ PATH
   remote: .
   specs:
     super_diff (0.0.1)
+      diff-lcs
       patience_diff
 
 GEM
@@ -80,4 +81,4 @@ DEPENDENCIES
   super_diff!
 
 BUNDLED WITH
-   1.16.2
+   1.16.4

lib/super_diff.rb

@@ -1,4 +1,5 @@
 require "patience_diff"
+require "diff-lcs"
 require "pry-byebug"
 
 require_relative "super_diff/csi"
@@ -15,6 +16,9 @@
 require_relative "super_diff/equality_matchers"
 require_relative "super_diff/equality_matcher"
 
+require_relative "super_diff/operations/unary_operation"
+require_relative "super_diff/operations/binary_operation"
+
 require_relative "super_diff/operation_sequences/base"
 require_relative "super_diff/operation_sequences/array"
 require_relative "super_diff/operation_sequences/hash"
@@ -23,6 +27,7 @@
 require_relative "super_diff/operational_sequencers/base"
 require_relative "super_diff/operational_sequencers/array"
 require_relative "super_diff/operational_sequencers/hash"
+require_relative "super_diff/operational_sequencers/multi_line_string"
 require_relative "super_diff/operational_sequencers/object"
 require_relative "super_diff/operational_sequencers"
 require_relative "super_diff/operational_sequencer"

lib/super_diff/diff_formatters/array.rb

@@ -10,7 +10,7 @@ def call
           open_token: "[",
           close_token: "]",
           collection_prefix: collection_prefix,
-          build_item_prefix: -> (op) { "" },
+          build_item_prefix: -> (operation) { "" },
           operations: operations,
           indent_level: indent_level,
           add_comma: add_comma?

lib/super_diff/diff_formatters/collection.rb

@@ -44,26 +44,23 @@ def lines
       end
 
       def contents
-        operations.map do |op|
-          if op.name == :change
-            max_size = [op.left_collection.size, op.right_collection.size].max
-            add_comma = op.index < max_size - 1
-            op.child_operations.to_diff(
+        operations.map do |operation|
+          if operation.name == :change
+            operation.child_operations.to_diff(
               indent_level: indent_level + 1,
-              collection_prefix: build_item_prefix.call(op),
-              add_comma: add_comma
+              collection_prefix: build_item_prefix.call(operation),
+              add_comma: operation.should_add_comma_after_displaying?
             )
           else
-            collection = op.collection
-            icon = ICONS.fetch(op.name, " ")
-            style_name = STYLES.fetch(op.name, :normal)
+            icon = ICONS.fetch(operation.name, " ")
+            style_name = STYLES.fetch(operation.name, :normal)
             chunk = build_chunk(
-              Helpers.inspect_object(op.value, single_line: false),
-              prefix: build_item_prefix.call(op),
+              Helpers.inspect_object(operation.value, single_line: false),
+              prefix: build_item_prefix.call(operation),
               icon: icon
             )
 
-            if op.index < collection.size - 1
+            if operation.should_add_comma_after_displaying?
               chunk << ","
             end
 

lib/super_diff/diff_formatters/hash.rb

@@ -10,11 +10,18 @@ def call
           open_token: "{",
           close_token: "}",
           collection_prefix: collection_prefix,
-          build_item_prefix: -> (op) {
-            if op.key.is_a?(Symbol)
-              "#{op.key}: "
+          build_item_prefix: -> (operation) {
+            key =
+              if operation.respond_to?(:left_key)
+                operation.left_key
+              else
+                operation.key
+              end
+
+            if key.is_a?(Symbol)
+              "#{key}: "
             else
-              "#{op.key.inspect} => "
+              "#{key.inspect} => "
             end
           },
           operations: operations,

lib/super_diff/diff_formatters/multi_line_string.rb

@@ -12,17 +12,17 @@ def call
       private
 
       def lines
-        operations.inject([]) do |array, op|
-          case op.name
+        operations.inject([]) do |array, operation|
+          case operation.name
           when :change
-            array << Helpers.style(:deleted, "- #{op.left_value}")
-            array << Helpers.style(:inserted, "+ #{op.right_value}")
+            array << Helpers.style(:deleted, "- #{operation.left_value}")
+            array << Helpers.style(:inserted, "+ #{operation.right_value}")
           when :delete
-            array << Helpers.style(:deleted, "- #{op.value}")
+            array << Helpers.style(:deleted, "- #{operation.value}")
           when :insert
-            array << Helpers.style(:inserted, "+ #{op.value}")
+            array << Helpers.style(:inserted, "+ #{operation.value}")
           else
-            array << Helpers.style(:normal, "  #{op.value}")
+            array << Helpers.style(:normal, "  #{operation.value}")
           end
         end
       end

lib/super_diff/diff_formatters/object.rb

@@ -10,7 +10,7 @@ def call
           open_token: "#<#{value_class} {",
           close_token: "}>",
           collection_prefix: collection_prefix,
-          build_item_prefix: -> (op) { "#{op.key}: " },
+          build_item_prefix: -> (operation) { "#{operation.key}: " },
           operations: operations,
           indent_level: indent_level,
           add_comma: add_comma?

lib/super_diff/differs/string.rb

@@ -39,7 +39,7 @@ def styled_lines_for(icon, style_name, collection)
       end
 
       def unstyled_line_for(icon, collection)
-        value = collection[op.index]
+        value = collection[operation.index]
         line = "#{icon} #{indentation}#{value.inspect}"
 
         if index_in_collection < collection.length - 1

lib/super_diff/equality_matchers/multi_line_string.rb

@@ -2,20 +2,7 @@ module SuperDiff
   module EqualityMatchers
     class MultiLineString < Base
       def self.applies_to?(value)
-        value.class == ::String && value.include?("\n")
-      end
-
-      def initialize(
-        expected:,
-        actual:,
-        extra_operational_sequencer_classes:,
-        extra_diff_formatter_classes:
-      )
-        @expected = split_into_lines(expected)
-        @actual = split_into_lines(actual)
-
-        @original_expected = @expected.join
-        @original_actual = @actual.join
+        value.is_a?(::String) && value.include?("\n")
       end
 
       def fail
@@ -25,13 +12,13 @@ def fail
           #{
             Helpers.style(
               :deleted,
-              "Expected: #{Helpers.inspect_object(original_expected)}"
+              "Expected: #{Helpers.inspect_object(expected)}"
             )
           }
           #{
             Helpers.style(
               :inserted,
-              "  Actual: #{Helpers.inspect_object(original_actual)}"
+              "  Actual: #{Helpers.inspect_object(actual)}"
             )
           }
 
@@ -43,18 +30,15 @@ def fail
 
       private
 
-      attr_reader :original_expected, :original_actual
-
-      def split_into_lines(str)
-        str.split(/(\n)/).map { |v| v.tr("\n", "⏎") }.each_slice(2).map(&:join)
-      end
-
       def diff
         DiffFormatters::MultiLineString.call(operations, indent_level: 0)
       end
 
       def operations
-        OperationalSequencers::Array.call(expected: expected, actual: actual)
+        OperationalSequencers::MultiLineString.call(
+          expected: expected,
+          actual: actual
+        )
       end
     end
   end

lib/super_diff/operational_sequencers/array.rb

@@ -8,57 +8,119 @@ def self.applies_to?(value)
       def initialize(*args)
         super(*args)
 
-        @sequence_matcher = PatienceDiff::SequenceMatcher.new
+        @lcs_callbacks = LcsCallbacks.new(
+          expected: expected,
+          actual: actual,
+          extra_operational_sequencer_classes: extra_operational_sequencer_classes,
+          extra_diff_formatter_classes: extra_diff_formatter_classes
+        )
       end
 
-      protected
+      def call
+        Diff::LCS.traverse_balanced(expected, actual, lcs_callbacks)
+        lcs_callbacks.operations
+      end
+
+      private
+
+      attr_reader :lcs_callbacks
+
+      class LcsCallbacks
+        attr_reader :operations
 
-      def unary_operations
-        opcodes.flat_map do |code, a_start, a_end, b_start, b_end|
-          if code == :delete
-            (a_start..a_end).map do |index|
-              UnaryOperation.new(
-                name: :delete,
-                collection: expected,
-                key: index,
-                index: index,
-                value: expected[index]
-              )
-            end
-          elsif code == :insert
-            (b_start..b_end).map do |index|
-              UnaryOperation.new(
-                name: :insert,
-                collection: actual,
-                key: index,
-                index: index,
-                value: actual[index]
-              )
-            end
+        def initialize(
+          expected:,
+          actual:,
+          extra_operational_sequencer_classes:,
+          extra_diff_formatter_classes:
+        )
+          @expected = expected
+          @actual = actual
+          @operations = OperationSequences::Array.new([])
+          @extra_operational_sequencer_classes =
+            extra_operational_sequencer_classes
+          @extra_diff_formatter_classes = extra_diff_formatter_classes
+        end
+
+        def match(event)
+          operations << ::SuperDiff::Operations::UnaryOperation.new(
+            name: :noop,
+            collection: actual,
+            key: event.new_position,
+            value: event.new_element,
+            index: event.new_position
+          )
+        end
+
+        def discard_a(event)
+          operations << ::SuperDiff::Operations::UnaryOperation.new(
+            name: :delete,
+            collection: expected,
+            key: event.old_position,
+            value: event.old_element,
+            index: event.old_position
+          )
+        end
+
+        def discard_b(event)
+          operations << ::SuperDiff::Operations::UnaryOperation.new(
+            name: :insert,
+            collection: actual,
+            key: event.new_position,
+            value: event.new_element,
+            index: event.new_position
+          )
+        end
+
+        def change(event)
+          child_operations = sequence(event.old_element, event.new_element)
+
+          if child_operations
+            operations << ::SuperDiff::Operations::BinaryOperation.new(
+              name: :change,
+              left_collection: expected,
+              right_collection: actual,
+              left_key: event.old_position,
+              right_key: event.new_position,
+              left_value: event.old_element,
+              right_value: event.new_element,
+              left_index: event.old_position,
+              right_index: event.new_position,
+              child_operations: child_operations
+            )
           else
-            (b_start..b_end).map do |index|
-              UnaryOperation.new(
-                name: :noop,
-                collection: actual,
-                key: index,
-                index: index,
-                value: actual[index]
-              )
-            end
+            operations << Operations::UnaryOperation.new(
+              name: :delete,
+              collection: expected,
+              key: event.old_position,
+              value: event.old_element,
+              index: event.old_position
+            )
+            operations << Operations::UnaryOperation.new(
+              name: :insert,
+              collection: actual,
+              key: event.new_position,
+              value: event.new_element,
+              index: event.new_position
+            )
           end
         end
-      end
 
-      def operation_sequence_class
-        OperationSequences::Array
-      end
-
-      private
+        private
 
-      attr_reader :sequence_matcher
+        attr_reader :expected, :actual, :extra_operational_sequencer_classes,
+          :extra_diff_formatter_classes
 
-      def opcodes
-        sequence_matcher.diff_opcodes(expected, actual)
+        def sequence(expected, actual)
+          OperationalSequencer.call(
+            expected: expected,
+            actual: actual,
+            extra_classes: extra_operational_sequencer_classes,
+            extra_diff_formatter_classes: extra_diff_formatter_classes
+          )
+        rescue NoOperationalSequencerAvailableError
+          nil
+        end
       end
     end
   end