18 files changed, 587 insertions, 13 deletions

diff --git a/rubocop/cop/active_record_dependent.rb b/rubocop/cop/active_record_dependent.rb
new file mode 100644
index 00000000000..8d15f150885
--- /dev/null
+++ b/rubocop/cop/active_record_dependent.rb
@@ -0,0 +1,26 @@
+require_relative '../model_helpers'
+
+module RuboCop
+  module Cop
+    # Cop that prevents the use of `dependent: ...` in ActiveRecord models.
+    class ActiveRecordDependent < RuboCop::Cop::Cop
+      include ModelHelpers
+
+      MSG = 'Do not use `dependent: to remove associated data, ' \
+        'use foreign keys with cascading deletes instead'.freeze
+
+      METHOD_NAMES = [:has_many, :has_one, :belongs_to].freeze
+
+      def on_send(node)
+        return unless in_model?(node)
+        return unless METHOD_NAMES.include?(node.children[1])
+
+        node.children.last.each_node(:pair) do |pair|
+          key_name = pair.children[0].children[0]
+
+          add_offense(pair, :expression) if key_name == :dependent
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/active_record_serialize.rb b/rubocop/cop/active_record_serialize.rb
new file mode 100644
index 00000000000..204caf37f8b
--- /dev/null
+++ b/rubocop/cop/active_record_serialize.rb
@@ -0,0 +1,18 @@
+require_relative '../model_helpers'
+
+module RuboCop
+  module Cop
+    # Cop that prevents the use of `serialize` in ActiveRecord models.
+    class ActiveRecordSerialize < RuboCop::Cop::Cop
+      include ModelHelpers
+
+      MSG = 'Do not store serialized data in the database, use separate columns and/or tables instead'.freeze
+
+      def on_send(node)
+        return unless in_model?(node)
+
+        add_offense(node, :selector) if node.children[1] == :serialize
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/in_batches.rb b/rubocop/cop/in_batches.rb
new file mode 100644
index 00000000000..c0240187e66
--- /dev/null
+++ b/rubocop/cop/in_batches.rb
@@ -0,0 +1,16 @@
+require_relative '../model_helpers'
+
+module RuboCop
+  module Cop
+    # Cop that prevents the use of `in_batches`
+    class InBatches < RuboCop::Cop::Cop
+      MSG = 'Do not use `in_batches`, use `each_batch` from the EachBatch module instead'.freeze
+
+      def on_send(node)
+        return unless node.children[1] == :in_batches
+
+        add_offense(node, :selector)
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/add_column_with_default_to_large_table.rb b/rubocop/cop/migration/add_column_with_default_to_large_table.rb
new file mode 100644
index 00000000000..fb363f95b56
--- /dev/null
+++ b/rubocop/cop/migration/add_column_with_default_to_large_table.rb
@@ -0,0 +1,55 @@
+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 AddColumnWithDefaultToLargeTable < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Using `add_column_with_default` on the `%s` table will take a ' \
+          'long time to complete, and should be avoided unless absolutely ' \
+          'necessary'.freeze
+
+        LARGE_TABLES = %i[
+          ci_pipelines
+          ci_builds
+          events
+          issues
+          merge_request_diff_files
+          merge_request_diffs
+          merge_requests
+          namespaces
+          notes
+          projects
+          routes
+          users
+        ].freeze
+
+        def_node_matcher :add_column_with_default?, <<~PATTERN
+          (send nil :add_column_with_default $(sym ...) ...)
+        PATTERN
+
+        def on_send(node)
+          return unless in_migration?(node)
+
+          matched = add_column_with_default?(node)
+          return unless matched
+
+          table = matched.to_a.first
+          return unless LARGE_TABLES.include?(table)
+
+          add_offense(node, :expression, format(MSG, table))
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/add_timestamps.rb b/rubocop/cop/migration/add_timestamps.rb
new file mode 100644
index 00000000000..08ddd91e54d
--- /dev/null
+++ b/rubocop/cop/migration/add_timestamps.rb
@@ -0,0 +1,25 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if 'add_timestamps' method is called with timezone information.
+      class AddTimestamps < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead'.freeze
+
+        # Check methods.
+        def on_send(node)
+          return unless in_migration?(node)
+
+          add_offense(node, :selector) if method_name(node) == :add_timestamps
+        end
+
+        def method_name(node)
+          node.children[1]
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/datetime.rb b/rubocop/cop/migration/datetime.rb
new file mode 100644
index 00000000000..651935dd53e
--- /dev/null
+++ b/rubocop/cop/migration/datetime.rb
@@ -0,0 +1,36 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if datetime data type is added with timezone information.
+      class Datetime < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Do not use the `datetime` data type, use `datetime_with_timezone` instead'.freeze
+
+        # Check methods in table creation.
+        def on_def(node)
+          return unless in_migration?(node)
+
+          node.each_descendant(:send) do |send_node|
+            add_offense(send_node, :selector) if method_name(send_node) == :datetime
+          end
+        end
+
+        # Check methods.
+        def on_send(node)
+          return unless in_migration?(node)
+
+          node.each_descendant do |descendant|
+            add_offense(node, :expression) if descendant.type == :sym && descendant.children.last == :datetime
+          end
+        end
+
+        def method_name(node)
+          node.children[1]
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/hash_index.rb b/rubocop/cop/migration/hash_index.rb
new file mode 100644
index 00000000000..2cc59691d84
--- /dev/null
+++ b/rubocop/cop/migration/hash_index.rb
@@ -0,0 +1,51 @@
+require 'set'
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that prevents the use of hash indexes in database migrations
+      class HashIndex < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'hash indexes should be avoided at all costs since they are not ' \
+          'recorded in the PostgreSQL WAL, you should use a btree index instead'.freeze
+
+        NAMES = Set.new([:add_index, :index, :add_concurrent_index]).freeze
+
+        def on_send(node)
+          return unless in_migration?(node)
+
+          name = node.children[1]
+
+          return unless NAMES.include?(name)
+
+          opts = node.children.last
+
+          return unless opts && opts.type == :hash
+
+          opts.each_node(:pair) do |pair|
+            next unless hash_key_type(pair) == :sym &&
+                hash_key_name(pair) == :using
+
+            if hash_key_value(pair).to_s == 'hash'
+              add_offense(pair, :expression)
+            end
+          end
+        end
+
+        def hash_key_type(pair)
+          pair.children[0].type
+        end
+
+        def hash_key_name(pair)
+          pair.children[0].children[0]
+        end
+
+        def hash_key_value(pair)
+          pair.children[1].children[0]
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/reversible_add_column_with_default.rb
index 54a920d4b49..f413f06f39b 100644
--- a/rubocop/cop/migration/add_column_with_default.rb
+++ b/rubocop/cop/migration/reversible_add_column_with_default.rb
@@ -5,29 +5,30 @@ module RuboCop
     module Migration
       # Cop that checks if `add_column_with_default` is used with `up`/`down` methods
       # and not `change`.
