Highlight merge request conflicts displayed in diff

Return particular types for diff lines if the line contain a conflict

app/models/merge_request.rb

@@ -1710,6 +1710,10 @@ class MergeRequest < ApplicationRecord
     false
   end
 
+  def highlight_diff_conflicts?
+    !branch_missing? && Feature.enabled?(:highlight_merge_conflicts_in_diff, project)
+  end
+
   private
 
   def with_rebase_lock

app/serializers/diff_file_entity.rb

@@ -54,7 +54,7 @@ class DiffFileEntity < DiffFileBaseEntity
 
   # Used for inline diffs
   expose :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, options) { inline_diff_view?(options, diff_file) && diff_file.text? } do |diff_file|
-    diff_file.diff_lines_for_serializer
+    diff_file.diff_lines_for_serializer(conflicts: options[:conflicts])
   end
 
   expose :is_fully_expanded do |diff_file|

app/serializers/diffs_entity.rb

@@ -71,7 +71,7 @@ class DiffsEntity < Grape::Entity
     submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository)
 
     DiffFileEntity.represent(diffs.diff_files,
-      options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs)))
+      options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs), conflicts: conflicts))
   end
 
   expose :merge_request_diffs, using: MergeRequestDiffEntity, if: -> (_, options) { options[:merge_request_diffs]&.any? } do |diffs|
@@ -116,4 +116,10 @@ class DiffsEntity < Grape::Entity
       next_commit_id: next_commit_id
     )
   end
+
+  def conflicts
+    return unless merge_request&.highlight_diff_conflicts?
+
+    MergeRequests::Conflicts::ListService.new(merge_request).conflicts # rubocop:disable CodeReuse/ServiceClass
+  end
 end

app/serializers/paginated_diff_entity.rb

@@ -15,7 +15,8 @@ class PaginatedDiffEntity < Grape::Entity
       diffs.diff_files,
       options.merge(
         submodule_links: submodule_links,
-        code_navigation_path: code_navigation_path(diffs)
+        code_navigation_path: code_navigation_path(diffs),
+        conflicts: conflicts
       )
     )
   end
@@ -58,4 +59,10 @@ class PaginatedDiffEntity < Grape::Entity
   def merge_request
     options[:merge_request]
   end
+
+  def conflicts
+    return unless merge_request&.highlight_diff_conflicts?
+
+    MergeRequests::Conflicts::ListService.new(merge_request).conflicts # rubocop:disable CodeReuse/ServiceClass
+  end
 end

config/feature_flags/development/highlight_merge_conflicts_in_diff.yml

+---
+name: highlight_merge_conflicts_in_diff
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47003
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/277097
+milestone: '13.6'
+type: development
+group: group::code review
+default_enabled: false

lib/gitlab/diff/file.rb

@@ -195,9 +195,9 @@ module Gitlab
         @highlighted_diff_lines.present?
       end
 
-      def highlighted_diff_lines
+      def highlighted_diff_lines(conflicts: nil)
         @highlighted_diff_lines ||=
-          Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight
+          Gitlab::Diff::Highlight.new(self, repository: self.repository, conflicts: conflicts).highlight
       end
 
       # Array[<Hash>] with right/left keys that contains Gitlab::Diff::Line objects which text is hightlighted
@@ -336,9 +336,9 @@ module Gitlab
 
       # This adds the bottom match line to the array if needed. It contains
       # the data to load more context lines.
-      def diff_lines_for_serializer
+      def diff_lines_for_serializer(conflicts: nil)
         strong_memoize(:diff_lines_for_serializer) do
-          lines = highlighted_diff_lines
+          lines = highlighted_diff_lines(conflicts: conflicts)
 
           next if lines.empty?
           next if blob.nil?

lib/gitlab/diff/highlight.rb

@@ -3,12 +3,15 @@
 module Gitlab
   module Diff
     class Highlight
