Skip to content

Commit

Permalink
Fix false positive reversing remove_foreign_key
Browse files Browse the repository at this point in the history
The other table can be specified either as the second argument, or in
the :to_table key of a hash as the second argument. The latter was not
supported.
  • Loading branch information
joshpencheon committed Feb 20, 2020
1 parent 652bb56 commit dff35b8
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

* [#180](https://github.com/rubocop-hq/rubocop-rails/issues/180): Fix a false positive for `HttpPositionalArguments` when using `get` method with `:to` option. ([@koic][])
* [#193](https://github.com/rubocop-hq/rubocop-rails/issues/193): Make `Rails/EnvironmentComparison` aware of `Rails.env` is used in RHS or when `!=` is used for comparison. ([@koic][])
* [#205](https://github.com/rubocop-hq/rubocop-rails/pull/205): Make `Rails/ReversibleMigration` aware of `:to_table` option of `remove_foreign_key`. ([@joshpencheon][])

## 2.4.2 (2020-01-26)

Expand Down
7 changes: 6 additions & 1 deletion lib/rubocop/cop/rails/reversible_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ module Rails
# remove_foreign_key :accounts, :branches
# end
#
# # good
# def change
# remove_foreign_key :accounts, to_table: :branches
# end
#
# @example
# # change_table
#
Expand Down Expand Up @@ -210,7 +215,7 @@ def check_remove_column_node(node)

def check_remove_foreign_key_node(node)
remove_foreign_key_call(node) do |arg|
if arg.hash_type?
if arg.hash_type? && !all_hash_key?(arg, :to_table)
add_offense(
node,
message: format(MSG,
Expand Down
5 changes: 5 additions & 0 deletions spec/rubocop/cop/rails/reversible_migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ def change
remove_foreign_key :accounts, :branches
RUBY

it_behaves_like 'accepts',
'remove_foreign_key(with to_table option)', <<-RUBY
remove_foreign_key :accounts, to_table: :branches
RUBY

it_behaves_like 'offense', 'remove_foreign_key(without table)', <<~RUBY
remove_foreign_key :accounts, column: :owner_id
RUBY
Expand Down

0 comments on commit dff35b8

Please sign in to comment.