-      class AddColumnWithDefault < RuboCop::Cop::Cop
+      class ReversibleAddColumnWithDefault < RuboCop::Cop::Cop
         include MigrationHelpers
 
+        def_node_matcher :add_column_with_default?, <<~PATTERN
+          (send nil :add_column_with_default $...)
+        PATTERN
+
+        def_node_matcher :defines_change?, <<~PATTERN
+          (def :change ...)
+        PATTERN
+
         MSG = '`add_column_with_default` is not reversible so you must manually define ' \
           'the `up` and `down` methods in your migration class, using `remove_column` in `down`'.freeze
 
         def on_send(node)
           return unless in_migration?(node)
-
-          name = node.children[1]
-
-          return unless name == :add_column_with_default
+          return unless add_column_with_default?(node)
 
           node.each_ancestor(:def) do |def_node|
-            next unless method_name(def_node) == :change
+            next unless defines_change?(def_node)
 
             add_offense(def_node, :name)
           end
         end
-
-        def method_name(node)
-          node.children.first
-        end
       end
     end
   end
diff --git a/rubocop/cop/migration/safer_boolean_column.rb b/rubocop/cop/migration/safer_boolean_column.rb
new file mode 100644
index 00000000000..0335c25d85d
--- /dev/null
+++ b/rubocop/cop/migration/safer_boolean_column.rb
@@ -0,0 +1,94 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # This cop requires a default value and disallows nulls for boolean
+      # columns on small tables.
+      #
+      # In general, this prevents 3-state-booleans.
+      # https://robots.thoughtbot.com/avoid-the-threestate-boolean-problem
+      #
+      # In particular, for the `application_settings` table, this ensures that
+      # upgraded installations get a proper default for the new boolean setting.
+      # A developer might otherwise mistakenly assume that a value in
+      # `ApplicationSetting.defaults` is sufficient.
+      #
+      # See https://gitlab.com/gitlab-org/gitlab-ee/issues/2750 for more
+      # information.
+      class SaferBooleanColumn < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default. You may wish to use `add_column_with_default`.'.freeze
+        NULL_OFFENSE = 'Boolean columns on the `%s` table should disallow nulls.'.freeze
+        DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls. You may wish to use `add_column_with_default`.'.freeze
+
+        SMALL_TABLES = %i[
+          application_settings
+        ].freeze
+
+        def_node_matcher :add_column?, <<~PATTERN
+          (send nil :add_column $...)
+        PATTERN
+
+        def on_send(node)
+          return unless in_migration?(node)
+
+          matched = add_column?(node)
+
+          return unless matched
+
+          table, _, type = matched.to_a.take(3).map(&:children).map(&:first)
+          opts = matched[3]
+
+          return unless SMALL_TABLES.include?(table) && type == :boolean
+
+          no_default = no_default?(opts)
+          nulls_allowed = nulls_allowed?(opts)
+
+          offense = if no_default && nulls_allowed
+                      DEFAULT_AND_NULL_OFFENSE
+                    elsif no_default
+                      DEFAULT_OFFENSE
+                    elsif nulls_allowed
+                      NULL_OFFENSE
+                    end
+
+          add_offense(node, :expression, format(offense, table)) if offense
+        end
+
+        def no_default?(opts)
+          return true unless opts
+
+          each_hash_node_pair(opts) do |key, value|
+            return value == 'nil' if key == :default
+          end
+        end
+
+        def nulls_allowed?(opts)
+          return true unless opts
+
+          each_hash_node_pair(opts) do |key, value|
+            return value != 'false' if key == :null
+          end
+        end
+
+        def each_hash_node_pair(hash_node, &block)
+          hash_node.each_node(:pair) do |pair|
+            key = hash_pair_key(pair)
+            value = hash_pair_value(pair)
+            yield(key, value)
+          end
+        end
+
+        def hash_pair_key(pair)
+          pair.children[0].children[0]
+        end
+
+        def hash_pair_value(pair)
+          pair.children[1].source
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/timestamps.rb b/rubocop/cop/migration/timestamps.rb
new file mode 100644
index 00000000000..71a9420cc3b
--- /dev/null
+++ b/rubocop/cop/migration/timestamps.rb
@@ -0,0 +1,27 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if 'timestamps' method is called with timezone information.
+      class Timestamps < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Do not use `timestamps`, use `timestamps_with_timezone` instead'.freeze
+
+        # Check methods in table creation.
+        def on_def(node)
+          return unless in_migration?(node)
+
+          node.each_descendant(:send) do |send_node|
+            add_offense(send_node, :selector) if method_name(send_node) == :timestamps
+          end
+        end
+
+        def method_name(node)
+          node.children[1]
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/migration/update_column_in_batches.rb b/rubocop/cop/migration/update_column_in_batches.rb
new file mode 100644
index 00000000000..3f886cbfea3
--- /dev/null
+++ b/rubocop/cop/migration/update_column_in_batches.rb
@@ -0,0 +1,43 @@
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      # Cop that checks if a spec file exists for any migration using
+      # `update_column_in_batches`.
+      class UpdateColumnInBatches < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'Migration running `update_column_in_batches` must have a spec file at' \
+          ' `%s`.'.freeze
+
+        def on_send(node)
+          return unless in_migration?(node)
+          return unless node.children[1] == :update_column_in_batches
+
+          spec_path = spec_filename(node)
+
+          unless File.exist?(File.expand_path(spec_path, rails_root))
+            add_offense(node, :expression, format(MSG, spec_path))
+          end
+        end
+
+        private
+
+        def spec_filename(node)
+          source_name = node.location.expression.source_buffer.name
+          path = Pathname.new(source_name).relative_path_from(rails_root)
+          dirname = File.dirname(path)
+            .sub(%r{\Adb/(migrate|post_migrate)}, 'spec/migrations')
+          filename = File.basename(source_name, '.rb').sub(%r{\A\d+_}, '')
+
+          File.join(dirname, "#{filename}_spec.rb")
+        end
+
+        def rails_root
+          Pathname.new(File.expand_path('../../..', __dir__))
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/polymorphic_associations.rb b/rubocop/cop/polymorphic_associations.rb
new file mode 100644
index 00000000000..7d554704550
--- /dev/null
+++ b/rubocop/cop/polymorphic_associations.rb
@@ -0,0 +1,23 @@
+require_relative '../model_helpers'
+
+module RuboCop
+  module Cop
+    # Cop that prevents the use of polymorphic associations
+    class PolymorphicAssociations < RuboCop::Cop::Cop
+      include ModelHelpers
+
+      MSG = 'Do not use polymorphic associations, use separate tables instead'.freeze
+
+      def on_send(node)
+        return unless in_model?(node)
+        return unless node.children[1] == :belongs_to
+
+        node.children.last.each_node(:pair) do |pair|
+          key_name = pair.children[0].children[0]
+
+          add_offense(pair, :expression) if key_name == :polymorphic
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/project_path_helper.rb b/rubocop/cop/project_path_helper.rb
new file mode 100644
index 00000000000..3e1ce71ac06
--- /dev/null
+++ b/rubocop/cop/project_path_helper.rb
@@ -0,0 +1,51 @@
+module RuboCop
+  module Cop
+    class ProjectPathHelper < RuboCop::Cop::Cop
+      MSG = 'Use short project path helpers without explicitly passing the namespace: ' \
+        '`foo_project_bar_path(project, bar)` instead of ' \
+        '`foo_namespace_project_bar_path(project.namespace, project, bar)`.'.freeze
+
+      METHOD_NAME_PATTERN = /\A([a-z_]+_)?namespace_project(?:_[a-z_]+)?_(?:url|path)\z/.freeze
+
+      def on_send(node)
+        return unless method_name(node).to_s =~ METHOD_NAME_PATTERN
+
+        namespace_expr, project_expr = arguments(node)
+        return unless namespace_expr && project_expr
+
+        return unless namespace_expr.type == :send
+        return unless method_name(namespace_expr) == :namespace
+        return unless receiver(namespace_expr) == project_expr
+
+        add_offense(node, :selector)
+      end
+
+      def autocorrect(node)
+        helper_name = method_name(node).to_s.sub('namespace_project', 'project')
+
+        arguments = arguments(node)
+        arguments.shift # Remove namespace argument
+
+        replacement = "#{helper_name}(#{arguments.map(&:source).join(', ')})"
+
+        lambda do |corrector|
+          corrector.replace(node.source_range, replacement)
+        end
+      end
+
+      private
+
+      def receiver(node)
+        node.children[0]
+      end
+
+      def method_name(node)
+        node.children[1]
+      end
+
+      def arguments(node)
+        node.children[2..-1]
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/redirect_with_status.rb b/rubocop/cop/redirect_with_status.rb
new file mode 100644
index 00000000000..36810642c88
--- /dev/null
+++ b/rubocop/cop/redirect_with_status.rb
@@ -0,0 +1,44 @@
+module RuboCop
+  module Cop
+    # This cop prevents usage of 'redirect_to' in actions 'destroy' without specifying 'status'.
+    # See https://gitlab.com/gitlab-org/gitlab-ce/issues/31840
+    class RedirectWithStatus < RuboCop::Cop::Cop
+      MSG = 'Do not use "redirect_to" without "status" in "destroy" action'.freeze
+
+      def on_def(node)
+        return unless in_controller?(node)
+        return unless destroy?(node) || destroy_all?(node)
+
+        node.each_descendant(:send) do |def_node|
+          next unless redirect_to?(def_node)
+
+          methods = []
+
+          def_node.children.last.each_node(:pair) do |pair|
+            methods << pair.children.first.children.first
+          end
+
+          add_offense(def_node, :selector) unless methods.include?(:status)
+        end
+      end
+
+      private
+
+      def in_controller?(node)
+        node.location.expression.source_buffer.name.end_with?('_controller.rb')
+      end
+
+      def destroy?(node)
+        node.children.first == :destroy
+      end
+
+      def destroy_all?(node)
+        node.children.first == :destroy_all
+      end
+
+      def redirect_to?(node)
+        node.children[1] == :redirect_to
+      end
+    end
+  end
+end
diff --git a/rubocop/cop/rspec/single_line_hook.rb b/rubocop/cop/rspec/single_line_hook.rb
new file mode 100644
index 00000000000..be611054323
--- /dev/null
+++ b/rubocop/cop/rspec/single_line_hook.rb
@@ -0,0 +1,38 @@
+require 'rubocop-rspec'
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for single-line hook blocks
+      #
+      # @example
+      #
+      #   # bad
+      #   before { do_something }
+      #   after(:each) { undo_something }
+      #
+      #   # good
+      #   before do
+      #     do_something
+      #   end
+      #
+      #   after(:each) do
+      #     undo_something
+      #   end
+      class SingleLineHook < Cop
+        MESSAGE = "Don't use single-line hook blocks.".freeze
+
+        def_node_search :rspec_hook?, <<~PATTERN
+          (send nil {:after :around :before} ...)
+        PATTERN
+
+        def on_block(node)
+          return unless rspec_hook?(node)
+          return unless node.single_line?
+
+          add_offense(node, :expression, MESSAGE)
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb
index 3160a784a04..c3473771178 100644
--- a/rubocop/migration_helpers.rb
+++ b/rubocop/migration_helpers.rb
@@ -3,8 +3,9 @@ module RuboCop
   module MigrationHelpers
     # Returns true if the given node originated from the db/migrate directory.
     def in_migration?(node)
