[Fix rubocop#144] false positive for Rails/ReversibleMigration

change_table_comment is only irreversible when passing a string as
the comment. If we pass a hash with `:from` and `:to` keys, it
works as a reversible migration

Fixes rubocop#144

CHANGELOG.md

@@ -11,6 +11,8 @@
 * [#120](https://github.com/rubocop-hq/rubocop-rails/issues/120): Fix message for `Rails/SaveBang` when the save is in the body of a conditional. ([@jas14][])
 * [#131](https://github.com/rubocop-hq/rubocop-rails/pull/131): Fix an incorrect autocorrect for `Rails/Presence` when using `[]` method. ([@forresty][])
 * [#142](https://github.com/rubocop-hq/rubocop-rails/pull/142): Fix an incorrect autocorrect for `Rails/EnumHash` when using nested constants. ([@koic][])
+* [#144](https://github.com/rubocop-hq/rubocop-rails/issues/144): Fix a false positive for `Rails/ReversibleMigration` when using change_table_comment with a `:from` and `:to` hash. ([@DNA][])
+
 
 ## 2.3.2 (2019-09-01)
 
@@ -93,3 +95,4 @@
 [@eugeneius]: https://github.com/eugeneius
 [@jas14]: https://github.com/jas14
 [@forresty]: https://github.com/forresty
+[@DNA]: https://github.com/DNA

lib/rubocop/cop/rails/reversible_migration.rb

@@ -132,7 +132,7 @@ class ReversibleMigration < Cop
         ].freeze
 
         def_node_matcher :irreversible_schema_statement_call, <<-PATTERN
-          (send nil? ${:change_table_comment :execute :remove_belongs_to} ...)
+          (send nil? ${:execute :remove_belongs_to} ...)
         PATTERN
 
         def_node_matcher :drop_table_call, <<-PATTERN
@@ -143,6 +143,14 @@ class ReversibleMigration < Cop
           (send nil? :change_column_default {[(sym _) (sym _)] (splat _)} $...)
         PATTERN
 
+        def_node_matcher :change_table_comment_call, <<-PATTERN
+          (send nil? :change_table_comment {[(sym _)]} $...)
+        PATTERN
+
+        def_node_matcher :change_column_comment_call, <<-PATTERN
+          (send nil? :change_column_comment {[(sym _)]} $...)
+        PATTERN
+
         def_node_matcher :remove_column_call, <<-PATTERN
           (send nil? :remove_column $...)
         PATTERN
@@ -162,6 +170,8 @@ def on_send(node)
           check_irreversible_schema_statement_node(node)
           check_drop_table_node(node)
           check_change_column_default_node(node)
+          check_change_table_comment_node(node)
+          check_change_column_comment_node(node)
           check_remove_column_node(node)
           check_remove_foreign_key_node(node)
         end
@@ -206,6 +216,32 @@ def check_change_column_default_node(node)
           end
         end
 
+        def check_change_table_comment_node(node)
+          change_table_comment_call(node) do |args|
+            unless all_hash_key?(args.last, :from, :to)
+              add_offense(
+                node,
+                message: format(
+                  MSG, action: 'change_table_comment(without :from and :to)'
+                )
+              )
+            end
+          end
+        end
+
+        def check_change_column_comment_node(node)
+          change_column_comment_call(node) do |args|
+            unless all_hash_key?(args.last, :from, :to)
+              add_offense(
+                node,
+                message: format(
+                  MSG, action: 'change_column_comment(without :from and :to)'
+                )
+              )
+            end
+          end
+        end
+
         def check_remove_column_node(node)
           remove_column_call(node) do |args|
             if args.to_a.size < 3

spec/rubocop/cop/rails/reversible_migration_spec.rb

@@ -126,6 +126,30 @@ def change
     RUBY
   end
 
+  context 'change_table_comment' do
+    it_behaves_like 'accepts',
+                    'change_table_comment(with :from and :to)', <<-RUBY
+      change_table_comment(:posts, from: nil, to: "draft")
+    RUBY
+
+    it_behaves_like 'offense',
+                    'change_table_comment(without :from and :to)', <<-RUBY
+      change_table_comment(:suppliers, 'new')
+    RUBY
+  end
+
+  context 'change_column_comment' do
+    it_behaves_like 'accepts',
+                    'change_column_comment(with :from and :to)', <<-RUBY
+      change_column_comment(:posts, :state, from: nil, to: "draft")
+    RUBY
+
+    it_behaves_like 'offense',
+                    'change_column_comment(without :from and :to)', <<-RUBY
+      change_column_comment(:suppliers, :qualification, 'new')
+    RUBY
+  end
+
   context 'remove_column' do
     it_behaves_like 'accepts', 'remove_column(with type)', <<-RUBY
       remove_column(:suppliers, :qualification, :string)