Add latest changes from gitlab-org/gitlab@15-9-stable-eev15.9.0-rc42

11 files changed, 376 insertions, 33 deletions

diff --git a/rubocop/cop/avoid_return_from_blocks.rb b/rubocop/cop/avoid_return_from_blocks.rb
index 61edfd0a789..c6a7a87c548 100644
--- a/rubocop/cop/avoid_return_from_blocks.rb
+++ b/rubocop/cop/avoid_return_from_blocks.rb
@@ -32,7 +32,7 @@ module RuboCop
         return unless top_block?(node)
 
         block_body.each_node(:return) do |return_node|
-          next if parent_blocks(node, return_node).all?(&method(:whitelisted?))
+          next if parent_blocks(node, return_node).all? { |block_node| whitelisted?(block_node) }
 
           add_offense(return_node)
         end
diff --git a/rubocop/cop/database/multiple_databases.rb b/rubocop/cop/database/multiple_databases.rb
index 26beecdeba7..eccb4a78219 100644
--- a/rubocop/cop/database/multiple_databases.rb
+++ b/rubocop/cop/database/multiple_databases.rb
@@ -20,6 +20,7 @@ module RuboCop
         ALLOWED_METHODS = %i[
           no_touching
           configurations
+          connection_handler
           logger
         ].freeze
 
