Add latest changes from gitlab-org/gitlab@master

3 files changed, 96 insertions, 9 deletions

diff --git a/rubocop/cop/migration/add_reference.rb b/rubocop/cop/migration/add_reference.rb
index 1d471b9797e..33840fc7caf 100644
--- a/rubocop/cop/migration/add_reference.rb
+++ b/rubocop/cop/migration/add_reference.rb
@@ -4,22 +4,62 @@ require_relative '../../migration_helpers'
 module RuboCop
   module Cop
     module Migration
-      # Cop that checks if a foreign key constraint is added and require a index for it
+      # add_reference can only be used with newly created tables.
+      # Additionally, the cop here checks that we create an index for the foreign key, too.
       class AddReference < RuboCop::Cop::Cop
         include MigrationHelpers
 
-        MSG = '`add_reference` requires `index: true` or `index: { options... }`'
+        MSG = '`add_reference` requires downtime for existing tables, use `add_concurrent_foreign_key` instead. When used for new tables, `index: true` or `index: { options... } is required.`'
 
-        def on_send(node)
+        def on_def(node)
           return unless in_migration?(node)
 
-          name = node.children[1]
+          new_tables = []
 
-          return unless name == :add_reference
+          node.each_descendant(:send) do |send_node|
+            first_arg = first_argument(send_node)
 
+            # The first argument of "create_table" / "add_reference" is the table
+            # name.
+            new_tables << first_arg if create_table?(send_node)
+
+            next if method_name(send_node) != :add_reference
+
+            # Using "add_reference" is fine for newly created tables as there's no
+            # data in these tables yet.
+            if existing_table?(new_tables, first_arg)
+              add_offense(send_node, location: :selector)
+            end
+
+            # We require an index on the foreign key column.
+            if index_missing?(node)
+              add_offense(send_node, location: :selector)
+            end
+          end
+        end
+
+        private
+
+        def existing_table?(new_tables, table)
+          !new_tables.include?(table)
+        end
+
+        def create_table?(node)
+          method_name(node) == :create_table
+        end
+
+        def method_name(node)
+          node.children[1]
+        end
+
+        def first_argument(node)
+          node.children[2]
+        end
+
+        def index_missing?(node)
           opts = node.children.last
 
-          add_offense(node, location: :selector) unless opts && opts.type == :hash
+          return true if opts && opts.type == :hash
 
           index_present = false
 
@@ -27,11 +67,9 @@ module RuboCop
             index_present ||= index_enabled?(pair)
           end
 
-          add_offense(node, location: :selector) unless index_present
+          !index_present
         end
 
-        private
-
         def index_enabled?(pair)
           return unless hash_key_type(pair) == :sym
           return unless hash_key_name(pair) == :index
diff --git a/rubocop/cop/qa/ambiguous_page_object_name.rb b/rubocop/cop/qa/ambiguous_page_object_name.rb
new file mode 100644
index 00000000000..5cd8ea25c87
--- /dev/null
+++ b/rubocop/cop/qa/ambiguous_page_object_name.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require_relative '../../qa_helpers'
+
+module RuboCop
+  module Cop
+    module QA
+      # This cop checks for the usage of the ambiguous name "page"
+      #
+      # @example
+      #
+      #   # bad
+      #   Page::Object.perform do |page| do ...
+      #   Page::Another.perform { |page| ... }
+      #
+      #   # good
+      #   Page::Object.perform do |object| do ...
+      #   Page::Another.perform { |another| ... }
+      class AmbiguousPageObjectName < RuboCop::Cop::Cop
+        include QAHelpers
+
+        MESSAGE = "Don't use 'page' as a name for a Page Object. Use `%s` instead.".freeze
+
+        def_node_matcher :ambiguous_page?, <<~PATTERN
+          (block
+            (send
+              (const ...) :perform)
+            (args
+              (arg :page)) ...)
+        PATTERN
+
+        def on_block(node)
+          return unless in_qa_file?(node)
+          return unless ambiguous_page?(node)
+
+          add_offense(node.arguments.each_node(:arg).first,
+                      message: MESSAGE % page_object_name(node))
+        end
+
+        private
+
+        def page_object_name(node)
+          node.send_node.children[-2].const_name.downcase.split('::').last
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 29864777f59..8e7df62ea75 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -36,6 +36,7 @@ require_relative 'cop/rspec/env_assignment'
 require_relative 'cop/rspec/factories_in_migration_specs'
 require_relative 'cop/rspec/top_level_describe_path'
 require_relative 'cop/qa/element_with_pattern'
+require_relative 'cop/qa/ambiguous_page_object_name'
 require_relative 'cop/sidekiq_options_queue'
 require_relative 'cop/scalability/file_uploads'
 require_relative 'cop/destroy_all'