From 542fccd67e237e697c22a3cf18980f37879de2f0 Mon Sep 17 00:00:00 2001
From: David Fernandez <dfernandez@gitlab.com>
Date: Wed, 31 Jan 2024 14:56:48 +0100
Subject: [PATCH] Fix the Migration/AddLimitToTextColumns cop
when encountering a table name stored in a constant.
This avoids a rubocop crash.
---
.../migration/add_limit_to_text_columns.rb | 10 ++-
.../add_limit_to_text_columns_spec.rb | 72 +++++++++++++++++++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb
index 2d3b5d5094fc..d97c8d383121 100644
--- a/rubocop/cop/migration/add_limit_to_text_columns.rb
+++ b/rubocop/cop/migration/add_limit_to_text_columns.rb
@@ -105,7 +105,7 @@ def table_and_attribute_name(node)
.find { |n| TABLE_METHODS.include?(n.children[1]) }
if create_table_node
- table_name = create_table_node.children[2].value
+ table_name = table_name_or_const_name(create_table_node.children[2])
else
# Guard against errors when a new table create/change migration
# helper is introduced and warn the author so that it can be
@@ -117,7 +117,7 @@ def table_and_attribute_name(node)
attribute_name = node.children[2].value
else
# We are in a node for one of the ADD_COLUMN_METHODS
- table_name = node.children[2].value
+ table_name = table_name_or_const_name(node.children[2])
attribute_name = node.children[3].value
end
@@ -151,12 +151,16 @@ def matching_set_text_limit?(send_node, attribute_name)
end
def matching_add_text_limit?(send_node, table_name, attribute_name)
- limit_table = send_node.children[2].value
+ limit_table = table_name_or_const_name(send_node.children[2])
limit_attribute = send_node.children[3].value
limit_table == table_name && limit_attribute == attribute_name
end
+ def table_name_or_const_name(node)
+ node.type == :const ? node.const_name : node.value
+ end
+
def encrypted_attribute_name?(attribute_name)
attribute_name.to_s.start_with?('encrypted_')
end
diff --git a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
index 032cc12ab941..1a5ba32602cd 100644
--- a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
+++ b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
@@ -46,6 +46,44 @@ def up
end
RUBY
end
+
+ context 'when the table name is a constant' do
+ it 'registers an offense' do
+ expect_offense(<<~RUBY)
+ class TestTextLimits < ActiveRecord::Migration[6.0]
+ disable_ddl_transaction!
+
+ TABLE_NAME = :test_text_limits
+
+ def up
+ create_table TABLE_NAME, id: false do |t|
+ t.integer :test_id, null: false
+ t.text :name
+ ^^^^ #{msg}
+ end
+
+ create_table TABLE_NAME do |t|
+ t.integer :test_id, null: false
+ t.text :title
+ t.text :description
+ ^^^^ #{msg}
+
+ t.text_limit :title, 100
+ end
+
+ add_column :TABLE_NAME, :email, :text
+ ^^^^^^^^^^ #{msg}
+
+ add_column :TABLE_NAME, :role, :text, default: 'default'
+ ^^^^^^^^^^ #{msg}
+
+ change_column_type_concurrently :TABLE_NAME, :test_id, :text
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{msg}
+ end
+ end
+ RUBY
+ end
+ end
end
context 'when text columns are defined with a limit' do
@@ -78,6 +116,40 @@ def up
end
RUBY
end
+
+ context 'when the table name is a constant' do
+ it 'registers no offense' do
+ expect_no_offenses(<<~RUBY)
+ class TestTextLimits < ActiveRecord::Migration[6.0]
+ disable_ddl_transaction!
+
+ TABLE_NAME = :test_text_limits
+
+ def up
+ create_table TABLE_NAME, id: false do |t|
+ t.integer :test_id, null: false
+ t.text :name
+ end
+
+ create_table TABLE_NAME do |t|
+ t.integer :test_id, null: false
+ t.text :title, limit: 100
+ t.text :description, limit: 255
+ end
+
+ add_column TABLE_NAME, :email, :text
+ add_column TABLE_NAME, :role, :text, default: 'default'
+ change_column_type_concurrently TABLE_NAME, :test_id, :text
+
+ add_text_limit TABLE_NAME, :name, 255
+ add_text_limit TABLE_NAME, :email, 255
+ add_text_limit TABLE_NAME, :role, 255
+ add_text_limit TABLE_NAME, :test_id, 255
+ end
+ end
+ RUBY
+ end
+ end
end
context 'when text array columns are defined without a limit' do
--
GitLab