From 59715f2b87998004eba98a7d6e25fe17369d702a Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 7 Nov 2022 00:46:13 +0900 Subject: [PATCH] [Fix #11165] Fix a false positive for `Style/RedundantEach` Follow up https://github.com/faker-ruby/faker/pull/2613#issuecomment-1304316381. Fixes #11165. This PR fixes a false positive for `Style/RedundantEach` when any method is used between methods containing `each` in the method name. That method in between may convert from `Enumerator` to `Array`. e.g. `map(&:do_something)` ```ruby 'string'.each_char.class # => Enumerator 'string'.each_char.map(&:to_i).class # => Array ``` `each_with_index` is a method of `Array`, not of `Enumerator`. ```ruby 'string'.each_char.map(&:to_i).respond_to?(:each_with_index) # => true 'string'.each_char.map(&:to_i).respond_to?(:with_index) # => false ``` Therefore, it allows any method to be used between methods containing `each` to prevent `NoMethodError`. --- .../fix_false_positive_for_style_redundant_each.md | 1 + lib/rubocop/cop/style/redundant_each.rb | 12 +++++++----- spec/rubocop/cop/style/redundant_each_spec.rb | 6 ++++++ 3 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 changelog/fix_false_positive_for_style_redundant_each.md diff --git a/changelog/fix_false_positive_for_style_redundant_each.md b/changelog/fix_false_positive_for_style_redundant_each.md new file mode 100644 index 00000000000..02f1bc2bd11 --- /dev/null +++ b/changelog/fix_false_positive_for_style_redundant_each.md @@ -0,0 +1 @@ +* [#11165](https://github.com/rubocop/rubocop/issues/11165): Fix a false positive for `Style/RedundantEach` when any method is used between methods containing `each` in the method name. ([@koic][]) diff --git a/lib/rubocop/cop/style/redundant_each.rb b/lib/rubocop/cop/style/redundant_each.rb index 0a85a948028..6a651a3d849 100644 --- a/lib/rubocop/cop/style/redundant_each.rb +++ b/lib/rubocop/cop/style/redundant_each.rb @@ -68,15 +68,17 @@ def redundant_each_method(node) ancestor.receiver == node && (RESTRICT_ON_SEND.include?(ancestor.method_name) || ancestor.method?(:reverse_each)) end + + return ancestor_node if ancestor_node end - ancestor_node || node.each_descendant(:send).detect do |descendant| - next if descendant.parent.block_type? || descendant.last_argument&.block_pass_type? + return unless (prev_method = node.children.first) + return if !prev_method.send_type? || + prev_method.parent.block_type? || prev_method.last_argument&.block_pass_type? - detected = descendant.method_name.to_s.start_with?('each_') unless node.method?(:each) + detected = prev_method.method_name.to_s.start_with?('each_') unless node.method?(:each) - detected || descendant.method?(:reverse_each) - end + prev_method if detected || prev_method.method?(:reverse_each) end # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity diff --git a/spec/rubocop/cop/style/redundant_each_spec.rb b/spec/rubocop/cop/style/redundant_each_spec.rb index f47e68ef38b..454d1a4c747 100644 --- a/spec/rubocop/cop/style/redundant_each_spec.rb +++ b/spec/rubocop/cop/style/redundant_each_spec.rb @@ -143,6 +143,12 @@ RUBY end + it 'does not register an offense when any method is used between methods with `each` in the method name' do + expect_no_offenses(<<~RUBY) + string.each_char.map(&:to_i).reverse.each_with_index.map { |v, i| do_something(v, i) } + RUBY + end + it 'does not register an offense when using `each.with_object`' do expect_no_offenses(<<~RUBY) array.each.with_object { |v, o| do_something(v, o) }