Skip to content

Commit

Permalink
[Fix #11165] Fix a false positive for Style/RedundantEach
Browse files Browse the repository at this point in the history
Follow up faker-ruby/faker#2613 (comment).
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`.
  • Loading branch information
koic authored and bbatsov committed Nov 10, 2022
1 parent 84c480c commit 59715f2
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
1 change: 1 addition & 0 deletions 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][])
12 changes: 7 additions & 5 deletions lib/rubocop/cop/style/redundant_each.rb
Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions spec/rubocop/cop/style/redundant_each_spec.rb
Expand Up @@ -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) }
Expand Down

0 comments on commit 59715f2

Please sign in to comment.