Merge pull request #592 from mattmccormick/mccormick-prevent-change-column-in-change-method

[Fix #591] Add `change_column` check to `Rails/ReversibleMigration`

Author: koic

changelog/change_add_change_column_check_to_railsreversiblemigration.md

@@ -0,0 +1 @@
+* [#591](https://github.com/rubocop/rubocop-rails/issues/591): Add `change_column` check to `Rails/ReversibleMigration`. ([@mattmccormick][])

changelog/change_add_remove_reference_check_to_railsreversiblemigration.md

@@ -0,0 +1 @@
+* Add `remove_reference` check to `Rails/ReversibleMigration`. ([@mattmccormick][])

lib/rubocop/cop/rails/reversible_migration.rb

@@ -179,7 +179,7 @@ class ReversibleMigration < Base
         MSG = '%<action>s is not reversible.'
 
         def_node_matcher :irreversible_schema_statement_call, <<~PATTERN
-          (send nil? ${:execute :remove_belongs_to} ...)
+          (send nil? ${:change_column :execute :remove_belongs_to :remove_reference} ...)
         PATTERN
 
         def_node_matcher :drop_table_call, <<~PATTERN

spec/rubocop/cop/rails/reversible_migration_spec.rb

@@ -110,6 +110,20 @@ def change
     RUBY
   end
 
+  context 'change_column' do
+    it_behaves_like 'accepts', 'up_only', <<~RUBY
+      up_only { change_column(:posts, :state, :string) }
+    RUBY
+
+    it_behaves_like 'offense', 'change_column', <<~RUBY
+      change_column(:posts, :state, :string)
+    RUBY
+
+    it_behaves_like 'offense', 'change_column', <<~RUBY
+      change_column(:posts, :state, :string, null: false)
+    RUBY
+  end
+
   context 'change_column_default' do
     it_behaves_like 'accepts',
                     'change_column_default(with :from and :to)', <<-RUBY
@@ -157,6 +171,20 @@ def change
     RUBY
   end
 
+  context 'remove_belongs_to' do
+    it_behaves_like 'accepts', 'up_only', <<~RUBY
+      up_only { remove_belongs_to(:products, :user, index: false) }
+    RUBY
+
+    it_behaves_like 'offense', 'remove_belongs_to', <<~RUBY
+      remove_belongs_to(:products, :user, index: false)
+    RUBY
+
+    it_behaves_like 'offense', 'remove_belongs_to', <<~RUBY
+      remove_belongs_to(:products, :supplier, polymorphic: true)
+    RUBY
+  end
+
   context 'remove_column' do
     it_behaves_like 'accepts', 'remove_column(with type)', <<~RUBY
       remove_column(:suppliers, :qualification, :string)
@@ -185,6 +213,20 @@ def change
     RUBY
   end
 
+  context 'remove_reference' do
+    it_behaves_like 'accepts', 'up_only', <<~RUBY
+      up_only { remove_reference(:products, :user, index: false) }
+    RUBY
+
+    it_behaves_like 'offense', 'remove_reference', <<~RUBY
+      remove_reference(:products, :user, index: false)
+    RUBY
+
+    it_behaves_like 'offense', 'remove_reference', <<~RUBY
+      remove_reference(:products, :supplier, polymorphic: true)
+    RUBY
+  end
+
   context 'change_table' do
     it_behaves_like 'accepts', 'change_table(with reversible calls)', <<~RUBY
       change_table :users do |t|