From 37ec3532b4e37796371a3750ef26939fb76034c2 Mon Sep 17 00:00:00 2001
From: sinsoku <sinsoku.listy@gmail.com>
Date: Tue, 27 Aug 2019 18:24:25 +0900
Subject: [PATCH] Fix a false positive for `Rails/ReversibleMigration`

The `change_table` method is reversible when using `change_default` with
the :from and :to options.
However, RuboCop detects `Rails/ReversibleMigration` incorrectly, so it fixes.
---
 CHANGELOG.md                                     |  2 ++
 lib/rubocop/cop/rails/reversible_migration.rb    | 16 ++++++++++++----
 .../cop/rails/reversible_migration_spec.rb       |  7 +++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9ecb6037ef..fcee2a44ee 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
 * [#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][])
+* [#136](https://github.com/rubocop-hq/rubocop-rails/pull/136): Fix a false positive for `Rails/ReversibleMigration` when using `change_default` with `:from` and `:to` options. ([@sinsoku][])
 
 ## 2.3.2 (2019-09-01)
 
@@ -93,3 +94,4 @@
 [@eugeneius]: https://github.com/eugeneius
 [@jas14]: https://github.com/jas14
 [@forresty]: https://github.com/forresty
+[@sinsoku]: https://github.com/sinsoku
diff --git a/lib/rubocop/cop/rails/reversible_migration.rb b/lib/rubocop/cop/rails/reversible_migration.rb
index 3484c6b59d..ba77ecba9a 100644
--- a/lib/rubocop/cop/rails/reversible_migration.rb
+++ b/lib/rubocop/cop/rails/reversible_migration.rb
@@ -127,9 +127,6 @@ module Rails
       # @see https://api.rubyonrails.org/classes/ActiveRecord/Migration/CommandRecorder.html
       class ReversibleMigration < Cop
         MSG = '%<action>s is not reversible.'
-        IRREVERSIBLE_CHANGE_TABLE_CALLS = %i[
-          change change_default remove
-        ].freeze
 
         def_node_matcher :irreversible_schema_statement_call, <<-PATTERN
           (send nil? ${:change_table_comment :execute :remove_belongs_to} ...)
@@ -244,7 +241,7 @@ def check_change_table_node(node, block)
         def check_change_table_offense(receiver, node)
           method_name = node.method_name
           return if receiver != node.receiver &&
-                    !IRREVERSIBLE_CHANGE_TABLE_CALLS.include?(method_name)
+                    reversible_change_table_call?(node)
 
           add_offense(
             node,
@@ -252,6 +249,17 @@ def check_change_table_offense(receiver, node)
           )
         end
 
+        def reversible_change_table_call?(node)
+          case node.method_name
+          when :change, :remove
+            false
+          when :change_default
+            all_hash_key?(node.arguments.last, :from, :to)
+          else
+            true
+          end
+        end
+
         def within_change_method?(node)
           node.each_ancestor(:def).any? do |ancestor|
             ancestor.method?(:change)
diff --git a/spec/rubocop/cop/rails/reversible_migration_spec.rb b/spec/rubocop/cop/rails/reversible_migration_spec.rb
index 45dfb9dcde..bae2e99f51 100644
--- a/spec/rubocop/cop/rails/reversible_migration_spec.rb
+++ b/spec/rubocop/cop/rails/reversible_migration_spec.rb
@@ -165,6 +165,13 @@ def change
       end
     RUBY
 
+    it_behaves_like 'accepts',
+                    'change_table(with reversible change_default)', <<-RUBY
+      change_table :users do |t|
+        t.change_default :authorized, from: nil, to: 1
+      end
+    RUBY
+
     it_behaves_like 'offense', 'change_table(with change_default)', <<-RUBY
       change_table :users do |t|
         t.change_default :authorized, 1