diff --git a/changelog/fix_false_positive_for_rails_reversible_migration.md b/changelog/fix_false_positive_for_rails_reversible_migration.md new file mode 100644 index 0000000000..9a6d82a05d --- /dev/null +++ b/changelog/fix_false_positive_for_rails_reversible_migration.md @@ -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][]) diff --git a/lib/rubocop/cop/rails/reversible_migration.rb b/lib/rubocop/cop/rails/reversible_migration.rb index 4bafa6fbaf..9f8d27a22c 100644 --- a/lib/rubocop/cop/rails/reversible_migration.rb +++ b/lib/rubocop/cop/rails/reversible_migration.rb @@ -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) diff --git a/spec/rubocop/cop/rails/reversible_migration_spec.rb b/spec/rubocop/cop/rails/reversible_migration_spec.rb index 9788fc57bf..aba92b052a 100644 --- a/spec/rubocop/cop/rails/reversible_migration_spec.rb +++ b/spec/rubocop/cop/rails/reversible_migration_spec.rb @@ -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