diff --git a/changelog/fix_change_column_null_in_bulk_change_table.md b/changelog/fix_change_column_null_in_bulk_change_table.md new file mode 100644 index 0000000000..f937582d0c --- /dev/null +++ b/changelog/fix_change_column_null_in_bulk_change_table.md @@ -0,0 +1 @@ +* [#1280](https://github.com/rubocop/rubocop-rails/issues/1280): Look for change_column_null for `BulkChangeTable`. ([@ccutrer][]) diff --git a/lib/rubocop/cop/rails/bulk_change_table.rb b/lib/rubocop/cop/rails/bulk_change_table.rb index 2d5a503d6f..8d73be77ec 100644 --- a/lib/rubocop/cop/rails/bulk_change_table.rb +++ b/lib/rubocop/cop/rails/bulk_change_table.rb @@ -113,8 +113,10 @@ class BulkChangeTable < Base MYSQL_COMBINABLE_ALTER_METHODS = %i[rename_column add_index remove_index].freeze POSTGRESQL_COMBINABLE_TRANSFORMATIONS = %i[change_default].freeze + POSTGRESQL_COMBINABLE_TRANSFORMATIONS_6_1_PLUS = %i[change_null].freeze POSTGRESQL_COMBINABLE_ALTER_METHODS = %i[change_column_default].freeze + POSTGRESQL_COMBINABLE_ALTER_METHODS_6_1_PLUS = %i[change_column_null].freeze def on_def(node) return unless support_bulk_alter? @@ -196,7 +198,9 @@ def combinable_alter_methods when MYSQL COMBINABLE_ALTER_METHODS + MYSQL_COMBINABLE_ALTER_METHODS when POSTGRESQL - COMBINABLE_ALTER_METHODS + POSTGRESQL_COMBINABLE_ALTER_METHODS + result = COMBINABLE_ALTER_METHODS + POSTGRESQL_COMBINABLE_ALTER_METHODS + result += POSTGRESQL_COMBINABLE_ALTER_METHODS_6_1_PLUS if target_rails_version >= 6.1 + result end end @@ -205,7 +209,9 @@ def combinable_transformations when MYSQL COMBINABLE_TRANSFORMATIONS + MYSQL_COMBINABLE_TRANSFORMATIONS when POSTGRESQL - COMBINABLE_TRANSFORMATIONS + POSTGRESQL_COMBINABLE_TRANSFORMATIONS + result = COMBINABLE_TRANSFORMATIONS + POSTGRESQL_COMBINABLE_TRANSFORMATIONS + result += POSTGRESQL_COMBINABLE_TRANSFORMATIONS_6_1_PLUS if target_rails_version >= 6.1 + result end end diff --git a/spec/rubocop/cop/rails/bulk_change_table_spec.rb b/spec/rubocop/cop/rails/bulk_change_table_spec.rb index 5dc5707dcf..d7054408a7 100644 --- a/spec/rubocop/cop/rails/bulk_change_table_spec.rb +++ b/spec/rubocop/cop/rails/bulk_change_table_spec.rb @@ -416,6 +416,26 @@ def change end RUBY end + + it 'does not register an offense for multiple `change_column_null`' do + expect_no_offenses(<<~RUBY) + def change + change_column_null :users, :name, false + change_column_null :users, :address, false + end + RUBY + end + + it 'does not register an offense for multiple `t.change_null`' do + expect_no_offenses(<<~RUBY) + def change + change_table :users do |t| + t.change_null :name, false + t.change_null :address, false + end + end + RUBY + end end context 'when database is PostgreSQL' do @@ -430,6 +450,26 @@ def change it_behaves_like 'offense' it_behaves_like 'no offense for mysql' it_behaves_like 'offense for postgresql' + + it 'does not register an offense for multiple `change_column_null`' do + expect_no_offenses(<<~RUBY) + def change + change_column_null :users, :name, false + change_column_null :users, :address, false + end + RUBY + end + + it 'does not register an offense for multiple `t.change_null`' do + expect_no_offenses(<<~RUBY) + def change + change_table :users do |t| + t.change_null :name, false + t.change_null :address, false + end + end + RUBY + end end context 'with Rails 5.1', :rails51 do @@ -437,6 +477,41 @@ def change it_behaves_like 'no offense for mysql' it_behaves_like 'no offense for postgresql' end + + context 'with Rails 6.1', :rails61 do + it 'registers an offense for multiple `change_column_null`' do + expect_offense(<<~RUBY) + def change + change_column_null :users, :name, false + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ You can use `change_table :users, bulk: true` to combine alter queries. + change_column_null :users, :address, false + end + RUBY + end + + it 'registers an offense for multiple `t.change_null`' do + expect_offense(<<~RUBY) + def change + change_table :users do |t| + ^^^^^^^^^^^^^^^^^^^ You can combine alter queries using `bulk: true` options. + t.change_null :name, false + t.change_null :address, false + end + end + RUBY + end + + it 'does not register an offense for multiple `t.change_null` with `bulk: true`' do + expect_no_offenses(<<~RUBY) + def change + change_table :users, bulk: true do |t| + t.change_null :name, false + t.change_null :address, false + end + end + RUBY + end + end end context 'when `database.yml` is exists' do