-      attr_reader :diff_file, :diff_lines, :raw_lines, :repository
+      include Gitlab::Utils::StrongMemoize
+
+      attr_reader :diff_file, :diff_lines, :raw_lines, :repository, :conflicts
 
       delegate :old_path, :new_path, :old_sha, :new_sha, to: :diff_file, prefix: :diff
 
-      def initialize(diff_lines, repository: nil)
+      def initialize(diff_lines, repository: nil, conflicts: nil)
         @repository = repository
+        @conflicts = conflicts
 
         if diff_lines.is_a?(Gitlab::Diff::File)
           @diff_file = diff_lines
@@ -41,12 +44,23 @@ module Gitlab
 
           diff_line.rich_text = rich_line
 
+          diff_line_type = conflicts_parser&.diff_line_type(diff_line.text)
+          diff_line.type = diff_line_type if diff_line_type
+
           diff_line
         end
       end
 
       private
 
+      def conflicts_parser
+        strong_memoize(:conflicts_parser) do
+          next unless conflicts && diff_file
+
+          Gitlab::Git::Conflict::LineParser.new(diff_file.new_path, conflicts)
+        end
+      end
+
       def highlight_line(diff_line)
         return unless diff_file && diff_file.diff_refs
 

lib/gitlab/diff/line.rb

@@ -8,9 +8,9 @@ module Gitlab
       #
       SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze
 
-      attr_reader :line_code, :type, :old_pos, :new_pos
+      attr_reader :line_code, :old_pos, :new_pos
       attr_writer :rich_text
-      attr_accessor :text, :index
+      attr_accessor :text, :index, :type
 
       def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil)
         @text, @type, @index = text, type, index

lib/gitlab/git/conflict/line_parser.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Git
+    module Conflict
+      class LineParser
+        CONFLICT_OUR = 'conflict_our'
+        CONFLICT_THEIR = 'conflict_their'
+        CONFLICT_MARKER = 'conflict_marker'
+
+        attr_reader :markers
+        attr_accessor :type
+
+        def initialize(path, conflicts)
+          @markers = build_markers(path, conflicts)
+          @type = nil
+        end
+
+        def diff_line_type(line)
+          return unless markers
+
+          if markers.has_key?(line)
+            self.type = markers[line]
+
+            return CONFLICT_MARKER
+          end
+
+          type
+        end
+
+        private
+
+        def build_markers(path, conflicts)
+          return unless path
+
+          conflict = conflicts.files.find { |conflict| conflict.our_path == path }
+
+          return unless conflict
+
+          {
+            "+<<<<<<< #{conflict.our_path}" => CONFLICT_OUR,
+            "+=======" => CONFLICT_THEIR,
+            "+>>>>>>> #{conflict.their_path}" => nil
+          }
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/diff/file_spec.rb

@@ -60,12 +60,14 @@ RSpec.describe Gitlab::Diff::File do
 
   describe '#highlighted_diff_lines' do
     it 'highlights the diff and memoises the result' do
+      conflicts = double(files: [])
+
       expect(Gitlab::Diff::Highlight).to receive(:new)
-                                           .with(diff_file, repository: project.repository)
+                                           .with(diff_file, repository: project.repository, conflicts: conflicts)
                                            .once
                                            .and_call_original
 
-      diff_file.highlighted_diff_lines
+      diff_file.highlighted_diff_lines(conflicts: conflicts)
     end
   end
 

