Skip to content

Commit

Permalink
Update Lint/UnmodifiedReduceAccumulator to handle numblocks and mor…
Browse files Browse the repository at this point in the history
…e than 2 arguments.
  • Loading branch information
dvandersluis authored and bbatsov committed Nov 26, 2020
1 parent c67ecc7 commit 0186487
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog/change_update_lintunmodifiedreduceaccumulator.md
@@ -0,0 +1 @@
* [#9108](https://github.com/rubocop-hq/rubocop/pull/9108): Update `Lint/UnmodifiedReduceAccumulator` to handle numblocks and more than 2 arguments. ([@dvandersluis][])
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -2007,6 +2007,7 @@ Lint/UnmodifiedReduceAccumulator:
Description: Checks for `reduce` or `inject` blocks that do not update the accumulator each iteration.
Enabled: pending
VersionAdded: '1.1'
VersionChanged: <<next>>

Lint/UnreachableCode:
Description: 'Unreachable code.'
Expand Down
13 changes: 8 additions & 5 deletions lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb
Expand Up @@ -12,8 +12,7 @@ module Lint
# could be rewritten as such without a loop.
#
# Also catches instances where an index of the accumulator is returned, as
# this may change the type of object being retained. As well, detects when
# fewer than 2 block arguments are specified.
# this may change the type of object being retained.
#
# NOTE: For the purpose of reducing false positives, this cop only flags
# returns in `reduce` blocks where the element is the only variable in
Expand Down Expand Up @@ -68,7 +67,10 @@ class UnmodifiedReduceAccumulator < Base
MSG_INDEX = 'Do not return an element of the accumulator in `%<method>s`.'

def_node_matcher :reduce_with_block?, <<~PATTERN
(block (send _recv {:reduce :inject} ...) args ...)
{
(block (send _recv {:reduce :inject} ...) args ...)
(numblock (send _recv {:reduce :inject} ...) ...)
}
PATTERN

def_node_matcher :accumulator_index?, <<~PATTERN
Expand Down Expand Up @@ -107,10 +109,11 @@ class UnmodifiedReduceAccumulator < Base

def on_block(node)
return unless reduce_with_block?(node)
return unless node.arguments.length >= 2
return unless node.argument_list.length >= 2

check_return_values(node)
end
alias on_numblock on_block

private

Expand Down Expand Up @@ -148,7 +151,7 @@ def check_return_values(block_node)
end

def block_arg_name(node, index)
node.arguments[index].node_parts[0]
node.argument_list[index].name
end

# Look for an index of the accumulator being returned, except where the index
Expand Down
2 changes: 1 addition & 1 deletion rubocop.gemspec
Expand Up @@ -35,7 +35,7 @@ Gem::Specification.new do |s|
s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0')
s.add_runtime_dependency('regexp_parser', '>= 2.0')
s.add_runtime_dependency('rexml')
s.add_runtime_dependency('rubocop-ast', '>= 1.1.1')
s.add_runtime_dependency('rubocop-ast', '>= 1.2.0')
s.add_runtime_dependency('ruby-progressbar', '~> 1.7')
s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 2.0')

Expand Down
31 changes: 31 additions & 0 deletions spec/rubocop/cop/lint/unmodified_reduce_accumulator_spec.rb
Expand Up @@ -585,6 +585,37 @@
(1..4).#{method}(0) { |acc| acc.foo }
RUBY
end

it 'ignores when there is a splat argument' do
expect_no_offenses(<<~RUBY, method: method)
values.#{method}(0) { |*x| x[0] + x[1] }
RUBY
end

it 'registers an offense when there are more than two arguments but the element is returned' do
expect_offense(<<~RUBY)
(1..4).each_with_index.#{method}([]) do |acc, (el, index)|
acc[el] = method(index)
el
^^ Ensure the accumulator `acc` will be modified by `#{method}`.
end
RUBY
end
end

context 'numblocks', :ruby27 do
it 'registers an offense when returning the element' do
expect_offense(<<~RUBY, method: method)
(1..4).#{method}(0) { _2 }
_{method} ^^ Ensure the accumulator `_1` will be modified by `#{method}`.
RUBY
end

it 'does not register an offense when when returning the accumulator' do
expect_no_offenses(<<~RUBY)
values.#{method}(0) { _1 + _2 }
RUBY
end
end
end
end
Expand Down

0 comments on commit 0186487

Please sign in to comment.