From b471b43530f2ec6afeb60b6c3b600d955a07d26e Mon Sep 17 00:00:00 2001
From: Cody Cutrer <cody@instructure.com>
Date: Wed, 22 May 2024 12:22:18 -0600
Subject: [PATCH] [Fix #1280] Handle change_column_null for BulkChangeTable

change_column_null is only combinable with PostgreSQL, and is only
available as change_null within a change_table block since Rails 6.1,
so target to only those cases.
---
 ...change_column_null_in_bulk_change_table.md |  1 +
 lib/rubocop/cop/rails/bulk_change_table.rb    | 10 ++-
 .../cop/rails/bulk_change_table_spec.rb       | 75 +++++++++++++++++++
 3 files changed, 84 insertions(+), 2 deletions(-)
 create mode 100644 changelog/fix_change_column_null_in_bulk_change_table.md

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