Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Lint/UnmodifiedReduceAccumulator to accept blocks which return in the form accumulator[element] #9066

Merged
merged 1 commit into from
Nov 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 }
dvandersluis marked this conversation as resolved.
Show resolved Hide resolved
_{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