Merge pull request #136 from sinsoku/patch/reversible_change_default

Fix a false positive for `Rails/ReversibleMigration`
rubocop · Nov 3, 2019 · 7e540be · 7e540be
2 parents 109dcdd + 37ec353
commit 7e540be
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 4 deletions.
diff --git 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
@@ -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,14 +241,25 @@ 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,
             message: format(MSG, action: "change_table(with #{method_name})")
           )
         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
@@ -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