Skip to content

Commit

Permalink
[Fix rubocop#5821] Support AR::Migration#up_only for `Rails/Reversi…
Browse files Browse the repository at this point in the history
…bleMigration`

Fixes rubocop#5821.

This PR supports the following `AR::Migration#up_only` method.

```console
% cat db/migrate/example.rb
class AddPublishedToPosts < ActiveRecord::Migration[5.2]
  def change
    add_column :posts, :published, :boolean, default: false

    up_only do
      execute "UPDATE posts SET published = 'true'"
    end
  end
end
```

http://api.rubyonrails.org/classes/ActiveRecord/Migration.html#method-i-up_only

This PR fixes the following false posive when using Rails 5.2.0.

```console
% rubocop db/migrate/example.rb --only Rails/ReversibleMigration
Inspecting 1 file
C

Offenses:

db/migrate/example.rb:6:7: C: Rails/ReversibleMigration: execute is not
reversible.
      execute "UPDATE posts SET published = 'true'"
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected
```
  • Loading branch information
koic committed Apr 26, 2018
1 parent dd779a3 commit 3307053
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#5801](https://github.com/bbatsov/rubocop/pull/5801): Add new `Rails/RefuteMethods` cop. ([@koic][])
* [#5805](https://github.com/bbatsov/rubocop/pull/5805): Add new `Rails/AssertNot` cop. ([@composerinteralia][])
* [#4136](https://github.com/bbatsov/rubocop/issues/4136): Allow more robust `Layout/ClosingParenthesisIndentation` detection including method chaining. ([@jfelchner][])
* [#5821](https://github.com/bbatsov/rubocop/pull/5821): Support `AR::Migration#up_only` for `Rails/ReversibleMigration` cop. ([@koic][])

### Bug fixes

Expand Down
10 changes: 6 additions & 4 deletions lib/rubocop/cop/rails/reversible_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class ReversibleMigration < Cop

def on_send(node)
return unless within_change_method?(node)
return if within_reversible_block?(node)
return if within_reversible_or_up_only_block?(node)

check_irreversible_schema_statement_node(node)
check_drop_table_node(node)
Expand All @@ -168,7 +168,7 @@ def on_send(node)

def on_block(node)
return unless within_change_method?(node)
return if within_reversible_block?(node)
return if within_reversible_or_up_only_block?(node)

check_change_table_node(node.send_node, node.body)
end
Expand Down Expand Up @@ -261,9 +261,11 @@ def within_change_method?(node)
end
end

def within_reversible_block?(node)
def within_reversible_or_up_only_block?(node)
node.each_ancestor(:block).any? do |ancestor|
ancestor.block_type? && ancestor.send_node.method?(:reversible)
ancestor.block_type? &&
ancestor.send_node.method?(:reversible) ||
ancestor.send_node.method?(:up_only)
end
end

Expand Down
4 changes: 4 additions & 0 deletions spec/rubocop/cop/rails/reversible_migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ def change
execute "ALTER TABLE `pages_linked_pages` ADD UNIQUE `page_id_linked_page_id` (`page_id`,`linked_page_id`)"
RUBY

it_behaves_like :accepts, 'up_only', <<-RUBY
up_only { execute "UPDATE posts SET published = 'true'" }
RUBY

context 'within block' do
it_behaves_like :accepts, 'create_table', <<-RUBY
[:users, :articles].each do |table|
Expand Down

0 comments on commit 3307053

Please sign in to comment.