Skip to content

Commit

Permalink
Update Lint/UnmodifiedReduceAccumulator to accept blocks which retu…
Browse files Browse the repository at this point in the history
…rn in the form `accumulator[element]`.
  • Loading branch information
dvandersluis authored and mergify[bot] committed Nov 18, 2020
1 parent b09456f commit 449a62c
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9059](https://github.com/rubocop-hq/rubocop/pull/9059): Update `Lint/UnmodifiedReduceAccumulator` to accept blocks which return in the form `accumulator[element]`. ([@dvandersluis][])
17 changes: 13 additions & 4 deletions lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ module Lint
# value
# end
#
# # good, recursive
# keys.reduce(self) { |result, key| result[key] }
#
# # ignored as the return value cannot be determined
# enum.reduce do |acc, el|
# x = foo(acc, el)
Expand Down Expand Up @@ -131,7 +134,7 @@ def check_return_values(block_node)
element_name = block_arg_name(block_node, 1)
message_opts = { method: block_node.method_name, accum: accumulator_name }

if (node = returned_accumulator_index(return_values, accumulator_name))
if (node = returned_accumulator_index(return_values, accumulator_name, element_name))
add_offense(node, message: format(MSG_INDEX, message_opts))
elsif potential_offense?(return_values, block_node.body, element_name, accumulator_name)
return_values.each do |return_val|
Expand All @@ -146,11 +149,17 @@ def block_arg_name(node, index)
node.arguments[index].node_parts[0]
end

# Look for an index of the accumulator being returned
# Look for an index of the accumulator being returned, except where the index
# is the element.
# This is always an offense, in order to try to catch potential exceptions
# due to type mismatches
def returned_accumulator_index(return_values, accumulator_name)
return_values.detect { |val| accumulator_index?(val, accumulator_name) }
def returned_accumulator_index(return_values, accumulator_name, element_name)
return_values.detect do |val|
next unless accumulator_index?(val, accumulator_name)
next true if val.method?(:[]=)

val.arguments.none? { |arg| lvar_used?(arg, element_name) }
end
end

def potential_offense?(return_values, block_body, element_name, accumulator_name)
Expand Down
32 changes: 30 additions & 2 deletions spec/rubocop/cop/lint/unmodified_reduce_accumulator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -373,15 +373,43 @@
RUBY
end

it 'does not register an offense when calling a method on the accumulator with the element' do
expect_no_offenses(<<~RUBY)
foo.#{method} { |result, key| result.method(key) }
RUBY
end

it 'registers an offense when returning an index of the accumulator' do
expect_offense(<<~RUBY)
%w(a b c).#{method}({}) do |acc, letter|
acc[foo]
^^^^^^^^ Do not return an element of the accumulator in `#{method}`.
end
RUBY
end

it 'registers an offense when returning an index setter on the accumulator' do
expect_offense(<<~RUBY)
%w(a b c).#{method}({}) do |acc, letter|
acc[letter] = true
^^^^^^^^^^^^^^^^^^ Do not return an element of the accumulator in `#{method}`.
acc[foo] = bar
^^^^^^^^^^^^^^ Do not return an element of the accumulator in `#{method}`.
end
RUBY
end

it 'does not register an offense when returning accumulator[element]' do
expect_no_offenses(<<~RUBY)
foo.#{method} { |result, key| result[key] }
RUBY
end

it 'registers an offense when returning accumulator[element]=' do
expect_offense(<<~RUBY, method: method)
foo.#{method} { |result, key| result[key] = foo }
_{method} ^^^^^^^^^^^^^^^^^ Do not return an element of the accumulator in `#{method}`.
RUBY
end

it 'registers an offense when returning an expression with the element' do
expect_offense(<<~RUBY)
(1..4).#{method}(0) do |acc, el|
Expand Down

0 comments on commit 449a62c

Please sign in to comment.