Skip to content

Commit

Permalink
Fix a false positive for Rails/ReversibleMigration
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sinsoku committed Oct 8, 2019
1 parent 3421da1 commit b4ee0fe
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Bug fixes

* [#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][])
* [#136](https://github.com/rubocop-hq/rubocop-rails/pull/136): Fix a false positive for `Rails/ReversibleMigration` when using `change_default` with the :from and :to options. ([@sinsoku][])

## 2.3.2 (2019-09-01)

Expand Down Expand Up @@ -90,3 +91,4 @@
[@MaximeLaurenty]: https://github.com/MaximeLaurenty
[@eugeneius]: https://github.com/eugeneius
[@jas14]: https://github.com/jas14
[@sinsoku]: https://github.com/sinsoku
10 changes: 8 additions & 2 deletions lib/rubocop/cop/rails/reversible_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ module Rails
class ReversibleMigration < Cop
MSG = '%<action>s is not reversible.'
IRREVERSIBLE_CHANGE_TABLE_CALLS = %i[
change change_default remove
change remove
].freeze

def_node_matcher :irreversible_schema_statement_call, <<-PATTERN
Expand Down Expand Up @@ -244,14 +244,20 @@ 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)
!IRREVERSIBLE_CHANGE_TABLE_CALLS.include?(method_name) &&
!irreversible_change_default?(node)

add_offense(
node,
message: format(MSG, action: "change_table(with #{method_name})")
)
end

def irreversible_change_default?(node)
node.method_name == :change_default &&
!all_hash_key?(node.arguments.last, :from, :to)
end

def within_change_method?(node)
node.each_ancestor(:def).any? do |ancestor|
ancestor.method?(:change)
Expand Down
7 changes: 7 additions & 0 deletions spec/rubocop/cop/rails/reversible_migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b4ee0fe

Please sign in to comment.