Commit ad20fb49 authored by pbair's avatar pbair

Add cop to detect complex indexes without name

Add a new cop that detects instances where migrations create or remove
a complex indexes that do not specify an explicit index name.
parent 99c64100
......@@ -509,3 +509,8 @@ Cop/PutGroupRoutesUnderScope:
Include:
- 'config/routes/group.rb'
- 'ee/config/routes/group.rb'
Migration/ComplexIndexesRequireName:
Exclude:
- !ruby/regexp /\Adb\/(post_)?migrate\/201.*\.rb\z/
- !ruby/regexp /\Adb\/(post_)?migrate\/20200[1-7].*\.rb\z/
......@@ -15,10 +15,12 @@ class RemoveOldExternalDiffMigrationIndex < ActiveRecord::Migration[6.0]
end
def down
# rubocop:disable Migration/ComplexIndexesRequireName
add_concurrent_index(
:merge_request_diffs,
[:merge_request_id, :id],
where: { stored_externally: [nil, false] }
)
# rubocop:enable Migration/ComplexIndexesRequireName
end
end
......@@ -7,7 +7,9 @@ class AddUniqueIndexToProjectIdOnProjectRegistry < ActiveRecord::Migration[4.2]
def up
remove_concurrent_index :project_registry, :project_id if index_exists? :project_registry, :project_id
# rubocop:disable Migration/ComplexIndexesRequireName
add_concurrent_index :project_registry, :project_id, unique: true
# rubocop:enable Migration/ComplexIndexesRequireName
end
def down
......
# frozen_string_literal: true
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
class ComplexIndexesRequireName < RuboCop::Cop::Cop
include MigrationHelpers
MSG = 'indexes added with custom options must be explicitly named'
def_node_matcher :match_add_index_with_options, <<~PATTERN
(send _ {:add_concurrent_index} _ _ (hash $...))
PATTERN
def_node_matcher :name_option?, <<~PATTERN
(pair {(sym :name) (str "name")} _)
PATTERN
def on_def(node)
return unless in_migration?(node)
node.each_descendant(:send) do |send_node|
next unless add_index_offense?(send_node)
add_offense(send_node, location: :selector)
end
end
private
def add_index_offense?(send_node)
match_add_index_with_options(send_node) { |option_nodes| needs_name_option?(option_nodes) }
end
def needs_name_option?(option_nodes)
option_nodes.none? { |node| name_option?(node) }
end
end
end
end
end
# frozen_string_literal: true
#
require 'fast_spec_helper'
require 'rubocop'
require_relative '../../../../rubocop/cop/migration/complex_indexes_require_name'
RSpec.describe RuboCop::Cop::Migration::ComplexIndexesRequireName, type: :rubocop do
include CopHelper
subject(:cop) { described_class.new }
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
context 'when indexes are configured with an options hash, but no name' do
it 'registers an offense' do
expect_offense(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
INDEX_NAME = 'my_test_name'
disable_ddl_transaction!
def up
add_concurrent_index :test_indexes, :column1
add_concurrent_index :test_indexes, :column2, where: "column2 = 'value'", order: { column4: :desc }
^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
add_concurrent_index :test_indexes, :column3, where: 'column3 = 10', name: 'idx_equal_to_10'
end
def down
add_concurrent_index :test_indexes, :column4, 'unique' => true
^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
add_concurrent_index :test_indexes, :column5, using: :gin, name: INDEX_NAME
add_concurrent_index :test_indexes, :column6, using: :gin, opclass: :gin_trgm_ops
^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
end
end
RUBY
expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}"))
end
end
end
context 'outside migration' do
before do
allow(cop).to receive(:in_migration?).and_return(false)
end
it 'registers no offenses' do
expect_no_offenses(<<~RUBY)
class TestComplexIndexes < ActiveRecord::Migration[6.0]
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_index :test_indexes, :column1, where: "some_column = 'value'"
end
def down
add_concurrent_index :test_indexes, :column2, unique: true
end
end
RUBY
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment