Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc42

14 files changed, 350 insertions, 33 deletions

diff --git a/rubocop/cop/api/grape_api_instance.rb b/rubocop/cop/api/base.rb
index de11b9ef3f6..85b19e9a833 100644
--- a/rubocop/cop/api/grape_api_instance.rb
+++ b/rubocop/cop/api/base.rb
@@ -3,8 +3,8 @@
 module RuboCop
   module Cop
     module API
-      class GrapeAPIInstance < RuboCop::Cop::Cop
-        # This cop checks that APIs subclass Grape::API::Instance with Grape v1.3+.
+      class Base < RuboCop::Cop::Cop
+        # This cop checks that APIs subclass API::Base.
         #
         # @example
         #
@@ -14,19 +14,24 @@ module RuboCop
         #   end
         # end
         #
-        # # good
         # module API
         #   class Projects < Grape::API::Instance
         #   end
         # end
-        MSG = 'Inherit from Grape::API::Instance instead of Grape::API. ' \
-              'For more details check the https://gitlab.com/gitlab-org/gitlab/-/issues/215230.'
+        #
+        # # good
+        # module API
+        #   class Projects < ::API::Base
+        #   end
+        # end
+        MSG = 'Inherit from ::API::Base instead of Grape::API::Instance or Grape::API. ' \
+              'For more details check https://gitlab.com/gitlab-org/gitlab/-/issues/215230.'
 
+        def_node_matcher :grape_api, '(const (const {nil? (cbase)} :Grape) :API)'
         def_node_matcher :grape_api_definition, <<~PATTERN
           (class
             (const _ _)
-            (const
-              (const nil? :Grape) :API)
+            {#grape_api (const #grape_api :Instance)}
             ...
           )
         PATTERN
@@ -36,6 +41,12 @@ module RuboCop
             add_offense(node.children[1])
           end
         end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            corrector.replace(node, '::API::Base')
+          end
+        end
       end
     end
   end
diff --git a/rubocop/cop/code_reuse/active_record.rb b/rubocop/cop/code_reuse/active_record.rb
index 8c77c292315..f6b5d66647d 100644
--- a/rubocop/cop/code_reuse/active_record.rb
+++ b/rubocop/cop/code_reuse/active_record.rb
@@ -5,20 +5,20 @@ require_relative '../../code_reuse_helpers'
 module RuboCop
   module Cop
     module CodeReuse
-      # Cop that blacklists the use of ActiveRecord methods outside of models.
+      # Cop that denies the use of ActiveRecord methods outside of models.
       class ActiveRecord < RuboCop::Cop::Cop
         include CodeReuseHelpers
 
         MSG = 'This method can only be used inside an ActiveRecord model: ' \
         'https://gitlab.com/gitlab-org/gitlab-foss/issues/49653'
 
-        # Various methods from ActiveRecord::Querying that are blacklisted. We
+        # Various methods from ActiveRecord::Querying that are denied. We
         # exclude some generic ones such as `any?` and `first`, as these may
         # lead to too many false positives, since `Array` also supports these
         # methods.
         #
-        # The keys of this Hash are the blacklisted method names. The values are
-        # booleans that indicate if the method should only be blacklisted if any
+        # The keys of this Hash are the denied method names. The values are
+        # booleans that indicate if the method should only be denied if any
         # arguments are provided.
         NOT_ALLOWED = {
           average: true,
@@ -57,7 +57,6 @@ module RuboCop
           references: true,
           reorder: true,
           rewhere: true,
-          sum: false,
           take: false,
           take!: false,
           unscope: false,
@@ -65,9 +64,9 @@ module RuboCop
           with: true
         }.freeze
 
-        # Directories that allow the use of the blacklisted methods. These
+        # Directories that allow the use of the denied methods. These
         # directories are checked relative to both . and ee/
-        WHITELISTED_DIRECTORIES = %w[
+        ALLOWED_DIRECTORIES = %w[
           app/models
           config
           danger
@@ -88,7 +87,7 @@ module RuboCop
         ].freeze
 
         def on_send(node)
-          return if in_whitelisted_directory?(node)
+          return if in_allowed_directory?(node)
 
           receiver = node.children[0]
           send_name = node.children[1]
@@ -105,12 +104,12 @@ module RuboCop
           end
         end
 
-        # Returns true if the node resides in one of the whitelisted
+        # Returns true if the node resides in one of the allowed
         # directories.
-        def in_whitelisted_directory?(node)
+        def in_allowed_directory?(node)
           path = file_path_for_node(node)
 
-          WHITELISTED_DIRECTORIES.any? do |directory|
+          ALLOWED_DIRECTORIES.any? do |directory|
             path.start_with?(
               File.join(rails_root, directory),
               File.join(rails_root, 'ee', directory)
@@ -119,12 +118,12 @@ module RuboCop
         end
 
         # We can not auto correct code like this, as it requires manual
-        # refactoring. Instead, we'll just whitelist the surrounding scope.
+        # refactoring. Instead, we'll just allow the surrounding scope.
         #
         # Despite this method's presence, you should not use it. This method
-        # exists to make it possible to whitelist large chunks of offenses we
+        # exists to make it possible to allow large chunks of offenses we
         # can't fix in the short term. If you are writing new code, follow the
-        # code reuse guidelines, instead of whitelisting any new offenses.
+        # code reuse guidelines, instead of allowing any new offenses.
         def autocorrect(node)
           scope = surrounding_scope_of(node)
           indent = indentation_of(scope)
@@ -132,7 +131,7 @@ module RuboCop
           lambda do |corrector|
             # This prevents us from inserting the same enable/disable comment
             # for a method or block that has multiple offenses.
-            next if whitelisted_scopes.include?(scope)
+            next if allowed_scopes.include?(scope)
 
             corrector.insert_before(
               scope.source_range,
@@ -144,7 +143,7 @@ module RuboCop
               "\n#{indent}# rubocop: enable #{cop_name}"
             )
 
-            whitelisted_scopes << scope
+            allowed_scopes << scope
           end
         end
 
@@ -160,8 +159,8 @@ module RuboCop
           end
         end
 
-        def whitelisted_scopes
-          @whitelisted_scopes ||= Set.new
+        def allowed_scopes
+          @allowed_scopes ||= Set.new
         end
       end
     end
diff --git a/rubocop/cop/graphql/gid_expected_type.rb b/rubocop/cop/graphql/gid_expected_type.rb
new file mode 100644
index 00000000000..354c5516752
--- /dev/null
+++ b/rubocop/cop/graphql/gid_expected_type.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Graphql
+      class GIDExpectedType < RuboCop::Cop::Cop
+        MSG = 'Add an expected_type parameter to #object_from_id calls if possible.'
+
+        def_node_search :id_from_object?, <<~PATTERN
+          (send ... :object_from_id (...))
+        PATTERN
+
+        def on_send(node)
+          return unless id_from_object?(node)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/graphql/id_type.rb b/rubocop/cop/graphql/id_type.rb
new file mode 100644
index 00000000000..96f90ac136a
--- /dev/null
+++ b/rubocop/cop/graphql/id_type.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Graphql
+      class IDType < RuboCop::Cop::Cop
+        MSG = 'Do not use GraphQL::ID_TYPE, use a specific GlobalIDType instead'
+
+        WHITELISTED_ARGUMENTS = %i[iid full_path project_path group_path target_project_path].freeze
+
+        def_node_search :graphql_id_type?, <<~PATTERN
+          (send nil? :argument (_ #does_not_match?) (const (const nil? :GraphQL) :ID_TYPE) ...)
+        PATTERN
+
+        def on_send(node)
+          return unless graphql_id_type?(node)
+
+          add_offense(node)
+        end
+
+        private
+
+        def does_not_match?(arg)
+          !WHITELISTED_ARGUMENTS.include?(arg)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/add_concurrent_foreign_key.rb b/rubocop/cop/migration/add_concurrent_foreign_key.rb
index 236de6224a4..31cf426b2d4 100644
--- a/rubocop/cop/migration/add_concurrent_foreign_key.rb
+++ b/rubocop/cop/migration/add_concurrent_foreign_key.rb
@@ -11,7 +11,11 @@ module RuboCop
         MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead'.freeze
 
         def_node_matcher :false_node?, <<~PATTERN
-        (false)
+          (false)
+        PATTERN
+
+        def_node_matcher :with_lock_retries?, <<~PATTERN
+          (:send nil? :with_lock_retries)
         PATTERN
 
         def on_send(node)
@@ -19,9 +23,11 @@ module RuboCop
 
           name = node.children[1]
 
-          if name == :add_foreign_key && !not_valid_fk?(node)
-            add_offense(node, location: :selector)
-          end
+          return unless name == :add_foreign_key
+          return if in_with_lock_retries?(node)
+          return if not_valid_fk?(node)
+
+          add_offense(node, location: :selector)
         end
 
         def method_name(node)
@@ -33,6 +39,12 @@ module RuboCop
             pair.children[0].children[0] == :validate && false_node?(pair.children[1])
           end
         end
+
+        def in_with_lock_retries?(node)
+          node.each_ancestor(:block).any? do |parent|
+            with_lock_retries?(parent.to_a.first)
+          end
+        end
       end
     end
   end
diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb
index b578e73f19e..b2e37ad5137 100644
--- a/rubocop/cop/migration/add_limit_to_text_columns.rb
+++ b/rubocop/cop/migration/add_limit_to_text_columns.rb
@@ -6,6 +6,10 @@ module RuboCop
   module Cop
     module Migration
       # Cop that enforces always adding a limit on text columns
+      #
+      # Text columns starting with `encrypted_` are very likely used
+      # by `attr_encrypted` which controls the text length. Those columns
+      # should not add a text limit.
       class AddLimitToTextColumns < RuboCop::Cop::Cop
         include MigrationHelpers
 
@@ -102,6 +106,8 @@ module RuboCop
         # Check if there is an `add_text_limit` call for the provided
         # table and attribute name
         def text_limit_missing?(node, table_name, attribute_name)
+          return false if encrypted_attribute_name?(attribute_name)
+
           limit_found = false
 
           node.each_descendant(:send) do |send_node|
@@ -118,6 +124,10 @@ module RuboCop
 
           !limit_found
         end
+
+        def encrypted_attribute_name?(attribute_name)
+          attribute_name.to_s.start_with?('encrypted_')
+        end
       end
     end
   end
diff --git a/rubocop/cop/migration/create_table_with_foreign_keys.rb b/rubocop/cop/migration/create_table_with_foreign_keys.rb
index 01cab032049..382a2d6f65b 100644
--- a/rubocop/cop/migration/create_table_with_foreign_keys.rb
+++ b/rubocop/cop/migration/create_table_with_foreign_keys.rb
@@ -9,7 +9,7 @@ module RuboCop
         include MigrationHelpers
 
         MSG = 'Creating a table with more than one foreign key at once violates our migration style guide. ' \
-          'For more details check the https://docs.gitlab.com/ce/development/migration_style_guide.html#examples'
+          'For more details check the https://docs.gitlab.com/ee/development/migration_style_guide.html#examples'
 
         def_node_matcher :create_table_with_block?, <<~PATTERN
           (block
diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb
index a89c33c298b..aef19517a9d 100644
--- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb
+++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb
@@ -29,9 +29,14 @@ module RuboCop
         ].sort.freeze
 
         MSG = "The method is not allowed to be called within the `with_lock_retries` block, the only allowed methods are: #{ALLOWED_MIGRATION_METHODS.join(', ')}"
+        MSG_ONLY_ONE_FK_ALLOWED = "Avoid adding more than one foreign key within the `with_lock_retries`. See https://docs.gitlab.com/ee/development/migration_style_guide.html#examples"
 
         def_node_matcher :send_node?, <<~PATTERN
-        send
+          send
+        PATTERN
+
+        def_node_matcher :add_foreign_key?, <<~PATTERN
+          (send nil? :add_foreign_key ...)
         PATTERN
 
         def on_block(node)
@@ -53,6 +58,17 @@ module RuboCop
 
           name = node.children[1]
           add_offense(node, location: :expression) unless ALLOWED_MIGRATION_METHODS.include?(name)
+          add_offense(node, location: :selector, message: MSG_ONLY_ONE_FK_ALLOWED) if multiple_fks?(node)
+        end
+
+        def multiple_fks?(node)
+          return unless add_foreign_key?(node)
+
+          count = node.parent.each_descendant(:send).count do |node|
+            add_foreign_key?(node)
+          end
+
+          count > 1
         end
       end
     end
diff --git a/rubocop/cop/rspec/expect_gitlab_tracking.rb b/rubocop/cop/rspec/expect_gitlab_tracking.rb
new file mode 100644
index 00000000000..ba658558705
--- /dev/null
+++ b/rubocop/cop/rspec/expect_gitlab_tracking.rb
@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+require 'rack/utils'
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for `expect(Gitlab::Tracking).to receive(:event)` usage in specs.
+      # See /spec/support/helpers/snowplow_helpers.rb for details on the replacement.
+      #
+      # @example
+      #
+      # # bad
+      # it 'expects a snowplow event' do
+      #   expect(Gitlab::Tracking).to receive(:event).with("Category", "action", ...)
+      # end
+      #
+      # # good
+      # it 'expects a snowplow event', :snowplow do
+      #   expect_snowplow_event(category: "Category", action: "action", ...)
+      # end
+      #
+      # # bad
+      # it 'does not expect a snowplow event' do
+      #   expect(Gitlab::Tracking).not_to receive(:event)
+      # end
+      #
+      # # good
+      # it 'does not expect a snowplow event', :snowplow do
+      #   expect_no_snowplow_event
+      # end
+      class ExpectGitlabTracking < RuboCop::Cop::Cop
+        MSG = 'Do not expect directly on `Gitlab::Tracking#event`, add the `snowplow` annotation and use ' \
+              '`expect_snowplow_event` instead. ' \
+              'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#test-snowplow-events'
+
+        def_node_matcher :expect_gitlab_tracking?, <<~PATTERN
+          (send
+            (send nil? {:expect :allow}
+              (const (const nil? :Gitlab) :Tracking)
+            )
+            ${:to :to_not :not_to}
+            {
+              (
+                send nil? {:receive :have_received} (sym :event) ...
+              )
+
+              (send
+                (send nil? {:receive :have_received} (sym :event)) ...
+              )
+            }
+            ...
+          )
+        PATTERN
+
+        RESTRICT_ON_SEND = [:expect, :allow].freeze
+
+        def on_send(node)
+          return unless expect_gitlab_tracking?(node)
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/rspec/factory_bot/inline_association.rb b/rubocop/cop/rspec/factory_bot/inline_association.rb
new file mode 100644
index 00000000000..1c2b8b55b46
--- /dev/null
+++ b/rubocop/cop/rspec/factory_bot/inline_association.rb
@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      module FactoryBot
+        # This cop encourages the use of inline associations in FactoryBot.
+        # The explicit use of `create` and `build` is discouraged.
+        #
+        # See https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition
+        #
+        # @example
+        #
+        # Context:
+        #
+        #   Factory.define do
+        #     factory :project, class: 'Project'
+        #       # EXAMPLE below
+        #     end
+        #   end
+        #
+        # # bad
+        # creator { create(:user) }
+        # creator { create(:user, :admin) }
+        # creator { build(:user) }
+        # creator { FactoryBot.build(:user) }
+        # creator { ::FactoryBot.build(:user) }
+        # add_attribute(:creator) { build(:user) }
+        #
+        # # good
+        # creator { association(:user) }
+        # creator { association(:user, :admin) }
+        # add_attribute(:creator) { association(:user) }
+        #
+        # # Accepted
+        # after(:build) do |instance|
+        #   instance.creator = create(:user)
+        # end
+        #
+        # initialize_with do
+        #   create(:project)
+        # end
+        #
+        # creator_id { create(:user).id }
+        #
+        class InlineAssociation < RuboCop::Cop::Cop
+          MSG = 'Prefer inline `association` over `%{type}`. ' \
+            'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories'
+
+          REPLACEMENT = 'association'
+
+          def_node_matcher :create_or_build, <<~PATTERN
+            (
+              send
+              ${ nil? (const { nil? (cbase) } :FactoryBot) }
+              ${ :create :build }
+              (sym _)
+              ...
+            )
+          PATTERN
+
+          def_node_matcher :association_definition, <<~PATTERN
+            (block
+              {
+                (send nil? $_)
+                (send nil? :add_attribute (sym $_))
+              }
+              ...
+            )
+          PATTERN
+
+          def_node_matcher :chained_call?, <<~PATTERN
+            (send _ _)
+          PATTERN
+
+          SKIP_NAMES = %i[initialize_with].to_set.freeze
+
+          def on_send(node)
+            _receiver, type = create_or_build(node)
+            return unless type
+            return if chained_call?(node.parent)
+            return unless inside_assocation_definition?(node)
+
+            add_offense(node, message: format(MSG, type: type))
+          end
+
+          def autocorrect(node)
+            lambda do |corrector|
+              receiver, type = create_or_build(node)
+              receiver = "#{receiver.source}." if receiver
+              expression = "#{receiver}#{type}"
+              replacement = node.source.sub(expression, REPLACEMENT)
+              corrector.replace(node.source_range, replacement)
+            end
+          end
+
+          private
+
+          def inside_assocation_definition?(node)
+            node.each_ancestor(:block).any? do |parent|
+              name = association_definition(parent)
+              name && !SKIP_NAMES.include?(name)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/rspec/timecop_travel.rb b/rubocop/cop/rspec/timecop_travel.rb
new file mode 100644
index 00000000000..e5416953af7
--- /dev/null
+++ b/rubocop/cop/rspec/timecop_travel.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for `Timecop.travel` usage in specs.
+      #
+      # @example
+      #
+      #   # bad
+      #   Timecop.travel(1.day.ago) { create(:issue) }
+      #
+      #   # good
+      #   travel_to(1.day.ago) { create(:issue) }
+      #
+      class TimecopTravel < RuboCop::Cop::Cop
+        include MatchRange
+        MESSAGE = 'Do not use `Timecop.travel`, use `travel_to` instead. ' \
+                  'See https://gitlab.com/gitlab-org/gitlab/-/issues/214432 for more info.'
+
+        def_node_matcher :timecop_travel?, <<~PATTERN
+          (send (const nil? :Timecop) :travel _)
+        PATTERN
+
+        def on_send(node)
+          return unless timecop_travel?(node)
+
+          add_offense(node, location: :expression, message: MESSAGE)
+        end
+
+        def autocorrect(node)
+          -> (corrector) do
+            each_match_range(node.source_range, /^(Timecop\.travel)/) do |match_range|
+              corrector.replace(match_range, 'travel_to')
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb
index c26ba88f269..e9533fb65b2 100644
--- a/rubocop/migration_helpers.rb
+++ b/rubocop/migration_helpers.rb
@@ -21,7 +21,7 @@ module RuboCop
     TABLE_METHODS = %i(create_table create_table_if_not_exists change_table).freeze
 
     def high_traffic_tables
-      @high_traffic_tables ||= rubocop_migrations_config.dig('Migration/UpdateLargeTable', 'DeniedTables')
+      @high_traffic_tables ||= rubocop_migrations_config.dig('Migration/UpdateLargeTable', 'HighTrafficTables')
     end
 
     # Returns true if the given node originated from the db/migrate directory.
diff --git a/rubocop/rubocop-migrations.yml b/rubocop/rubocop-migrations.yml
index a919d570ccc..f8ff6e005f9 100644
--- a/rubocop/rubocop-migrations.yml
+++ b/rubocop/rubocop-migrations.yml
@@ -1,6 +1,7 @@
+# Make sure to update the docs if this file moves. Docs URL: https://docs.gitlab.com/ee/development/migration_style_guide.html#when-to-use-the-helper-method
 Migration/UpdateLargeTable:
   Enabled: true
-  DeniedTables: &denied_tables # size in GB (>= 10 GB on GitLab.com as of 02/2020) and/or number of records
+  HighTrafficTables: &high_traffic_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
diff --git a/rubocop/rubocop-usage-data.yml b/rubocop/rubocop-usage-data.yml
index 9f594bc5817..fdfe2a5e1aa 100644
--- a/rubocop/rubocop-usage-data.yml
+++ b/rubocop/rubocop-usage-data.yml
@@ -25,6 +25,8 @@ UsageData/LargeTable:
     - :Time
     - :SECURE_PRODUCT_TYPES
     - :Settings
+    - :CE_MEMOIZED_VALUES
+    - :EE_MEMOIZED_VALUES
   CountMethods:
     - :count
     - :distinct_count