From 253e1818aa3efafb05d9994aab0f778d339e26ac Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Tue, 16 Oct 2018 15:56:03 +0200
Subject: [PATCH] Create helper to get all changed files

Danger apparently has three different objects which could contain files
you often want to check:

 - git.added_files
 - git.modified_files
 - git.renamed_files

The problem: If a file is renamed, `modified_files` contains the file
path before the rename. In some Danger checks we use `added_files` +
`modified_files`, which might contain the deleted paths of renamed
files, but missing the new paths of renamed files.

So we need to consider `renamed_files` as well.
---
 Dangerfile                      |  1 +
 danger/database/Dangerfile      |  4 +---
 danger/documentation/Dangerfile |  4 +---
 danger/eslint/Dangerfile        |  2 +-
 danger/plugins/helper.rb        | 34 +++++++++++++++++++++++++++++++++
 danger/prettier/Dangerfile      |  2 +-
 6 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 danger/plugins/helper.rb

diff --git a/Dangerfile b/Dangerfile
index 10caacff4c4f6..469e77b251454 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -1,3 +1,4 @@
+danger.import_plugin('danger/plugins/helper.rb')
 danger.import_dangerfile(path: 'danger/metadata')
 danger.import_dangerfile(path: 'danger/changes_size')
 danger.import_dangerfile(path: 'danger/changelog')
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
index ad5f1c1e0f36b..38ccbd94edbee 100644
--- a/danger/database/Dangerfile
+++ b/danger/database/Dangerfile
@@ -39,8 +39,6 @@ def database_paths_requiring_review(files)
   to_review
 end
 
-all_files = git.added_files + git.modified_files
-
 non_geo_db_schema_updated = !git.modified_files.grep(%r{\Adb/schema\.rb}).empty?
 geo_db_schema_updated = !git.modified_files.grep(%r{\Aee/db/geo/schema\.rb}).empty?
 
@@ -55,7 +53,7 @@ if geo_migration_created && !geo_db_schema_updated
   warn format(SCHEMA_NOT_UPDATED_MESSAGE, migrations: 'Geo migrations', schema: gitlab.html_link("ee/db/geo/schema.rb"))
 end
 
-db_paths_to_review = database_paths_requiring_review(all_files)
+db_paths_to_review = database_paths_requiring_review(helper.all_changed_files)
 
 unless db_paths_to_review.empty?
   message 'This merge request adds or changes files that require a ' \
diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile
index d65bec123a92b..fe819ee250c30 100644
--- a/danger/documentation/Dangerfile
+++ b/danger/documentation/Dangerfile
@@ -11,9 +11,7 @@ def docs_paths_requiring_review(files)
   end
 end
 
-all_files = git.added_files + git.modified_files
-
-docs_paths_to_review = docs_paths_requiring_review(all_files)
+docs_paths_to_review = docs_paths_requiring_review(helper.all_changed_files)
 
 unless docs_paths_to_review.empty?
   message 'This merge request adds or changes files that require a ' \
diff --git a/danger/eslint/Dangerfile b/danger/eslint/Dangerfile
index f78488cfd0a95..4916cacfd7e35 100644
--- a/danger/eslint/Dangerfile
+++ b/danger/eslint/Dangerfile
@@ -7,7 +7,7 @@ def get_eslint_files(files)
   end
 end
 
-eslint_candidates = get_eslint_files(git.added_files + git.modified_files)
+eslint_candidates = get_eslint_files(helper.all_changed_files)
 
 return if eslint_candidates.empty?
 
diff --git a/danger/plugins/helper.rb b/danger/plugins/helper.rb
new file mode 100644
index 0000000000000..f4eb91192662c
--- /dev/null
+++ b/danger/plugins/helper.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Danger
+  # Common helper functions for our danger scripts
+  # If we find ourselves repeating code in our danger files, we might as well put them in here.
+  class Helper < Plugin
+    # Returns a list of all files that have been added, modified or renamed.
+    # `git.modified_files` might contain paths that already have been renamed,
+    # so we need to remove them from the list.
+    #
+    # Considering these changes:
+    #
+    # - A new_file.rb
+    # - D deleted_file.rb
+    # - M modified_file.rb
+    # - R renamed_file_before.rb -> renamed_file_after.rb
+    #
+    # it will return
+    # ```
+    # [ 'new_file.rb', 'modified_file.rb', 'renamed_file_after.rb' ]
+    # ```
+    #
+    # @return [Array<String>]
+    def all_changed_files
+      Set.new
+        .merge(git.added_files.to_a)
+        .merge(git.modified_files.to_a)
+        .merge(git.renamed_files.map { |x| x[:after] })
+        .subtract(git.renamed_files.map { |x| x[:before] })
+        .to_a
+        .sort
+    end
+  end
+end
diff --git a/danger/prettier/Dangerfile b/danger/prettier/Dangerfile
index 86f9f6af475a9..37c4b78a213bd 100644
--- a/danger/prettier/Dangerfile
+++ b/danger/prettier/Dangerfile
@@ -6,7 +6,7 @@ def get_prettier_files(files)
   end
 end
 
-prettier_candidates = get_prettier_files(git.added_files + git.modified_files)
+prettier_candidates = get_prettier_files(helper.all_changed_files)
 
 return if prettier_candidates.empty?
 
-- 
GitLab