diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb index 1486607acbe8ca14671d3ed9dc5ad866825018b2..aeeec36ecf0fcde1bcdf1872bdcaf077e9ac063d 100644 --- a/rubocop/cop/migration/prevent_index_creation.rb +++ b/rubocop/cop/migration/prevent_index_creation.rb @@ -12,16 +12,29 @@ class PreventIndexCreation < RuboCop::Cop::Cop MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886" + def on_new_investigation + super + @forbidden_tables_used = false + end + def_node_matcher :add_index?, <<~PATTERN - (send nil? :add_index (sym #forbidden_tables?) ...) + (send nil? :add_index ({sym|str} #forbidden_tables?) ...) PATTERN def_node_matcher :add_concurrent_index?, <<~PATTERN - (send nil? :add_concurrent_index (sym #forbidden_tables?) ...) + (send nil? :add_concurrent_index ({sym|str} #forbidden_tables?) ...) PATTERN - def forbidden_tables?(node) - FORBIDDEN_TABLES.include?(node) + def_node_matcher :forbidden_constant_defined?, <<~PATTERN + (casgn nil? _ ({sym|str} #forbidden_tables?)) + PATTERN + + def_node_matcher :add_concurrent_index_with_constant?, <<~PATTERN + (send nil? :add_concurrent_index (const nil? _) ...) + PATTERN + + def on_casgn(node) + @forbidden_tables_used = !!forbidden_constant_defined?(node) end def on_def(node) @@ -32,8 +45,18 @@ def on_def(node) end end + private + + def forbidden_tables?(node) + FORBIDDEN_TABLES.include?(node.to_sym) + end + def offense?(node) - add_index?(node) || add_concurrent_index?(node) + add_index?(node) || add_concurrent_index?(node) || any_constant_used_with_forbidden_tables?(node) + end + + def any_constant_used_with_forbidden_tables?(node) + add_concurrent_index_with_constant?(node) && @forbidden_tables_used end end end diff --git a/spec/rubocop/cop/migration/prevent_index_creation_spec.rb b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb index aa1779a8cd563776ed046ff66e444fad2051ef93..b5bb770553a3134526791f54e2dafdc3aabdee90 100644 --- a/spec/rubocop/cop/migration/prevent_index_creation_spec.rb +++ b/spec/rubocop/cop/migration/prevent_index_creation_spec.rb @@ -15,22 +15,63 @@ end context 'when adding an index to a forbidden table' do - it "registers an offense when add_index is used", :aggregate_failures do - forbidden_tables.each do |table| - expect_offense(<<~RUBY) - def change - add_index :#{table}, :protected - ^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 - end - RUBY + context 'when table_name is a symbol' do + it "registers an offense when add_index is used", :aggregate_failures do + forbidden_tables.each do |table_name| + expect_offense(<<~RUBY) + def change + add_index :#{table_name}, :protected + ^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 + end + RUBY + end + end + + it "registers an offense when add_concurrent_index is used", :aggregate_failures do + forbidden_tables.each do |table_name| + expect_offense(<<~RUBY) + def change + add_concurrent_index :#{table_name}, :protected + ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 + end + RUBY + end end end - it "registers an offense when add_concurrent_index is used", :aggregate_failures do - forbidden_tables.each do |table| + context 'when table_name is a string' do + it "registers an offense when add_index is used", :aggregate_failures do + forbidden_tables.each do |table_name| + expect_offense(<<~RUBY) + def change + add_index "#{table_name}", :protected + ^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 + end + RUBY + end + end + + it "registers an offense when add_concurrent_index is used", :aggregate_failures do + forbidden_tables.each do |table_name| + expect_offense(<<~RUBY) + def change + add_concurrent_index "#{table_name}", :protected + ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 + end + RUBY + end + end + end + + context 'when table_name is a constant' do + it "registers an offense when add_concurrent_index is used", :aggregate_failures do expect_offense(<<~RUBY) + INDEX_NAME = "index_name" + TABLE_NAME = :ci_builds + disable_ddl_transaction! + def change - add_concurrent_index :#{table}, :protected + add_concurrent_index TABLE_NAME, :protected ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 end RUBY @@ -46,6 +87,20 @@ def change end RUBY end + + context 'when using a constant' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + disable_ddl_transaction! + + TABLE_NAME = "not_forbidden" + + def up + add_concurrent_index TABLE_NAME, :protected + end + RUBY + end + end end end