Add latest changes from gitlab-org/gitlab@13-1-stable-ee

11 files changed, 197 insertions, 166 deletions

diff --git a/rubocop/cop/default_scope.rb b/rubocop/cop/default_scope.rb
new file mode 100644
index 00000000000..39f8c8e9ed0
--- /dev/null
+++ b/rubocop/cop/default_scope.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Cop that blacklists the use of `default_scope`.
+    class DefaultScope < RuboCop::Cop::Cop
+      MSG = <<~EOF
+        Do not use `default_scope`, as it does not follow the principle of
+        least surprise. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33847
+        for more details.
+      EOF
+
+      def_node_matcher :default_scope?, <<~PATTERN
+        (send {nil? (const nil? ...)} :default_scope ...)
+      PATTERN
+
+      def on_send(node)
+        return unless default_scope?(node)
+
+        add_offense(node, location: :expression)
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/gitlab/avoid_feature_get.rb b/rubocop/cop/gitlab/avoid_feature_get.rb
new file mode 100644
index 00000000000..e36e0b020c0
--- /dev/null
+++ b/rubocop/cop/gitlab/avoid_feature_get.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Gitlab
+      # Cop that blacklists the use of `Feature.get`.
+      class AvoidFeatureGet < RuboCop::Cop::Cop
+        MSG = 'Use `Feature.enable/disable` methods instead of `Feature.get`. ' \
+          'See doc/development/testing_guide/best_practices.md#feature-flags-in-tests for more information.'
+
+        def_node_matcher :feature_get?, <<~PATTERN
+          (send (const nil? :Feature) :get ...)
+        PATTERN
+
+        def_node_matcher :global_feature_get?, <<~PATTERN
+          (send (const (cbase) :Feature) :get ...)
+        PATTERN
+
+        def on_send(node)
+          return unless feature_get?(node) || global_feature_get?(node)
+
+          add_offense(node, location: :selector)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/gitlab/bulk_insert.rb b/rubocop/cop/gitlab/bulk_insert.rb
new file mode 100644
index 00000000000..c03ffbe0b2a
--- /dev/null
+++ b/rubocop/cop/gitlab/bulk_insert.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Gitlab
+      # Cop that disallows the use of `Gitlab::Database.bulk_insert`, in favour of using
+      # the `BulkInsertSafe` module.
+      class BulkInsert < RuboCop::Cop::Cop
+        MSG = 'Use the `BulkInsertSafe` concern, instead of using `Gitlab::Database.bulk_insert`. See https://docs.gitlab.com/ee/development/insert_into_tables_in_batches.html'
+
+        def_node_matcher :raw_union?, <<~PATTERN
+          (send (const (const nil? :Gitlab) :Database) :bulk_insert ...)
+        PATTERN
+
+        def on_send(node)
+          return unless raw_union?(node)
+
+          add_offense(node, location: :expression)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/inject_enterprise_edition_module.rb b/rubocop/cop/inject_enterprise_edition_module.rb
index 7edce94eaee..16e4b647265 100644
--- a/rubocop/cop/inject_enterprise_edition_module.rb
+++ b/rubocop/cop/inject_enterprise_edition_module.rb
@@ -2,9 +2,8 @@
 
 module RuboCop
   module Cop
-    # Cop that blacklists the injecting of EE specific modules anywhere but on
-    # the last line of a file. Injecting a module in the middle of a file will
-    # cause merge conflicts, while placing it on the last line will not.
+    # Cop that blacklists the injecting of EE specific modules before any lines which are not already injecting another module.
+    # It allows multiple module injections as long as they're all at the end.
     class InjectEnterpriseEditionModule < RuboCop::Cop::Cop
       INVALID_LINE = 'Injecting EE modules must be done on the last line of this file' \
           ', outside of any class or module definitions'
@@ -17,10 +16,12 @@ module RuboCop
       CHECK_LINE_METHODS =
         Set.new(%i[include_if_ee extend_if_ee prepend_if_ee]).freeze
 
-      CHECK_LINE_METHODS_REGEXP = Regexp.union(CHECK_LINE_METHODS.map(&:to_s)).freeze
-
       DISALLOW_METHODS = Set.new(%i[include extend prepend]).freeze
 
+      COMMENT_OR_EMPTY_LINE = /^\s*(#.*|$)/.freeze
+
+      CHECK_LINE_METHODS_REGEXP = Regexp.union((CHECK_LINE_METHODS + DISALLOW_METHODS).map(&:to_s) + [COMMENT_OR_EMPTY_LINE]).freeze
+
       def ee_const?(node)
         line = node.location.expression.source_line
 
@@ -44,15 +45,11 @@ module RuboCop
         line = node.location.line
         buffer = node.location.expression.source_buffer
         last_line = buffer.last_line
+        lines = buffer.source.split("\n")
+        # We allow multiple includes, extends and prepends as long as they're all at the end.
+        allowed_line = (line...last_line).all? { |i| CHECK_LINE_METHODS_REGEXP.match?(lines[i - 1]) }
 
-        # Parser treats the final newline (if present) as a separate line,
-        # meaning that a simple `line < last_line` would yield true even though
-        # the expression is the last line _of code_.
-        last_line -= 1 if buffer.source.end_with?("\n")
-
-        last_line_content = buffer.source.split("\n")[-1]
-
-        if CHECK_LINE_METHODS_REGEXP.match?(last_line_content)
+        if allowed_line
           ignore_node(node)
         elsif line < last_line
           add_offense(node, message: INVALID_LINE)
diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb
index 15c28bb9266..b578e73f19e 100644
--- a/rubocop/cop/migration/add_limit_to_text_columns.rb
+++ b/rubocop/cop/migration/add_limit_to_text_columns.rb
@@ -39,6 +39,9 @@ module RuboCop
         private
 
         def text_operation?(node)
+          # Don't complain about text arrays
+          return false if array_column?(node)
+
           modifier = node.children[0]
           migration_method = node.children[1]
 
diff --git a/rubocop/cop/migration/drop_table.rb b/rubocop/cop/migration/drop_table.rb
new file mode 100644
index 00000000000..2a0f57c0c13
--- /dev/null
+++ b/rubocop/cop/migration/drop_table.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if `drop_table` is called in deployment migrations.
+      # Calling it in deployment migrations can cause downtimes as there still may be code using the target tables.
+      class DropTable < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = <<-MESSAGE.delete("\n").squeeze
+          `drop_table` in deployment migrations requires downtime.
+          Drop tables in post-deployment migrations instead.
+        MESSAGE
+
+        def on_def(node)
+          return unless in_deployment_migration?(node)
+
+          node.each_descendant(:send) do |send_node|
+            next unless offensible?(send_node)
+
+            add_offense(send_node, location: :selector)
+          end
+        end
+
+        private
+
+        def offensible?(node)
+          drop_table?(node) || drop_table_in_execute?(node)
+        end
+
+        def drop_table?(node)
+          node.children[1] == :drop_table
+        end
+
+        def drop_table_in_execute?(node)
+          execute?(node) && drop_table_in_execute_sql?(node)
+        end
+
+        def execute?(node)
+          node.children[1] == :execute
+        end
+
+        def drop_table_in_execute_sql?(node)
+          node.children[2].to_s.match?(/drop\s+table/i)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/prevent_strings.rb b/rubocop/cop/migration/prevent_strings.rb
index 25c00194698..bfeabd2c78d 100644
--- a/rubocop/cop/migration/prevent_strings.rb
+++ b/rubocop/cop/migration/prevent_strings.rb
@@ -33,6 +33,9 @@ module RuboCop
         private
 
         def string_operation?(node)
+          # Don't complain about string arrays
+          return false if array_column?(node)
+
           modifier = node.children[0]
           migration_method = node.children[1]
 
diff --git a/rubocop/cop/migration/update_large_table.rb b/rubocop/cop/migration/update_large_table.rb
deleted file mode 100644
index e44eadbdb92..00000000000
--- a/rubocop/cop/migration/update_large_table.rb
+++ /dev/null
@@ -1,49 +0,0 @@
-require_relative '../../migration_helpers'
-
-module RuboCop
-  module Cop
-    module Migration
-      # This cop checks for `add_column_with_default` on a table that's been
-      # explicitly blacklisted because of its size.
-      #
-      # Even though this helper performs the update in batches to avoid
-      # downtime, using it with tables with millions of rows still causes a
-      # significant delay in the deploy process and is best avoided.
-      #
-      # See https://gitlab.com/gitlab-com/infrastructure/issues/1602 for more
-      # information.
-      class UpdateLargeTable < RuboCop::Cop::Cop
-        include MigrationHelpers
-
-        MSG = 'Using `%s` on the `%s` table will take a long time to ' \
-              'complete, and should be avoided unless absolutely ' \
-              'necessary'.freeze
-
-        BATCH_UPDATE_METHODS = %w[
-          :add_column_with_default
-          :change_column_type_concurrently
-          :rename_column_concurrently
-          :update_column_in_batches
-        ].join(' ').freeze
-
-        def_node_matcher :batch_update?, <<~PATTERN
-          (send nil? ${#{BATCH_UPDATE_METHODS}} $(sym ...) ...)
-        PATTERN
-
-        def on_send(node)
-          return unless in_migration?(node)
-
-          matches = batch_update?(node)
-          return unless matches
-
-          update_method = matches.first
-          table = matches.last.to_a.first
-
-          return unless BLACKLISTED_TABLES.include?(table)
-
-          add_offense(node, location: :expression, message: format(MSG, update_method, table))
-        end
-      end
-    end
-  end
-end
diff --git a/rubocop/cop/rspec/empty_line_after_shared_example.rb b/rubocop/cop/rspec/empty_line_after_shared_example.rb
deleted file mode 100644
index 5d09565bd5a..00000000000
--- a/rubocop/cop/rspec/empty_line_after_shared_example.rb
+++ /dev/null
@@ -1,64 +0,0 @@
-# frozen_string_literal: true
-
-require 'rubocop/rspec/final_end_location'
-require 'rubocop/rspec/blank_line_separation'
-require 'rubocop/rspec/language'
-
-module RuboCop
-  module Cop
-    module RSpec
-      # Checks if there is an empty line after shared example blocks.
-      #
-      # @example
-      #   # bad
-      #   RSpec.describe Foo do
-      #     it_behaves_like 'do this first'
-      #     it_behaves_like 'does this' do
-      #     end
-      #     it_behaves_like 'does that' do
-      #     end
-      #     it_behaves_like 'do some more'
-      #   end
-      #
-      #   # good
-      #   RSpec.describe Foo do
-      #     it_behaves_like 'do this first'
-      #     it_behaves_like 'does this' do
-      #     end
-      #
-      #     it_behaves_like 'does that' do
-      #     end
-      #
-      #     it_behaves_like 'do some more'
-      #   end
-      #
-      #   # fair - it's ok to have non-separated without blocks
-      #   RSpec.describe Foo do
-      #     it_behaves_like 'do this first'
-      #     it_behaves_like 'does this'
-      #   end
-      #
-      class EmptyLineAfterSharedExample < RuboCop::Cop::Cop
-        include RuboCop::RSpec::BlankLineSeparation
-        include RuboCop::RSpec::Language
-
-        MSG = 'Add an empty line after `%<example>s` block.'
-
-        def_node_matcher :shared_examples,
-                         (SharedGroups::ALL + Includes::ALL).block_pattern
-
-        def on_block(node)
-          shared_examples(node) do
-            break if last_child?(node)
-
-            missing_separating_line(node) do |location|
-              add_offense(node,
-                          location: location,
-                          message: format(MSG, example: node.method_name))
-            end
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb
index de377b15cc8..355450bbf57 100644
--- a/rubocop/migration_helpers.rb
+++ b/rubocop/migration_helpers.rb
@@ -7,45 +7,6 @@ module RuboCop
     ].freeze
 
     # Blacklisted tables due to:
-    #   - size in GB (>= 10 GB on GitLab.com as of 02/2020)
-    #   - number of records
-    BLACKLISTED_TABLES = %i[
-       audit_events
-       ci_build_trace_sections
-       ci_builds
-       ci_builds_metadata
-       ci_job_artifacts
-       ci_pipeline_variables
-       ci_pipelines
-       ci_stages
-       deployments
-       events
-       issues
-       merge_request_diff_commits
-       merge_request_diff_files
-       merge_request_diffs
-       merge_request_metrics
-       merge_requests
-       namespaces
-       note_diff_files
-       notes
-       project_authorizations
-       projects
-       project_ci_cd_settings
-       project_features
-       push_event_payloads
-       resource_label_events
-       routes
-       sent_notifications
-       services
-       system_note_metadata
-       taggings
-       todos
-       users
-       web_hook_logs
-    ].freeze
-
-    # Blacklisted tables due to:
     #   - number of columns (> 50 on GitLab.com as of 03/2020)
     #   - number of records
     WIDE_TABLES = %i[
@@ -62,7 +23,11 @@ module RuboCop
 
     # Returns true if the given node originated from the db/migrate directory.
     def in_migration?(node)
-      dirname(node).end_with?('db/migrate', 'db/geo/migrate') || in_post_deployment_migration?(node)
+      in_deployment_migration?(node) || in_post_deployment_migration?(node)
+    end
+
+    def in_deployment_migration?(node)
+      dirname(node).end_with?('db/migrate', 'db/geo/migrate')
     end
 
     def in_post_deployment_migration?(node)
@@ -73,6 +38,16 @@ module RuboCop
       File.basename(node.location.expression.source_buffer.name).split('_').first.to_i
     end
 
+    # Returns true if a column definition is for an array
+    # rubocop:disable Lint/BooleanSymbol
+    def array_column?(node)
+      node.each_descendant(:pair).any? do |pair_node|
+        pair_node.child_nodes[0].value == :array && # Searching for a (pair (sym :array) (true)) node
+        pair_node.child_nodes[1].type == :true # RuboCop::AST::Node uses symbols for types, even when that is a :true
+      end
+    end
+    # rubocop:enable Lint/BooleanSymbol
+
     private
 
     def dirname(node)
diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml
new file mode 100644
index 00000000000..8699f7b9c68
--- /dev/null
+++ b/rubocop/rubocop-migrations.yml
@@ -0,0 +1,40 @@
+Migration/UpdateLargeTable:
+  Enabled: true
+  DeniedTables: &denied_tables # size in GB (>= 10 GB on GitLab.com as of 02/2020) and/or number of records
+    - :audit_events
+    - :ci_build_trace_sections
+    - :ci_builds
+    - :ci_builds_metadata
+    - :ci_job_artifacts
+    - :ci_pipeline_variables
+    - :ci_pipelines
+    - :ci_stages
+    - :deployments
+    - :events
+    - :issues
+    - :merge_request_diff_commits
+    - :merge_request_diff_files
+    - :merge_request_diffs
+    - :merge_request_metrics
+    - :merge_requests
+    - :namespaces
+    - :note_diff_files
+    - :notes
+    - :project_authorizations
+    - :projects
+    - :project_ci_cd_settings
+    - :project_features
+    - :push_event_payloads
+    - :resource_label_events
+    - :routes
+    - :sent_notifications
+    - :services
+    - :system_note_metadata
+    - :taggings
+    - :todos
+    - :users
+    - :web_hook_logs
+  DeniedMethods:
+    - :change_column_type_concurrently
+    - :rename_column_concurrently
+    - :update_column_in_batches