spec/lib/gitlab/git/conflict/line_parser_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Git::Conflict::LineParser do
+  describe '.assign_type' do
+    let(:parser) { described_class.new(path, double(files: conflicts)) }
+    let(:path) { 'files/ruby/regex.rb' }
+
+    let(:text) do
+      <<~CONFLICT
+          module Gitlab
+          +<<<<<<< #{path}
+              def project_name_regexp
+          +=======
+              def project_name_regex
+          +>>>>>>> #{path}
+              end
+      CONFLICT
+    end
+
+    let(:diff_line_types) do
+      text.lines.map { |line| parser.diff_line_type(line.chomp) }
+    end
+
+    context 'when the file has valid conflicts' do
+      let(:conflicts) { [double(our_path: path, their_path: path)] }
+
+      it 'assigns conflict types to the diff lines' do
+        expect(diff_line_types).to eq([
+          nil,
+          'conflict_marker',
+          'conflict_our',
+          'conflict_marker',
+          'conflict_their',
+          'conflict_marker',
+          nil
+        ])
+      end
+    end
+
+    context 'when the file does not have conflicts' do
+      let(:conflicts) { [] }
+
+      it 'does not change type of the diff lines' do
+        expect(diff_line_types).to eq(Array.new(7))
+      end
+    end
+  end
+end

spec/models/merge_request_spec.rb

@@ -4357,4 +4357,30 @@ RSpec.describe MergeRequest, factory_default: :keep do
         .from(nil).to(ref)
     end
   end
+
+  describe '#highlight_diff_conflicts?' do
+    let(:merge_request) { build_stubbed(:merge_request) }
+
+    it 'returns true' do
+      expect(merge_request.highlight_diff_conflicts?).to eq(true)
+    end
+
+    context 'when branch is missing' do
+      let(:merge_request) { build_stubbed(:merge_request, source_branch: nil) }
+
+      it 'returns false' do
+        expect(merge_request.highlight_diff_conflicts?).to eq(false)
+      end
+    end
+
+    context 'highlight_merge_conflicts_in_diff feature flag is disabled' do
+      before do
+        stub_feature_flags(highlight_merge_conflicts_in_diff: false)
+      end
+
+      it 'returns false' do
+        expect(merge_request.highlight_diff_conflicts?).to eq(false)
+      end
+    end
+  end
 end

spec/serializers/diffs_entity_spec.rb

@@ -68,5 +68,35 @@ RSpec.describe DiffsEntity do
         end
       end
     end
+
+    context 'when there are conflicts' do
+      let(:conflicts) { double(files: []) }
+
+      before do
+        allow_next_instance_of(MergeRequests::Conflicts::ListService) do |instance|
+          allow(instance).to receive(:conflicts).and_return(conflicts)
+        end
+      end
+
+      it 'lines are parsed with passed conflicts' do
+        expect(Gitlab::Git::Conflict::LineParser).to(
+          receive(:new).exactly(17).times.with(anything, conflicts).and_call_original
+        )
+
+        subject
+      end
+
+      context 'when diff lines should not be highlighted' do
+        before do
+          allow(merge_request).to receive(:highlight_diff_conflicts?).and_return(false)
+        end
+
+        it 'conflicts has no impact on line parsing' do
+          expect(Gitlab::Git::Conflict::LineParser).not_to receive(:new)
+
+          subject
+        end
+      end
+    end
   end
 end

spec/serializers/paginated_diff_entity_spec.rb

@@ -31,4 +31,34 @@ RSpec.describe PaginatedDiffEntity do
       total_pages: 7
     )
   end
+
+  context 'when there are conflicts' do
+    let(:conflicts) { double(files: []) }
+
+    before do
+      allow_next_instance_of(MergeRequests::Conflicts::ListService) do |instance|
+        allow(instance).to receive(:conflicts).and_return(conflicts)
+      end
+    end
+
+    it 'lines are parsed with passed conflicts' do
+      expect(Gitlab::Git::Conflict::LineParser).to(
+        receive(:new).exactly(3).times.with(anything, conflicts).and_call_original
+      )
+
+      subject
+    end
+
+    context 'when diff lines should not be highlighted' do
+      before do
+        allow(merge_request).to receive(:highlight_diff_conflicts?).and_return(false)
+      end
+
+      it 'conflicts has no impact on line parsing' do
+        expect(Gitlab::Git::Conflict::LineParser).not_to receive(:new)
+
+        subject
+      end
+    end
+  end
 end