Commit 4bb269ee authored by Maxime Orefice's avatar Maxime Orefice Committed by Matthias Käppler

Extend PreventIndexCreation cop

This commit extends our existing cop to catch false positives
when the table_name is a string or a constant.
parent c6b4adcd
......@@ -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"
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 @@ module RuboCop
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
......
......@@ -15,22 +15,63 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
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 @@ RSpec.describe RuboCop::Cop::Migration::PreventIndexCreation do
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
......
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