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 = '%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