-      File.dirname(node.location.expression.source_buffer.name).
-        end_with?('db/migrate')
+      dirname = File.dirname(node.location.expression.source_buffer.name)
+
+      dirname.end_with?('db/migrate', 'db/post_migrate')
     end
   end
 end
diff --git a/rubocop/model_helpers.rb b/rubocop/model_helpers.rb
new file mode 100644
index 00000000000..309723dc34c
--- /dev/null
+++ b/rubocop/model_helpers.rb
@@ -0,0 +1,11 @@
+module RuboCop
+  module ModelHelpers
+    # Returns true if the given node originated from the models directory.
+    def in_model?(node)
+      path = node.location.expression.source_buffer.name
+      models_path = File.join(Dir.pwd, 'app', 'models')
+
+      path.start_with?(models_path)
+    end
+  end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index d580aa6857a..1b6e8991a17 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -1,9 +1,23 @@
 require_relative 'cop/custom_error_class'
 require_relative 'cop/gem_fetcher'
+require_relative 'cop/active_record_serialize'
+require_relative 'cop/redirect_with_status'
+require_relative 'cop/polymorphic_associations'
+require_relative 'cop/project_path_helper'
+require_relative 'cop/active_record_dependent'
+require_relative 'cop/in_batches'
 require_relative 'cop/migration/add_column'
-require_relative 'cop/migration/add_column_with_default'
+require_relative 'cop/migration/add_column_with_default_to_large_table'
 require_relative 'cop/migration/add_concurrent_foreign_key'
 require_relative 'cop/migration/add_concurrent_index'
 require_relative 'cop/migration/add_index'
+require_relative 'cop/migration/add_timestamps'
+require_relative 'cop/migration/datetime'
+require_relative 'cop/migration/safer_boolean_column'
+require_relative 'cop/migration/hash_index'
 require_relative 'cop/migration/remove_concurrent_index'
 require_relative 'cop/migration/remove_index'
+require_relative 'cop/migration/reversible_add_column_with_default'
+require_relative 'cop/migration/timestamps'
+require_relative 'cop/migration/update_column_in_batches'
+require_relative 'cop/rspec/single_line_hook'