Merge pull request rubocop#1284 from ccutrer/fix-change-column-null-i…

…n-bulk-change-table

[Fix rubocop#1280] Handle change_column_null for BulkChangeTable

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][])

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_SINCE_6_1 = %i[change_null].freeze
 
         POSTGRESQL_COMBINABLE_ALTER_METHODS = %i[change_column_default].freeze
+        POSTGRESQL_COMBINABLE_ALTER_METHODS_SINCE_6_1 = %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_SINCE_6_1 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_SINCE_6_1 if target_rails_version >= 6.1
+            result
           end
         end
 

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,13 +450,68 @@ 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
       it_behaves_like 'no offense'
       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