Skip to content

Commit

Permalink
Merge pull request #553 from koic/fix_false_positive_for_rails_revers…
Browse files Browse the repository at this point in the history
…ible_migration

Fix a false positive for `Rails/ReversibleMigration`
  • Loading branch information
koic authored Sep 25, 2021
2 parents 7c5ecd5 + 1038d95 commit 32ae8f0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#553](https://github.com/rubocop/rubocop-rails/pull/553): Fix a false positive for `Rails/ReversibleMigration` when using `t.remove` with `type` option in Rails 6.1. ([@koic][])
12 changes: 10 additions & 2 deletions lib/rubocop/cop/rails/reversible_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -317,16 +317,24 @@ def check_change_table_offense(receiver, node)
return if receiver != node.receiver &&
reversible_change_table_call?(node)

action = if method_name == :remove
target_rails_version >= 6.1 ? 't.remove (without type)' : 't.remove'
else
"change_table(with #{method_name})"
end

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

def reversible_change_table_call?(node)
case node.method_name
when :change, :remove
when :change
false
when :remove
target_rails_version >= 6.1 && all_hash_key?(node.arguments.last, :type)
when :change_default, :change_column_default, :change_table_comment,
:change_column_comment
all_hash_key?(node.arguments.last, :from, :to)
Expand Down
32 changes: 28 additions & 4 deletions spec/rubocop/cop/rails/reversible_migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,35 @@ def change
end
RUBY

it_behaves_like 'offense', 'change_table(with remove)', <<~RUBY
change_table :users do |t|
t.remove :qualification
context 'remove' do
context 'Rails >= 6.1', :rails61 do
it_behaves_like 'accepts', 't.remove (with type)', <<~RUBY
change_table :users do |t|
t.remove(:posts, type: :text)
end
RUBY

it_behaves_like 'offense', 't.remove (without type)', <<~RUBY
change_table :users do |t|
t.remove(:posts)
end
RUBY
end
RUBY

context 'Rails < 6.1', :rails60 do
it_behaves_like 'offense', 't.remove', <<~RUBY
change_table :users do |t|
t.remove(:posts, type: :text)
end
RUBY

it_behaves_like 'offense', 't.remove', <<~RUBY
change_table :users do |t|
t.remove(:posts)
end
RUBY
end
end
end

context 'remove_columns' do
Expand Down

0 comments on commit 32ae8f0

Please sign in to comment.