Add question mark to Gitlab::Diff::File predicate methods

11 files changed, 50 insertions, 43 deletions

diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 4a06ee653ee..da5838247a6 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -120,8 +120,8 @@ module DiffHelper
     }
   end
 
-  def editable_diff?(diff)
-    !diff.deleted_file && @merge_request && @merge_request.source_project
+  def editable_diff?(diff_file)
+    !diff_file.deleted_file? && @merge_request && @merge_request.source_project
   end
 
   private
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index 02eb7c8462c..e9e68b62771 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -26,28 +26,28 @@
 
   %h4 #{pluralize @message.diffs_count, "changed file"}:
 
+  - diff_files = @message.diffs
+
   %ul
-    - @message.diffs.each do |diff|
+    - diff_files.each do |diff_file|
       %li.file-stats
-        %a{ href: "#{@message.target_url if @message.disable_diffs?}##{hexdigest(diff.file_path)}" }
-          - if diff.deleted_file
+        %a{ href: "#{@message.target_url if @message.disable_diffs?}##{hexdigest(diff_file.file_path)}" }
+          - if diff_file.deleted_file?
             %span.deleted-file
               −
-              = diff.old_path
-          - elsif diff.renamed_file
-            = diff.old_path
+              = diff_file.old_path
+          - elsif diff_file.renamed_file?
+            = diff_file.old_path
             →
-            = diff.new_path
-          - elsif diff.new_file
+            = diff_file.new_path
+          - elsif diff_file.new_file?
             %span.new-file
               +
-              = diff.new_path
+              = diff_file.new_path
           - else
-            = diff.new_path
+            = diff_file.new_path
 
   - unless @message.disable_diffs?
-    - diff_files = @message.diffs
-
     - if @message.compare_timeout
       %h5 The diff was not included because it is too large.
     - else
@@ -56,11 +56,11 @@
         - file_hash = hexdigest(diff_file.file_path)
         %li{ id: file_hash }
           %a{ href: @message.target_url + "##{file_hash}" }<
-            - if diff_file.deleted_file
+            - if diff_file.deleted_file?
               %strong<
                 = diff_file.old_path
               deleted
-            - elsif diff_file.renamed_file
+            - elsif diff_file.renamed_file?
               %strong<
                 = diff_file.old_path
               &rarr;
diff --git a/app/views/notify/repository_push_email.text.haml b/app/views/notify/repository_push_email.text.haml
index 5ac23aa3997..895d8807e47 100644
--- a/app/views/notify/repository_push_email.text.haml
+++ b/app/views/notify/repository_push_email.text.haml
@@ -15,15 +15,15 @@
   \
   #{pluralize @message.diffs_count, "changed file"}:
   \
-  - @message.diffs.each do |diff|
-    - if diff.deleted_file
-      \- − #{diff.old_path}
-    - elsif diff.renamed_file
-      \- #{diff.old_path} → #{diff.new_path}
-    - elsif diff.new_file
-      \- + #{diff.new_path}
+  - @message.diffs.each do |diff_file|
+    - if diff_file.deleted_file?
+      \- − #{diff_file.old_path}
+    - elsif diff_file.renamed_file?
+      \- #{diff_file.old_path} → #{diff_file.new_path}
+    - elsif diff_file.new_file?
+      \- + #{diff_file.new_path}
     - else
-      \- #{diff.new_path}
+      \- #{diff_file.new_path}
   - unless @message.disable_diffs?
     - if @message.compare_timeout
       \
@@ -36,9 +36,9 @@
       - @message.diffs.each do |diff_file|
         \
         \=====================================
-        - if diff_file.deleted_file
+        - if diff_file.deleted_file?
           #{diff_file.old_path} deleted
-        - elsif diff_file.renamed_file
+        - elsif diff_file.renamed_file?
           #{diff_file.old_path} → #{diff_file.new_path}
         - else
           = diff_file.new_path
diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml
index c781e423c4d..a18451d8986 100644
--- a/app/views/projects/diffs/_content.html.haml
+++ b/app/views/projects/diffs/_content.html.haml
@@ -25,7 +25,7 @@
     - else
       - if diff_file.mode_changed?
         .nothing-here-block File mode changed
-      - elsif diff_file.renamed_file
+      - elsif diff_file.renamed_file?
         .nothing-here-block File moved
   - elsif blob.image?
     - old_blob = diff_file.old_blob(diff_file.old_content_commit || @base_commit)
diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml
index 45c95f7ab6a..8e5f4d2573d 100644
--- a/app/views/projects/diffs/_parallel_view.html.haml
+++ b/app/views/projects/diffs/_parallel_view.html.haml
@@ -49,7 +49,7 @@
 
       - if discussions_left || discussions_right
         = render "discussions/parallel_diff_discussion", discussions_left: discussions_left, discussions_right: discussions_right
-    - if !diff_file.new_file && !diff_file.deleted_file && diff_file.diff_lines.any?
+    - if !diff_file.new_file? && !diff_file.deleted_file? && diff_file.diff_lines.any?
       - last_line = diff_file.diff_lines.last
       - if last_line.new_pos < total_lines
         %tr.line_holder.parallel
diff --git a/app/views/projects/diffs/_stats.html.haml b/app/views/projects/diffs/_stats.html.haml
index fd4f3c8d3cc..e69c7f20d49 100644
--- a/app/views/projects/diffs/_stats.html.haml
+++ b/app/views/projects/diffs/_stats.html.haml
@@ -12,19 +12,19 @@
       - diff_files.each do |diff_file|
         - file_hash = hexdigest(diff_file.file_path)
         %li