diff --git a/rubocop/cop/gitlab/doc_url.rb b/rubocop/cop/gitlab/doc_url.rb
new file mode 100644
index 00000000000..cbfbdf7eb57
--- /dev/null
+++ b/rubocop/cop/gitlab/doc_url.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Gitlab
+      # This cop encourages using helper to link to documentation
+      # in string literals.
+      #
+      # @example
+      #   # bad
+      #   'See [the docs](https://docs.gitlab.com/ee/user/permissions#roles).'
+      #   _('See [the docs](https://docs.gitlab.com/ee/user/permissions#roles).')
+      #
+      #   # good
+      #   docs_link = link_to _('the docs'), help_page_url('user/permissions', anchor: 'roles')
+      #   "See #{docs_link}."
+      #   _('See %{docs_link}.').html_safe % { docs_link: docs_link.html_safe }
+      class DocUrl < RuboCop::Cop::Base
+        include RangeHelp
+
+        MSG = 'Use `#help_page_url` instead of directly including link. ' \
+              'See https://docs.gitlab.com/ee/development/documentation/#linking-to-help-in-ruby.'
+
+        DOCS_URL_REGEXP = %r{https://docs.gitlab.com/ee/[\w#%./-]+}.freeze
+
+        def on_str(node)
+          match = DOCS_URL_REGEXP.match(node.source)
+          return unless match
+
+          add_offense(bad_range(node, match))
+        end
+
+        private
+
+        def bad_range(node, match)
+          url_begin_pos, url_end_pos = match.offset(0)
+          begin_pos = node.loc.expression.begin_pos + url_begin_pos
+
+          range_between(begin_pos, begin_pos + (url_end_pos - url_begin_pos))
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb
index d1722a47c8a..ffd59c8fffc 100644
--- a/rubocop/cop/gitlab/mark_used_feature_flags.rb
+++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb
@@ -187,7 +187,7 @@ module RuboCop
         # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context}
         # is mostly used with dynamic event name.
         def track_usage_data_counters_known_events!
-          usage_data_counters_known_event_feature_flags.each(&method(:save_used_feature_flag))
+          usage_data_counters_known_event_feature_flags.each { |feature_flag_name| save_used_feature_flag(feature_flag_name) }
         end
 
         def usage_data_counters_known_event_feature_flags
diff --git a/rubocop/cop/migration/prevent_single_statement_with_disable_ddl_transaction.rb b/rubocop/cop/migration/prevent_single_statement_with_disable_ddl_transaction.rb
new file mode 100644
index 00000000000..547dfb60945
--- /dev/null
+++ b/rubocop/cop/migration/prevent_single_statement_with_disable_ddl_transaction.rb
@@ -0,0 +1,86 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that prevents usage of `disable_ddl_transaction!`
+      # if the only statement being called in the migration is :validate_foreign_key.
+      #
+      # We do this because PostgreSQL will add an implicit transaction for single
+      # statements. So there's no reason to add the disable_ddl_transaction!.
+      #
+      # This cop was introduced to clarify the need for disable_ddl_transaction!
+      # and to avoid bike-shedding and review back-and-forth.
+      #
+      # @examples
+      #
+      #   # bad
+      #   class SomeMigration < Gitlab::Database::Migration[2.1]
+      #     disable_ddl_transaction!
+      #     def up
+      #       validate_foreign_key :emails, :user_id
+      #     end
+      #     def down
+      #       # no-op
+      #     end
+      #   end
+      #
+      #   # good
+      #   class SomeMigration < Gitlab::Database::Migration[2.1]
+      #     def up
+      #       validate_foreign_key :emails, :user_id
+      #     end
+      #     def down
+      #       # no-op
+      #     end
+      #   end
+      class PreventSingleStatementWithDisableDdlTransaction < RuboCop::Cop::Base
+        include MigrationHelpers
+
+        MSG = "PostgreSQL will add an implicit transaction for single statements. " \
+              "So there's no reason to use `disable_ddl_transaction!`, if you're only " \
+              "executing validate_foreign_key."
+
+        def_node_matcher :disable_ddl_transaction?, <<~PATTERN
+          (send _ :disable_ddl_transaction! ...)
+        PATTERN
+
+        def_node_matcher :validate_foreign_key?, <<~PATTERN
+          (send :validate_foreign_key ...)
+        PATTERN
+
+        def on_begin(node)
+          return unless in_migration?(node)
+
+          disable_ddl_transaction_node = nil
+
+          # Only perform cop if disable_ddl_transaction! is present
+          node.each_descendant(:send) do |send_node|
+            disable_ddl_transaction_node = send_node if disable_ddl_transaction?(send_node)
+          end
+
+          return unless disable_ddl_transaction_node
+
+          # For each migration method, check if :validate_foreign_key is the only statement.
+          node.each_descendant(:def) do |def_node|
+            break unless [:up, :down, :change].include? def_node.children[0]
+
+            statement_count = 0
+            has_validate_foreign_key = false
+
+            def_node.each_descendant(:send) do |send_node|
+              has_validate_foreign_key = true if send_node.children[1] == :validate_foreign_key
+              statement_count += 1
+            end
+
+            if disable_ddl_transaction_node && has_validate_foreign_key && statement_count == 1
+              add_offense(disable_ddl_transaction_node, message: MSG)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/versioned_migration_class.rb b/rubocop/cop/migration/versioned_migration_class.rb
index 572ddcd1b12..27f1c0e16a1 100644
--- a/rubocop/cop/migration/versioned_migration_class.rb
+++ b/rubocop/cop/migration/versioned_migration_class.rb
@@ -8,13 +8,18 @@ module RuboCop
       class VersionedMigrationClass < RuboCop::Cop::Base
         include MigrationHelpers
 
-        ENFORCED_SINCE = 2021_09_02_00_00_00
-        CURRENT_DATABASE_MIGRATION_CLASS = 'Gitlab::Database::Migration[2.1]'
+        ENFORCED_SINCE = 2023_01_12_00_00_00
+        DOC_LINK = "https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning"
 
-        MSG_INHERIT = 'Don\'t inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.'
-        MSG_INCLUDE = 'Don\'t include migration helper modules directly. Inherit from Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.'
+        MSG_INHERIT = "Don't inherit from ActiveRecord::Migration or old versions of Gitlab::Database::Migration. " \
+                      "Use Gitlab::Database::Migration[2.1] instead. See #{DOC_LINK}."
 
+        MSG_INCLUDE = "Don't include migration helper modules directly. " \
+                      "Inherit from Gitlab::Database::Migration[2.1] instead. See #{DOC_LINK}."
+
+        GITLAB_MIGRATION_CLASS = 'Gitlab::Database::Migration'
         ACTIVERECORD_MIGRATION_CLASS = 'ActiveRecord::Migration'
+        CURRENT_MIGRATION_VERSION = 2.1 # Should be the same value as Gitlab::Database::Migration.current_version
 
         def_node_search :includes_helpers?, <<~PATTERN
         (send nil? :include
@@ -25,7 +30,7 @@ module RuboCop
 
         def on_class(node)
           return unless relevant_migration?(node)
-          return unless activerecord_migration_class?(node)
+          return unless activerecord_migration_class?(node) || old_version_migration_class?(node)
 
           add_offense(node, message: MSG_INHERIT)
         end
@@ -51,6 +56,15 @@ module RuboCop
 
           others.find { |node| node.const_type? && node&.const_name != 'Types' }&.const_name
         end
+
+        # Returns true for any parent class of format Gitlab::Database::Migration[version] if version < current_version
+        def old_version_migration_class?(class_node)
+          parent_class_node = class_node.parent_class
+          return false unless parent_class_node.send_type? && parent_class_node.arguments.last.float_type?
+          return false unless parent_class_node.children[0].const_name == GITLAB_MIGRATION_CLASS
+
+          parent_class_node.arguments[0].value < CURRENT_MIGRATION_VERSION
+        end
       end
     end
   end
diff --git a/rubocop/cop/rspec/env_mocking.rb b/rubocop/cop/rspec/env_mocking.rb
new file mode 100644
index 00000000000..96bd535df08
--- /dev/null
+++ b/rubocop/cop/rspec/env_mocking.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'rubocop-rspec'
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Check for ENV mocking in specs.
+      # See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#persistent-in-memory-application-state
+      #
+      # @example
+      #
+      #   # bad
+      #   allow(ENV).to receive(:[]).with('FOO').and_return('bar')
+      #   allow(ENV).to receive(:fetch).with('FOO').and_return('bar')
+      #   allow(ENV).to receive(:[]).with(key).and_return(value)
+      #   allow(ENV).to receive(:[]).with(fetch_key(object)).and_return(fetch_value(object))
+      #
+      #   # good
+      #   stub_env('FOO', 'bar')
+      #   stub_env(key, value)
+      #   stub_env(fetch_key(object), fetch_value(object))
+      class EnvMocking < RuboCop::Cop::Base
+        extend RuboCop::Cop::AutoCorrector
+
+        MESSAGE = "Don't mock the ENV, use `stub_env` instead."
+
+        def_node_matcher :env_mocking?, <<~PATTERN
+        (send
+          (send nil? :allow
+            (const {nil? cbase} :ENV)
+          )
+          :to
+          (send
+            (send
+              (send nil? :receive (sym {:[] :fetch}))
+              :with $_
+            )
+            :and_return $_
+          )
+          ...
+        )
+        PATTERN
+
+        def on_send(node)
+          env_mocking?(node) do |key, value|
+            add_offense(node, message: MESSAGE) do |corrector|
+              corrector.replace(node.loc.expression, stub_env(key.source, value.source))
+            end
+          end
+        end
+
+        def stub_env(key, value)
+          "stub_env(#{key}, #{value})"
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/rspec/invalid_feature_category.rb b/rubocop/cop/rspec/invalid_feature_category.rb
new file mode 100644
index 00000000000..9ef880d6aac
--- /dev/null
+++ b/rubocop/cop/rspec/invalid_feature_category.rb
@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+require 'rubocop/cop/rspec/base'
+require 'rubocop/cop/rspec/mixin/top_level_group'
+require 'did_you_mean'
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Ensures that feature categories in specs are valid.
+      #
+      # @example
+      #
+      #   # bad
+      #   RSpec.describe 'foo', feature_category: :invalid do
+      #   end
+      #
+      #   RSpec.describe 'foo', feature_category: :not_owned do
+      #   end
+      #
+      #   # good
+      #
+      #   RSpec.describe 'foo', feature_category: :wiki do
+      #   end
+      #
+      #   RSpec.describe 'foo', feature_category: :tooling do
+      #   end
+      #
+      class InvalidFeatureCategory < RuboCop::Cop::RSpec::Base
+        MSG = 'Please use a valid feature category. %{msg_suggestion}' \
+              'See https://docs.gitlab.com/ee/development/feature_categorization/#rspec-examples.'
+
+        MSG_DID_YOU_MEAN = 'Did you mean `:%{suggestion}`? '
+
+        MSG_SYMBOL = 'Please use a symbol as value.'
+
+        FEATURE_CATEGORIES_PATH = File.expand_path('../../../config/feature_categories.yml', __dir__).freeze
+
+        # List of feature categories which are not defined in config/feature_categories.yml
+        CUSTOM_FEATURE_CATEGORIES = [
+          # https://docs.gitlab.com/ee/development/feature_categorization/#tooling-feature-category
+          :tooling,
+          # https://docs.gitlab.com/ee/development/feature_categorization/#shared-feature-category
+          :shared
+        ].to_set.freeze
+
+        # @!method feature_category?(node)
+        def_node_matcher :feature_category_value, <<~PATTERN
+          (block
+            (send #rspec? {#ExampleGroups.all #Examples.all} ...
+              (hash <(pair (sym :feature_category) $_) ...>)
+            )
+            ...
+          )
+        PATTERN
+
+        def on_block(node)
+          value_node = feature_category_value(node)
+          return unless value_node
+
+          unless value_node.sym_type?
+            add_offense(value_node, message: MSG_SYMBOL)
+            return
+          end
+
+          return if valid_feature_category?(value_node)
+
+          message = format(MSG, msg_suggestion: suggestion_message(value_node))
+          add_offense(value_node, message: message)
+        end
+
+        # Used by RuboCop to invalidate its cache if the contents of
+        # config/feature_categories.yml changes.
+        def external_dependency_checksum
+          @external_dependency_checksum ||=
+            Digest::SHA256.file(FEATURE_CATEGORIES_PATH).hexdigest
+        end
+
+        private
+
+        def suggestion_message(value_node)
+          spell = DidYouMean::SpellChecker.new(dictionary: self.class.feature_categories)
+
+          suggestions = spell.correct(value_node.value)
+          return if suggestions.none?
+
+          format(MSG_DID_YOU_MEAN, suggestion: suggestions.first)
+        end
+
+        def valid_feature_category?(node)
+          self.class.feature_categories.include?(node.value)
+        end
+
+        def self.feature_categories
+          @feature_categories ||= YAML
+            .load_file(FEATURE_CATEGORIES_PATH)
+            .map(&:to_sym)
+            .to_set
+            .union(CUSTOM_FEATURE_CATEGORIES)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/rspec/missing_feature_category.rb b/rubocop/cop/rspec/missing_feature_category.rb
new file mode 100644
index 00000000000..0f517473982
--- /dev/null
+++ b/rubocop/cop/rspec/missing_feature_category.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'rubocop/cop/rspec/base'
+require 'rubocop/cop/rspec/mixin/top_level_group'
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Ensures that top level example group contains a feature category.
+      #
+      # @example
+      #
+      #   # bad
+      #   RSpec.describe 'foo' do
+      #   end
+      #
+      #   # good
+      #   RSpec.describe 'foo', feature_category: :wiki do
+      #   end
+      class MissingFeatureCategory < RuboCop::Cop::RSpec::Base
+        include RuboCop::Cop::RSpec::TopLevelGroup
+
+        MSG = 'Please add missing feature category. ' \
+              'See https://docs.gitlab.com/ee/development/feature_categorization/#rspec-examples.'
+
+        # @!method feature_category?(node)
+        def_node_matcher :feature_category?, <<~PATTERN
+            (block
+              (send ...
+                (hash <(pair (sym :feature_category) _) ...>)
+              )
+              ...
+            )
+        PATTERN
+
+        def on_top_level_example_group(node)
+          return if feature_category?(node)
+
+          add_offense(node.children.first)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/scalability/file_uploads.rb b/rubocop/cop/scalability/file_uploads.rb
index 3ccb9110e79..fc52444c551 100644
--- a/rubocop/cop/scalability/file_uploads.rb
+++ b/rubocop/cop/scalability/file_uploads.rb
@@ -26,34 +26,25 @@ module RuboCop
       #   end
       #
       class FileUploads < RuboCop::Cop::Base
-        MSG = 'Do not upload files without workhorse acceleration. Please refer to https://docs.gitlab.com/ee/development/uploads.html'
-
-        def_node_search :file_type_params?, <<~PATTERN
-          (send nil? {:requires :optional} (sym _) (hash <(pair (sym :type)(const nil? :File)) ...>))
-        PATTERN
-
-        def_node_search :file_types_params?, <<~PATTERN
-          (send nil? {:requires :optional} (sym _) (hash <(pair (sym :types)(array <(const nil? :File) ...>)) ...>))
+        MSG = 'Do not upload files without workhorse acceleration. ' \
+              'Please refer to https://docs.gitlab.com/ee/development/uploads.html'
+
+        def_node_matcher :file_in_type, <<~PATTERN
+          (send nil? {:requires :optional}
+            (sym _)
+            (hash
+              {
+                <(pair (sym :types) (array <$(const nil? :File) ...>)) ...>
+                <(pair (sym :type) $(const nil? :File)) ...>
+              }
+            )
+          )
         PATTERN
 
-        def be_file_param_usage?(node)
-          file_type_params?(node) || file_types_params?(node)
-        end
-
         def on_send(node)
-          return unless be_file_param_usage?(node)
-
-          add_offense(find_file_param(node))
-        end
-
-        private
-
-        def find_file_param(node)
-          node.each_descendant.find { |children| file_node_pattern.match(children) }
-        end
-
-        def file_node_pattern
-          @file_node_pattern ||= RuboCop::NodePattern.new("(const nil? :File)")
+          file_in_type(node) do |file_node|
+            add_offense(file_node)
+          end
         end
       end
     end
diff --git a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb
index 34c3767ad11..7fe308d6b40 100644
--- a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb
+++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb
@@ -26,7 +26,7 @@ module RuboCop
       class WorkerDataConsistency < RuboCop::Cop::Base
         include CodeReuseHelpers
 
-        HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#job-data-consistency-strategies'
+        HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq/worker_attributes.html#job-data-consistency-strategies'
 
         MSG = <<~MSG
           Should define data_consistency expectation.