Skip to content

Commit

Permalink
[Fix rubocop#3308] Make Lint/NextWithoutAccumulator aware of nested…
Browse files Browse the repository at this point in the history
… enumeration

This cop would register an offense if a nested block contained a `next`
without an accumulator, e.g.:

```
[(1..3), (4..6)].reduce(0) do |acc, elems|
  elems.each_with_index do |elem, i|
    next if i == 1
    acc << elem
  end
  acc
end
```

This fix addresses that, by checking if the direct block parent of any
offending `next` is indeed the block that we were inspecting.
  • Loading branch information
Drenmi committed Jul 14, 2016
1 parent cd7246f commit c2c24e7
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
### Changes

* [#2645](https://github.com/bbatsov/rubocop/issues/2645): `Style/EmptyLiteral` no longer generates an offense for `String.new` when using frozen string literals. ([@drenmi][])
* [#3308](https://github.com/bbatsov/rubocop/issues/3308): Make `Lint/NextWithoutAccumulator` aware of nested enumeration. ([@drenmi][])

## 0.41.2 (2016-07-07)

Expand Down
10 changes: 9 additions & 1 deletion lib/rubocop/cop/lint/next_without_accumulator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,19 @@ class NextWithoutAccumulator < Cop

def on_block(node)
on_body_of_reduce(node) do |body|
void_next = body.each_node(:next).find { |n| n.children.empty? }
void_next = body.each_node(:next).find do |n|
n.children.empty? && parent_block_node(n) == node
end

add_offense(void_next, :expression) if void_next
end
end

private

def parent_block_node(node)
node.each_ancestor.find { |n| n.type == :block }
end
end
end
end
Expand Down
21 changes: 19 additions & 2 deletions spec/rubocop/cop/lint/next_without_accumulator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ def code_with_accumulator(method_name)
SOURCE
end

def code_with_nested_block(method_name)
<<-SOURCE
[(1..3), (4..6)].#{method_name}(0) do |acc, elems|
elems.each_with_index do |elem, i|
next if i == 1
acc << elem
end
acc
end
SOURCE
end

shared_examples 'reduce/inject' do |reduce_alias|
context "given a #{reduce_alias} block" do
it 'registers an offense for a bare next' do
Expand All @@ -36,11 +48,16 @@ def code_with_accumulator(method_name)
inspect_source(cop, code_with_accumulator(reduce_alias))
expect(cop.offenses).to be_empty
end

it 'accepts next within a nested block' do
inspect_source(cop, code_with_nested_block(reduce_alias))
expect(cop.offenses).to be_empty
end
end
end

include_examples 'reduce/inject', :reduce
include_examples 'reduce/inject', :inject
it_behaves_like 'reduce/inject', :reduce
it_behaves_like 'reduce/inject', :inject

context 'given an unrelated block' do
it 'accepts a bare next' do
Expand Down

0 comments on commit c2c24e7

Please sign in to comment.