Skip to content

Commit

Permalink
Merge pull request #439 from koic/fix_a_false_positive_for_performanc…
Browse files Browse the repository at this point in the history
…e_chain_array_allocation

[Fix #437] Fix a false positive for `Performance/ChainArrayAllocation`
  • Loading branch information
koic committed Feb 3, 2024
2 parents 3ba5d91 + 2103446 commit c07dcd2
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 1 deletion.
@@ -0,0 +1 @@
* [#437](https://github.com/rubocop/rubocop-performance/issues/437): Fix a false positive for `Performance/ChainArrayAllocation` when using `select` with block argument after `select`. ([@koic][])
11 changes: 10 additions & 1 deletion lib/rubocop/cop/performance/chain_array_allocation.rb
Expand Up @@ -54,20 +54,29 @@ class ChainArrayAllocation < Base
def_node_matcher :chain_array_allocation?, <<~PATTERN
(send {
(send _ $%RETURN_NEW_ARRAY_WHEN_ARGS {int lvar ivar cvar gvar send})
(block (send _ $%ALWAYS_RETURNS_NEW_ARRAY) ...)
({block numblock} (send _ $%ALWAYS_RETURNS_NEW_ARRAY) ...)
(send _ $%RETURNS_NEW_ARRAY ...)
} $%HAS_MUTATION_ALTERNATIVE ...)
PATTERN

def on_send(node)
chain_array_allocation?(node) do |fm, sm|
return if node.each_descendant(:send).any? { |descendant| descendant.method?(:lazy) }
return if node.method?(:select) && !enumerable_select_method?(node.receiver)

range = range_between(node.loc.dot.begin_pos, node.source_range.end_pos)

add_offense(range, message: format(MSG, method: fm, second_method: sm))
end
end

private

def enumerable_select_method?(node)
# NOTE: `QueryMethods#select` in Rails accepts positional arguments, whereas `Enumerable#select` does not.
# This difference can be utilized to reduce the knowledge requirements related to `select`.
(node.block_type? || node.numblock_type?) && node.send_node.arguments.empty?
end
end
end
end
Expand Down
34 changes: 34 additions & 0 deletions spec/rubocop/cop/performance/chain_array_allocation_spec.rb
Expand Up @@ -53,4 +53,38 @@
RUBY
end
end

describe 'when using `select` with block argument after `select` with positional arguments' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
model.select(:foo, :bar).select { |item| item.do_something }
RUBY
end
end

describe 'when using `select` with block argument after `select` with block argument' do
it 'registers an offense' do
expect_offense(<<~RUBY)
model.select { |item| item.foo }.select { |item| item.bar }
^^^^^^^ Use unchained `select` and `select!` (followed by `return array` if required) instead of chaining `select...select`.
RUBY
end
end

describe 'when using `select` with block argument after `select` with numbered block argument' do
it 'registers an offense' do
expect_offense(<<~RUBY)
model.select { _1.foo }.select { |item| item.bar }
^^^^^^^ Use unchained `select` and `select!` (followed by `return array` if required) instead of chaining `select...select`.
RUBY
end
end

describe 'when using `select` with positional arguments after `select`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
model.select(:foo, :bar).select(:baz, :qux)
RUBY
end
end
end

0 comments on commit c07dcd2

Please sign in to comment.