From 7e3d3652d4585a37a0bd6f32fe1d4905bad4bb81 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Thu, 14 Jul 2016 23:38:35 +0800 Subject: [PATCH] [Fix #3308] Make `Lint/NextWithoutAccumulator` aware of nested enumeration (#3313) 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. --- CHANGELOG.md | 1 + .../cop/lint/next_without_accumulator.rb | 10 ++++++++- .../cop/lint/next_without_accumulator_spec.rb | 21 +++++++++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 916aa3ff6620..1097df4f9c09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/rubocop/cop/lint/next_without_accumulator.rb b/lib/rubocop/cop/lint/next_without_accumulator.rb index 8b3627e6261f..43e6e07060ee 100644 --- a/lib/rubocop/cop/lint/next_without_accumulator.rb +++ b/lib/rubocop/cop/lint/next_without_accumulator.rb @@ -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 diff --git a/spec/rubocop/cop/lint/next_without_accumulator_spec.rb b/spec/rubocop/cop/lint/next_without_accumulator_spec.rb index 79b790812291..95d3046d6ee6 100644 --- a/spec/rubocop/cop/lint/next_without_accumulator_spec.rb +++ b/spec/rubocop/cop/lint/next_without_accumulator_spec.rb @@ -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 @@ -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