Commit dc12dc94 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch 'mo-extend-index-creation-prevention-cop' into 'master'

Extend PreventIndexCreation cop

See merge request gitlab-org/gitlab!69247
parents 15f1b494 4bb269ee
...@@ -12,16 +12,29 @@ module RuboCop ...@@ -12,16 +12,29 @@ module RuboCop
MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886" 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 def_node_matcher :add_index?, <<~PATTERN
(send nil? :add_index (sym #forbidden_tables?) ...) (send nil? :add_index ({sym|str} #forbidden_tables?) ...)
PATTERN PATTERN
def_node_matcher :add_concurrent_index?, <<~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 PATTERN
def forbidden_tables?(node) def_node_matcher :forbidden_constant_defined?, <<~PATTERN
FORBIDDEN_TABLES.include?(node) (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 end
def on_def(node) def on_def(node)
...@@ -32,8 +45,18 @@ module RuboCop ...@@ -32,8 +45,18 @@ module RuboCop
end end
end end
private
def forbidden_tables?(node)
FORBIDDEN_TABLES.include?(node.to_sym)
end
def offense?(node) 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 end
end end
......
...@@ -15,11 +15,12 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do ...@@ -15,11 +15,12 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
end end
context 'when adding an index to a forbidden table' do context 'when adding an index to a forbidden table' do
context 'when table_name is a symbol' do
it "registers an offense when add_index is used", :aggregate_failures do it "registers an offense when add_index is used", :aggregate_failures do
forbidden_tables.each do |table| forbidden_tables.each do |table_name|
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
def change def change
add_index :#{table}, :protected add_index :#{table_name}, :protected
^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 ^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
end end
RUBY RUBY
...@@ -27,10 +28,50 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do ...@@ -27,10 +28,50 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
end end
it "registers an offense when add_concurrent_index is used", :aggregate_failures do it "registers an offense when add_concurrent_index is used", :aggregate_failures do
forbidden_tables.each do |table| forbidden_tables.each do |table_name|
expect_offense(<<~RUBY) expect_offense(<<~RUBY)
def change 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
end
end
end
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_NAME, :protected
^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886 ^^^^^^^^^^^^^^^^^^^^ Adding new index to #{forbidden_tables_list} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886
end end
RUBY RUBY
...@@ -46,6 +87,20 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do ...@@ -46,6 +87,20 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
end end
RUBY RUBY
end 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
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