Add latest changes from gitlab-org/gitlab@master

6 files changed, 273 insertions, 59 deletions

diff --git a/rubocop/cop/migration/add_limit_to_string_columns.rb b/rubocop/cop/migration/add_limit_to_string_columns.rb
deleted file mode 100644
index 30affcbb089..00000000000
--- a/rubocop/cop/migration/add_limit_to_string_columns.rb
+++ /dev/null
@@ -1,59 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../migration_helpers'
-
-module RuboCop
-  module Cop
-    module Migration
-      # Cop that enforces length constraints to string columns
-      class AddLimitToStringColumns < RuboCop::Cop::Cop
-        include MigrationHelpers
-
-        ADD_COLUMNS_METHODS = %i(add_column add_column_with_default).freeze
-
-        MSG = 'String columns should have a limit constraint. 255 is suggested'.freeze
-
-        def on_def(node)
-          return unless in_migration?(node)
-
-          node.each_descendant(:send) do |send_node|
-            next unless string_operation?(send_node)
-
-            add_offense(send_node, location: :selector) unless limit_on_string_column?(send_node)
-          end
-        end
-
-        private
-
-        def string_operation?(node)
-          modifier = node.children[0]
-          migration_method = node.children[1]
-
-          if migration_method == :string
-            modifier.type == :lvar
-          elsif ADD_COLUMNS_METHODS.include?(migration_method)
-            modifier.nil? && string_column?(node.children[4])
-          end
-        end
-
-        def string_column?(column_type)
-          column_type.type == :sym && column_type.value == :string
-        end
-
-        def limit_on_string_column?(node)
-          migration_method = node.children[1]
-
-          if migration_method == :string
-            limit_present?(node.children)
-          elsif ADD_COLUMNS_METHODS.include?(migration_method)
-            limit_present?(node)
-          end
-        end
-
-        def limit_present?(statement)
-          !(statement.to_s =~ /:limit/).nil?
-        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
new file mode 100644
index 00000000000..15c28bb9266
--- /dev/null
+++ b/rubocop/cop/migration/add_limit_to_text_columns.rb
@@ -0,0 +1,121 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that enforces always adding a limit on text columns
+      class AddLimitToTextColumns < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Text columns should always have a limit set (255 is suggested). ' \
+          'You can add a limit to a `text` column by using `add_text_limit`'.freeze
+
+        def_node_matcher :reverting?, <<~PATTERN
+          (def :down ...)
+        PATTERN
+
+        def_node_matcher :add_text_limit?, <<~PATTERN
+          (send _ :add_text_limit ...)
+        PATTERN
+
+        def on_def(node)
+          return unless in_migration?(node)
+
+          # Don't enforce the rule when on down to keep consistency with existing schema
+          return if reverting?(node)
+
+          node.each_descendant(:send) do |send_node|
+            next unless text_operation?(send_node)
+
+            # We require a limit for the same table and attribute name
+            if text_limit_missing?(node, *table_and_attribute_name(send_node))
+              add_offense(send_node, location: :selector)
+            end
+          end
+        end
+
+        private
+
+        def text_operation?(node)
+          modifier = node.children[0]
+          migration_method = node.children[1]
+
+          if migration_method == :text
+            modifier.type == :lvar
+          elsif ADD_COLUMN_METHODS.include?(migration_method)
+            modifier.nil? && text_column?(node.children[4])
+          end
+        end
+
+        def text_column?(column_type)
+          column_type.type == :sym && column_type.value == :text
+        end
+
+        # For a given node, find the table and attribute this node is for
+        #
+        # Simple when we have calls to `add_column_XXX` helper methods
+        #
+        # A little bit more tricky when we have attributes defined as part of
+        # a create/change table block:
+        # - The attribute name is available on the node
+        # - Finding the table name requires to:
+        #   * go up
+        #   * find the first block the attribute def is part of
+        #   * go back down to find the create_table node
+        #   * fetch the table name from that node
+        def table_and_attribute_name(node)
+          migration_method = node.children[1]
+          table_name, attribute_name = ''
+
+          if migration_method == :text
+            # We are inside a node in a create/change table block
+            block_node = node.each_ancestor(:block).first
+            create_table_node = block_node
+                                  .children
+                                  .find { |n| TABLE_METHODS.include?(n.children[1])}
+
+            if create_table_node
+              table_name = create_table_node.children[2].value
+            else
+              # Guard against errors when a new table create/change migration
+              # helper is introduced and warn the author so that it can be
+              # added in TABLE_METHODS
+              table_name = 'unknown'
+              add_offense(block_node, message: 'Unknown table create/change helper')
+            end
+
+            attribute_name = node.children[2].value
+          else
+            # We are in a node for one of the ADD_COLUMN_METHODS
+            table_name = node.children[2].value
+            attribute_name = node.children[3].value
+          end
+
+          [table_name, attribute_name]
+        end
+
+        # 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)
+          limit_found = false
+
+          node.each_descendant(:send) do |send_node|
+            next unless add_text_limit?(send_node)
+
+            limit_table = send_node.children[2].value
+            limit_attribute = send_node.children[3].value
+
+            if limit_table == table_name && limit_attribute == attribute_name
+              limit_found = true
+              break
+            end
+          end
+
+          !limit_found
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/prevent_strings.rb b/rubocop/cop/migration/prevent_strings.rb
new file mode 100644
index 00000000000..25c00194698
--- /dev/null
+++ b/rubocop/cop/migration/prevent_strings.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that enforces using text instead of the string data type
+      class PreventStrings < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Do not use the `string` data type, use `text` instead. ' \
+          'Updating limits on strings requires downtime. This can be avoided ' \
+          'by using `text` and adding a limit with `add_text_limit`'.freeze
+
+        def_node_matcher :reverting?, <<~PATTERN
+          (def :down ...)
+        PATTERN
+
+        def on_def(node)
+          return unless in_migration?(node)
+
+          # Don't enforce the rule when on down to keep consistency with existing schema
+          return if reverting?(node)
+
+          node.each_descendant(:send) do |send_node|
+            next unless string_operation?(send_node)
+
+            add_offense(send_node, location: :selector)
+          end
+        end
+
+        private
+
+        def string_operation?(node)
+          modifier = node.children[0]
+          migration_method = node.children[1]
+
+          if migration_method == :string
+            modifier.type == :lvar
+          elsif ADD_COLUMN_METHODS.include?(migration_method)
+            modifier.nil? && string_column?(node.children[4])
+          end
+        end
+
+        def string_column?(column_type)
+          column_type.type == :sym && column_type.value == :string
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/rspec/modify_sidekiq_middleware.rb b/rubocop/cop/rspec/modify_sidekiq_middleware.rb
new file mode 100644
index 00000000000..c38f074eb3a
--- /dev/null
+++ b/rubocop/cop/rspec/modify_sidekiq_middleware.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for `Sidekiq::Testing.server_middleware`
+      # usage in specs.
+      #
+      # @example
+      #
+      #   # bad
+      #   Sidekiq::Testing.server_middleware do |chain|
+      #     chain.add(MyMiddlewareUnderTest)
+      #   end
+      #
+      #
+      #   # good
+      #   with_custom_sidekiq_middleware do |chain|
+      #     chain.add(MyMiddlewareUnderTest)
+      #   end
+      #
+      #
+      class ModifySidekiqMiddleware < RuboCop::Cop::Cop
+        MSG = <<~MSG
+          Don't modify global sidekiq middleware, use the `#with_sidekiq_server_middleware`
+          helper instead
+        MSG
+
+        def_node_search :modifies_sidekiq_middleware?, <<~PATTERN
+          (send
+            (const
+              (const nil? :Sidekiq) :Testing) :server_middleware)
+        PATTERN
+
+        def on_send(node)
+          return unless modifies_sidekiq_middleware?(node)
+
+          add_offense(node, location: :expression)
+        end
+
+        def autocorrect(node)
+          -> (corrector) do
+            corrector.replace(node.loc.expression,
+                              'with_sidekiq_server_middleware')
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/static_translation_definition.rb b/rubocop/cop/static_translation_definition.rb
new file mode 100644
index 00000000000..736d8767342
--- /dev/null
+++ b/rubocop/cop/static_translation_definition.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    class StaticTranslationDefinition < RuboCop::Cop::Cop
+      MSG = "The text you're translating will be already in the translated form when it's assigned to the constant. When a users changes the locale, these texts won't be translated again. Consider moving the translation logic to a method.".freeze
+
+      TRANSLATION_METHODS = %i[_ s_ n_].freeze
+
+      def_node_matcher :translation_method?, <<~PATTERN
+      (send _ _ str*)
+      PATTERN
+
+      def_node_matcher :lambda_node?, <<~PATTERN
+      (send _ :lambda)
+      PATTERN
+
+      def on_send(node)
+        return unless translation_method?(node)
+
+        method_name = node.children[1]
+        return unless TRANSLATION_METHODS.include?(method_name)
+
+        node.each_ancestor do |ancestor|
+          receiver, _ = *ancestor
+          break if lambda_node?(receiver) # translations defined in lambda nodes should be allowed
+
+          if constant_assignment?(ancestor)
+            add_offense(node, location: :expression)
+
+            break
+          end
+        end
+      end
+
+      private
+
+      def constant_assignment?(node)
+        node.type == :casgn
+      end
+    end
+  end
+end
diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb
index 5fa6f8c2a2c..de377b15cc8 100644
--- a/rubocop/migration_helpers.rb
+++ b/rubocop/migration_helpers.rb
@@ -32,6 +32,7 @@ module RuboCop
        project_authorizations
        projects
        project_ci_cd_settings
+       project_features
        push_event_payloads
        resource_label_events
        routes
@@ -53,6 +54,12 @@ module RuboCop
       ci_builds
     ].freeze
 
+    # List of helpers that add new columns, either directly (ADD_COLUMN_METHODS)
+    # or through a create/alter table (TABLE_METHODS)
+    ADD_COLUMN_METHODS = %i(add_column add_column_with_default change_column_type_concurrently).freeze
+
+    TABLE_METHODS = %i(create_table create_table_if_not_exists change_table).freeze
+
     # 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)