Commit ffc97fe1 authored by Yannis Roussos's avatar Yannis Roussos

Exclude text arrays from Rubocop migration checks

- Add array_column? to RuboCop::MigrationHelpers
- Update AddLimitToTextColumns cop to not complain about text arrays
- Update PreventStrings cop to not complain about string arrays
- Add specs for string and text array columns defined in migrations
parent 5200e024
......@@ -4,7 +4,7 @@ class AddContainerRegistryFeaturesToApplicationSettings < ActiveRecord::Migratio
DOWNTIME = false
def up
add_column :application_settings, :container_registry_features, :text, array: true, default: [], null: false # rubocop:disable Migration/AddLimitToTextColumns
add_column :application_settings, :container_registry_features, :text, array: true, default: [], null: false
end
def down
......
......@@ -39,6 +39,9 @@ module RuboCop
private
def text_operation?(node)
# Don't complain about text arrays
return false if array_column?(node)
modifier = node.children[0]
migration_method = node.children[1]
......
......@@ -33,6 +33,9 @@ module RuboCop
private
def string_operation?(node)
# Don't complain about string arrays
return false if array_column?(node)
modifier = node.children[0]
migration_method = node.children[1]
......
......@@ -34,6 +34,16 @@ module RuboCop
File.basename(node.location.expression.source_buffer.name).split('_').first.to_i
end
# Returns true if a column definition is for an array
# rubocop:disable Lint/BooleanSymbol
def array_column?(node)
node.each_descendant(:pair).any? do |pair_node|
pair_node.child_nodes[0].value == :array && # Searching for a (pair (sym :array) (true)) node
pair_node.child_nodes[1].type == :true # RuboCop::AST::Node uses symbols for types, even when that is a :true
end
end
# rubocop:enable Lint/BooleanSymbol
private
def dirname(node)
......
......@@ -73,6 +73,27 @@ describe RuboCop::Cop::Migration::AddLimitToTextColumns do
end
end
context 'when text array columns are defined without a limit' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestTextLimits < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :test_text_limits, id: false do |t|
t.integer :test_id, null: false
t.text :name, array: true, default: [], null: false
end
add_column :test_text_limits, :email, :text, array: true
add_column_with_default :test_text_limits, :role, :text, default: [], array: true
change_column_type_concurrently :test_text_limits, :test_id, :text, array: true
end
end
RUBY
end
end
# Make sure that the cop is properly checking for an `add_text_limit`
# over the same {table, attribute} as the one that triggered the offence
context 'when the limit is defined for a same name attribute but different table' do
......
......@@ -90,6 +90,27 @@ describe RuboCop::Cop::Migration::PreventStrings do
end
end
context 'when the string data type is used for arrays' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
class TestStringArrays < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
create_table :test_string_arrays, id: false do |t|
t.integer :test_id, null: false
t.string :name, array: true, default: [], null: false
end
add_column :test_string_arrays, :email, :string, array: true
add_column_with_default :test_string_arrays, :role, :string, default: [], array: true
change_column_type_concurrently :test_string_arrays, :test_id, :string, array: true
end
end
RUBY
end
end
context 'on down' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
......
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