Skip to content

Commit

Permalink
Merge pull request #16 from r7kamura/inline-in-batches
Browse files Browse the repository at this point in the history
Fix inline `in_batches` false-positive on `Migration/BatchInBatches`
  • Loading branch information
r7kamura committed Jan 27, 2024
2 parents d82c4c2 + 1d40e37 commit 8ade242
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 6 deletions.
29 changes: 25 additions & 4 deletions lib/rubocop/cop/migration/batch_in_batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ module Migration
# disable_ddl_transaction!
#
# def up
# User.in_batches.update_all(some_column: 'some value')
# end
# end
#
# # good
# class BackfillSomeColumnToUsers < ActiveRecord::Migration[7.0]
# disable_ddl_transaction!
#
# def up
# User.in_batches do |relation|
# relation.update_all(some_column: 'some value')
# end
Expand Down Expand Up @@ -75,19 +84,31 @@ def autocorrect(
)
end

# @param node [RuboCop::AST::Node]
# @param node [RuboCop::AST::SendNode]
# @return [Boolean]
def within_in_batches?(node)
def in_batches?(node)
in_block_batches?(node) || in_inline_batches?(node)
end

# @param node [RuboCop::AST::SendNode]
# @return [Boolean]
def in_block_batches?(node)
node.each_ancestor(:block).any? do |ancestor|
ancestor.method?(:in_batches)
end
end

# @param node [RuboCop::AST::SendNode]
# @return [Boolean]
def in_inline_batches?(node)
node.receiver.is_a?(::RuboCop::AST::SendNode) &&
node.receiver.method?(:in_batches)
end

# @param node [RuboCop::AST::SendNode]
# @return [Boolean]
def wrong?(node)
batch_processing?(node) &&
!within_in_batches?(node)
batch_processing?(node) && !in_batches?(node)
end
end
end
Expand Down
16 changes: 14 additions & 2 deletions spec/rubocop/cop/migration/batch_in_batches_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Migration::BatchInBatches, :config do
context 'when `update_all` is used within `in_batches`' do
context 'when `update_all` is used with block `in_batches`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class BackfillUsersSomeColumn < ActiveRecord::Migration[7.0]
Expand All @@ -15,7 +15,19 @@ def change
end
end

context 'when `update_all` is used not within `in_batches`' do
context 'when `update_all` is used with inline `in_batches`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class BackfillUsersSomeColumn < ActiveRecord::Migration[7.0]
def change
User.in_batches.update_all(some_column: 'some value')
end
end
RUBY
end
end

context 'when `update_all` is used not with block `in_batches`' do
it 'registers an offense' do
expect_offense(<<~RUBY)
class BackfillUsersSomeColumn < ActiveRecord::Migration[7.0]
Expand Down

0 comments on commit 8ade242

Please sign in to comment.