From 018648792b9ad70441bcda164a1930a930d83f4b Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Thu, 26 Nov 2020 16:06:20 -0500 Subject: [PATCH] Update `Lint/UnmodifiedReduceAccumulator` to handle numblocks and more than 2 arguments. --- ..._update_lintunmodifiedreduceaccumulator.md | 1 + config/default.yml | 1 + .../cop/lint/unmodified_reduce_accumulator.rb | 13 +++++--- rubocop.gemspec | 2 +- .../unmodified_reduce_accumulator_spec.rb | 31 +++++++++++++++++++ 5 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 changelog/change_update_lintunmodifiedreduceaccumulator.md diff --git a/changelog/change_update_lintunmodifiedreduceaccumulator.md b/changelog/change_update_lintunmodifiedreduceaccumulator.md new file mode 100644 index 00000000000..2e061d61a94 --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index 76bfa45e1ce..f66793187fe 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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: <> Lint/UnreachableCode: Description: 'Unreachable code.' diff --git a/lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb b/lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb index 01b3e495592..c0129d74348 100644 --- a/lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb +++ b/lib/rubocop/cop/lint/unmodified_reduce_accumulator.rb @@ -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 @@ -68,7 +67,10 @@ class UnmodifiedReduceAccumulator < Base MSG_INDEX = 'Do not return an element of the accumulator in `%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 @@ -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 @@ -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 diff --git a/rubocop.gemspec b/rubocop.gemspec index 114bf03e44a..e0c220c6e68 100644 --- a/rubocop.gemspec +++ b/rubocop.gemspec @@ -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') diff --git a/spec/rubocop/cop/lint/unmodified_reduce_accumulator_spec.rb b/spec/rubocop/cop/lint/unmodified_reduce_accumulator_spec.rb index 4b77f993036..f593ad6e704 100644 --- a/spec/rubocop/cop/lint/unmodified_reduce_accumulator_spec.rb +++ b/spec/rubocop/cop/lint/unmodified_reduce_accumulator_spec.rb @@ -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