Merge branch '16950_extract_pair_selector' into 'master'

Extract PairSelector logic into a class [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55398

lib/gitlab/diff/inline_diff.rb

@@ -3,22 +3,6 @@
 module Gitlab
   module Diff
     class InlineDiff
-      # Regex to find a run of deleted lines followed by the same number of added lines
-      LINE_PAIRS_PATTERN = %r{
-        # Runs start at the beginning of the string (the first line) or after a space (for an unchanged line)
-        (?:\A|\s)
-
-        # This matches a number of `-`s followed by the same number of `+`s through recursion
-        (?<del_ins>
-          -
-          \g<del_ins>?
-          \+
-        )
-
-        # Runs end at the end of the string (the last line) or before a space (for an unchanged line)
-        (?=\s|\z)
-      }x.freeze
-
       attr_accessor :old_line, :new_line, :offset
 
       def initialize(old_line, new_line, offset: 0)
@@ -40,11 +24,11 @@ module Gitlab
 
       class << self
         def for_lines(lines, project: nil)
-          changed_line_pairs = find_changed_line_pairs(lines)
+          pair_selector = Gitlab::Diff::PairSelector.new(lines)
 
           inline_diffs = []
 
-          changed_line_pairs.each do |old_index, new_index|
+          pair_selector.each do |old_index, new_index|
             old_line = lines[old_index]
             new_line = lines[new_index]
 
@@ -56,34 +40,6 @@ module Gitlab
 
           inline_diffs
         end
-
-        private
-
-        # Finds pairs of old/new line pairs that represent the same line that changed
-        # rubocop: disable CodeReuse/ActiveRecord
-        def find_changed_line_pairs(lines)
-          # Prefixes of all diff lines, indicating their types
-          # For example: `" - +  -+  ---+++ --+  -++"`
-          line_prefixes = lines.each_with_object(+"") { |line, s| s << (line[0] || ' ') }.gsub(/[^ +-]/, ' ')
-
-          changed_line_pairs = []
-          line_prefixes.scan(LINE_PAIRS_PATTERN) do
-            # For `"---+++"`, `begin_index == 0`, `end_index == 6`
-            begin_index, end_index = Regexp.last_match.offset(:del_ins)
-
-            # For `"---+++"`, `changed_line_count == 3`
-            changed_line_count = (end_index - begin_index) / 2
-
-            halfway_index = begin_index + changed_line_count
-            (begin_index...halfway_index).each do |i|
-              # For `"---+++"`, index 1 maps to 1 + 3 = 4
-              changed_line_pairs << [i, i + changed_line_count]
-            end
-          end
-
-          changed_line_pairs
-        end
-        # rubocop: enable CodeReuse/ActiveRecord
       end
 
       private

lib/gitlab/diff/pair_selector.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Diff
+    class PairSelector
+      include Enumerable
+
+      # Regex to find a run of deleted lines followed by the same number of added lines
+      # rubocop: disable Lint/MixedRegexpCaptureTypes
+      LINE_PAIRS_PATTERN = %r{
+        # Runs start at the beginning of the string (the first line) or after a space (for an unchanged line)
+        (?:\A|\s)
+
+        # This matches a number of `-`s followed by the same number of `+`s through recursion
+        (?<del_ins>
+          -
+          \g<del_ins>?
+          \+
+        )
+
+        # Runs end at the end of the string (the last line) or before a space (for an unchanged line)
+        (?=\s|\z)
+      }x.freeze
+      # rubocop: enable Lint/MixedRegexpCaptureTypes
+
+      def initialize(lines)
+        @lines = lines
+      end
+
+      # Finds pairs of old/new line pairs that represent the same line that changed
+      # rubocop: disable CodeReuse/ActiveRecord
+      def each
+        # Prefixes of all diff lines, indicating their types
+        # For example: `" - +  -+  ---+++ --+  -++"`
+        line_prefixes = lines.each_with_object(+"") { |line, s| s << (line[0] || ' ') }.gsub(/[^ +-]/, ' ')
+
+        line_prefixes.scan(LINE_PAIRS_PATTERN) do
+          # For `"---+++"`, `begin_index == 0`, `end_index == 6`
+          begin_index, end_index = Regexp.last_match.offset(:del_ins)
+
+          # For `"---+++"`, `changed_line_count == 3`
+          changed_line_count = (end_index - begin_index) / 2
+
+          halfway_index = begin_index + changed_line_count
+          (begin_index...halfway_index).each do |i|
+            # For `"---+++"`, index 1 maps to 1 + 3 = 4
+            yield [i, i + changed_line_count]
+          end
+        end
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
+      private
+
+      attr_reader :lines
+    end
+  end
+end

spec/lib/gitlab/diff/pair_selector_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Diff::PairSelector do
+  subject(:selector) { described_class.new(lines) }
+
+  describe '#to_a' do
+    subject { selector.to_a }
+
+    let(:lines) { diff.lines }
+
+    let(:diff) do
+      <<-EOF.strip_heredoc
+         class Test                       # 0
+        -  def initialize(test = true)    # 1
+        +  def initialize(test = false)   # 2
+             @test = test                 # 3
+        -    if true                      # 4
+        -      @foo = "bar"               # 5
+        +    unless false                 # 6
+        +      @foo = "baz"               # 7
+             end
+           end
+         end
+      EOF
+    end
+
+    it 'finds all pairs' do
+      is_expected.to match_array([[1, 2], [4, 6], [5, 7]])
+    end
+
+    context 'when there are empty lines' do
+      let(:lines) { ['- bar', '+ baz', ''] }
+
+      it { expect { subject }.not_to raise_error }
+    end
+
+    context 'when there are only removals' do
+      let(:diff) do
+        <<-EOF.strip_heredoc
+          - class Test
+          -  def initialize(test = true)
+          -  end
+          - end
+        EOF
+      end
+
+      it 'returns empty collection' do
+        is_expected.to eq([])
+      end
+    end
+
+    context 'when there are only additions' do
+      let(:diff) do
+        <<-EOF.strip_heredoc
+          + class Test
+          +  def initialize(test = true)
+          +  end
+          + end
+        EOF
+      end
+
+      it 'returns empty collection' do
+        is_expected.to eq([])
+      end
+    end
+
+    context 'when there are no changes' do
+      let(:diff) do
+        <<-EOF.strip_heredoc
+           class Test
+             def initialize(test = true)
+             end
+           end
+        EOF
+      end
+
+      it 'returns empty collection' do
+        is_expected.to eq([])
+      end
+    end
+  end
+end