Skip to content

Commit

Permalink
[Fix #12274] Fix a false positive for Lint/Void
Browse files Browse the repository at this point in the history
Fixes #12274.

This PR fixes a false positive for `Lint/Void` when `each`'s receiver is
an object of `Enumerator` to which `filter` has been applied.

`each` blocks are allowed to prevent false positives.
The expression inside the following block is not void:

```ruby
enumerator = [1, 2, 3].filter
enumerator.each { |item| item >= 2 } #=> [2, 3]
```

This might introduce false negatives, but higher severity Lint cop should
aim for as safe detections as possible, I think.
  • Loading branch information
koic authored and bbatsov committed Oct 25, 2023
1 parent 21cad51 commit 7bedbf4
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/fix_a_false_positive_for_lint_void.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12274](https://github.com/rubocop/rubocop/issues/12274): Fix a false positive for `Lint/Void` when `each`'s receiver is an object of `Enumerator` to which `filter` has been applied. ([@koic][])
20 changes: 17 additions & 3 deletions lib/rubocop/cop/lint/void.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ module Lint
# Checks for operators, variables, literals, lambda, proc and nonmutating
# methods used in void context.
#
# `each` blocks are allowed to prevent false positives.
# For example, the expression inside the `each` block below.
# It's not void, especially when the receiver is an `Enumerator`:
#
# [source,ruby]
# ----
# enumerator = [1, 2, 3].filter
# enumerator.each { |item| item >= 2 } #=> [2, 3]
# ----
#
# @example CheckForMethodsWithNoSideEffects: false (default)
# # bad
# def some_method
Expand Down Expand Up @@ -72,6 +82,7 @@ def on_block(node)
return unless node.body && !node.body.begin_type?
return unless in_void_context?(node.body)

check_void_op(node.body) { node.method?(:each) }
check_expression(node.body)
end

Expand All @@ -87,11 +98,13 @@ def on_begin(node)
def check_begin(node)
expressions = *node
expressions.pop unless in_void_context?(node)
expressions.each { |expr| check_expression(expr) }
expressions.each do |expr|
check_void_op(expr)
check_expression(expr)
end
end

def check_expression(expr)
check_void_op(expr)
check_literal(expr)
check_var(expr)
check_self(expr)
Expand All @@ -101,8 +114,9 @@ def check_expression(expr)
check_nonmutating(expr)
end

def check_void_op(node)
def check_void_op(node, &block)
return unless node.send_type? && OPERATORS.include?(node.method_name)
return if block && yield(node)

add_offense(node.loc.selector,
message: format(OP_MSG, op: node.method_name)) do |corrector|
Expand Down
10 changes: 10 additions & 0 deletions spec/rubocop/cop/lint/void_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,16 @@ def foo=(rhs)
RUBY
end

it 'does not register `#each` block with conditional expression' do
expect_no_offenses(<<~RUBY)
enumerator_as_filter.each do |item|
# The `filter` method is used to filter for matches with `42`.
# In this case, it's not void.
item == 42
end
RUBY
end

context 'Ruby 2.7', :ruby27 do
it 'registers two offenses for void literals in `#tap` method' do
expect_offense(<<~RUBY)
Expand Down

0 comments on commit 7bedbf4

Please sign in to comment.