-          - if diff_file.deleted_file
+          - if diff_file.deleted_file?
             %span.deleted-file
               %a{ href: "##{file_hash}" }
                 %i.fa.fa-minus
                 = diff_file.old_path
-          - elsif diff_file.renamed_file
+          - elsif diff_file.renamed_file?
             %span.renamed-file
               %a{ href: "##{file_hash}" }
                 %i.fa.fa-minus
                 = diff_file.old_path
                 &rarr;
                 = diff_file.new_path
-          - elsif diff_file.new_file
+          - elsif diff_file.new_file?
             %span.new-file
               %a{ href: "##{file_hash}" }
                 %i.fa.fa-plus
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index 5f3968b6709..5528014095c 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -9,7 +9,7 @@
     as: :line,
     locals: { diff_file: diff_file, discussions: @grouped_diff_discussions }
 
-  - if !diff_file.new_file && !diff_file.deleted_file && diff_file.highlighted_diff_lines.any?
+  - if !diff_file.new_file? && !diff_file.deleted_file? && diff_file.highlighted_diff_lines.any?
     - last_line = diff_file.highlighted_diff_lines.last
     - if last_line.new_pos < total_lines
       %tr.line_holder
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 01cc8e8e1ca..cf4affe1758 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -252,7 +252,9 @@ module API
 
     class RepoDiff < Grape::Entity
       expose :old_path, :new_path, :a_mode, :b_mode, :diff
-      expose :new_file, :renamed_file, :deleted_file
+      expose :new_file?, as: :new_file
+      expose :renamed_file?, as: :renamed_file
+      expose :deleted_file?, as: :deleted_file
     end
 
     class Milestone < ProjectEntity
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index c6bf25b5874..cb9fda36156 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -3,8 +3,8 @@ module Gitlab
     class File
       attr_reader :diff, :repository, :diff_refs
 
-      delegate :new_file, :deleted_file, :renamed_file,
-        :old_path, :new_path, :a_mode, :b_mode,
+      delegate :new_file?, :deleted_file?, :renamed_file?,
+        :old_path, :new_path, :a_mode, :b_mode, :mode_changed?,
         :submodule?, :too_large?, :collapsed?, to: :diff, prefix: false
 
       def initialize(diff, repository:, diff_refs: nil)
@@ -85,10 +85,6 @@ module Gitlab
         @parallel_diff_lines ||= Gitlab::Diff::ParallelDiff.new(self).parallelize
       end
 
-      def mode_changed?
-        a_mode && b_mode && a_mode != b_mode
-      end
-
       def raw_diff
         diff.diff.to_s
       end
@@ -119,6 +115,7 @@ module Gitlab
 
       def old_blob(commit = old_content_commit)
         return unless commit
+        return if new_file?
 
         repository.blob_at(commit.id, old_path)
       end
@@ -130,7 +127,7 @@ module Gitlab
       end
 
       def file_identifier
-        "#{file_path}-#{new_file}-#{deleted_file}-#{renamed_file}"
+        "#{file_path}-#{new_file?}-#{deleted_file?}-#{renamed_file?}"
       end
     end
   end
diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb
index 31d1b66b4f7..deade337354 100644
--- a/lib/gitlab/git/diff.rb
+++ b/lib/gitlab/git/diff.rb
@@ -11,6 +11,10 @@ module Gitlab
       # Stats properties
       attr_accessor :new_file, :renamed_file, :deleted_file
 
+      alias_method :new_file?, :new_file
+      alias_method :deleted_file?, :deleted_file
+      alias_method :renamed_file?, :renamed_file
+
       attr_accessor :too_large
 
       # The maximum size of a diff to display.
@@ -208,6 +212,10 @@ module Gitlab
         hash
       end
 
+      def mode_changed?
+        a_mode && b_mode && a_mode != b_mode
+      end
+
       def submodule?
         a_mode == '160000' || b_mode == '160000'
       end
diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb
index cdf0af6d7ef..7095104d75c 100644
--- a/spec/lib/gitlab/diff/position_spec.rb
+++ b/spec/lib/gitlab/diff/position_spec.rb
@@ -22,7 +22,7 @@ describe Gitlab::Diff::Position, lib: true do
       it "returns the correct diff file" do
         diff_file = subject.diff_file(project.repository)
 
-        expect(diff_file.new_file).to be true
+        expect(diff_file.new_file?).to be true
         expect(diff_file.new_path).to eq(subject.new_path)
         expect(diff_file.diff_refs).to eq(subject.diff_refs)
       end
@@ -314,7 +314,7 @@ describe Gitlab::Diff::Position, lib: true do
       it "returns the correct diff file" do
         diff_file = subject.diff_file(project.repository)
 
-        expect(diff_file.deleted_file).to be true
+        expect(diff_file.deleted_file?).to be true
         expect(diff_file.old_path).to eq(subject.old_path)
         expect(diff_file.diff_refs).to eq(subject.diff_refs)
       end
@@ -356,7 +356,7 @@ describe Gitlab::Diff::Position, lib: true do
       it "returns the correct diff file" do
         diff_file = subject.diff_file(project.repository)
 
-        expect(diff_file.new_file).to be true
+        expect(diff_file.new_file?).to be true
         expect(diff_file.new_path).to eq(subject.new_path)
         expect(diff_file.diff_refs).to eq(subject.diff_refs)
       end