Skip to content

Commit

Permalink
[Fix rubocop#10864] Fix a false positive for Style/SymbolProc
Browse files Browse the repository at this point in the history
Fixes rubocop#10864.

This PR fixes a false positive for `Style/SymbolProc` when using `Hash#reject`.
The same is true for `Haash#select`.

```ruby
{foo: :bar}.reject { p _1 } # `_1` is `:foo`
{foo: :bar}.select { p _1 } # `_1` is `:foo`
{foo: :bar}.detect { p _1 } # `_1` is `[:foo, :bar]`
{foo: :bar}.map { p _1 }    # `_1` is `[:foo, :bar]`
```

It fixes `on_block` method because the same issue occurs in normal blocks.

```ruby
{foo: :bar}.select {|item| item.nil? } #=> no erros.
{foo: :bar}.select(&:nil?)             #=> wrong number of arguments (given 1, expected 0) (ArgumentError)
```

`Style/SymbolProc` is already marked as unsafe, but it avoids avoidable issues.
  • Loading branch information
koic committed Aug 4, 2022
1 parent 6b6d554 commit d154c2a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 9 deletions.
@@ -0,0 +1 @@
* [#10864](https://github.com/rubocop/rubocop/issues/10864): Fix a false positive for `Style/SymbolProc` when using `Hash#reject`. ([@koic][])
10 changes: 8 additions & 2 deletions lib/rubocop/cop/style/symbol_proc.rb
Expand Up @@ -113,13 +113,14 @@ def self.autocorrect_incompatible_with
[Layout::SpaceBeforeBlockBraces]
end

# rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def on_block(node)
symbol_proc?(node) do |dispatch_node, arguments_node, method_name|
# TODO: Rails-specific handling that we should probably make
# configurable - https://github.com/rubocop/rubocop/issues/1485
# we should allow lambdas & procs
return if proc_node?(dispatch_node)
return if unsafe_hash_usage?(dispatch_node)
return if %i[lambda proc].include?(dispatch_node.method_name)
return if allowed_method_name?(dispatch_node.method_name)
return if allow_if_method_has_argument?(node.send_node)
Expand All @@ -129,7 +130,7 @@ def on_block(node)
register_offense(node, method_name, dispatch_node.method_name)
end
end
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
alias on_numblock on_block

def destructuring_block_argument?(argument_node)
Expand All @@ -138,6 +139,11 @@ def destructuring_block_argument?(argument_node)

private

# See: https://github.com/rubocop/rubocop/issues/10864
def unsafe_hash_usage?(node)
node.receiver&.hash_type? && %i[reject select].include?(node.method_name)
end

def allowed_method_name?(name)
allowed_method?(name) || matches_allowed_pattern?(name)
end
Expand Down
74 changes: 67 additions & 7 deletions spec/rubocop/cop/style/symbol_proc_spec.rb
Expand Up @@ -150,6 +150,36 @@
expect(&run).not_to raise_error
end

%w[reject select].each do |method|
it "registers an offense when receiver is an array literal and using `#{method}` with a block" do
expect_offense(<<~RUBY, method: method)
[1, 2, 3].%{method} {|item| item.foo }
_{method} ^^^^^^^^^^^^^^^^^^ Pass `&:foo` as an argument to `#{method}` instead of a block.
RUBY

expect_correction(<<~RUBY)
[1, 2, 3].#{method}(&:foo)
RUBY
end

it "registers an offense when receiver is some value and using `#{method}` with a block" do
expect_offense(<<~RUBY, method: method)
[1, 2, 3].#{method} {|item| item.foo }
_{method} ^^^^^^^^^^^^^^^^^^ Pass `&:foo` as an argument to `#{method}` instead of a block.
RUBY

expect_correction(<<~RUBY)
[1, 2, 3].#{method}(&:foo)
RUBY
end

it "does not register an offense when receiver is a hash literal and using `#{method}` with a block" do
expect_no_offenses(<<~RUBY, method: method)
{foo: 42}.#{method} {|item| item.foo }
RUBY
end
end

context 'when `AllowMethodsWithArguments: true`' do
let(:cop_config) { { 'AllowMethodsWithArguments' => true } }

Expand Down Expand Up @@ -290,7 +320,37 @@
end

context 'numblocks', :ruby27 do
it 'registers an offense for a block with a single numeric argument' do
%w[reject select].each do |method|
it "registers an offense when receiver is an array literal and using `#{method}` with a numblock" do
expect_offense(<<~RUBY, method: method)
[1, 2, 3].%{method} { _1.foo }
_{method} ^^^^^^^^^^ Pass `&:foo` as an argument to `#{method}` instead of a block.
RUBY

expect_correction(<<~RUBY)
[1, 2, 3].#{method}(&:foo)
RUBY
end

it "registers an offense when receiver is some value and using `#{method}` with a numblock" do
expect_offense(<<~RUBY, method: method)
do_something.%{method} { _1.foo }
_{method} ^^^^^^^^^^ Pass `&:foo` as an argument to `#{method}` instead of a block.
RUBY

expect_correction(<<~RUBY)
do_something.#{method}(&:foo)
RUBY
end

it "does not register an offense when receiver is a hash literal and using `#{method}` with a numblock" do
expect_no_offenses(<<~RUBY, method: method)
{foo: 42}.#{method} { _1.foo }
RUBY
end
end

it 'registers an offense for a block with a numbered parameter' do
expect_offense(<<~RUBY)
something { _1.foo }
^^^^^^^^^^ Pass `&:foo` as an argument to `something` instead of a block.
Expand All @@ -301,27 +361,27 @@
RUBY
end

it 'accepts block with multiple numeric argumnets' do
it 'accepts block with multiple numbered parameteres' do
expect_no_offenses('something { _1 + _2 }')
end

it 'accepts lambda with 1 argument' do
it 'accepts lambda with 1 numbered parameter' do
expect_no_offenses('-> { _1.method }')
end

it 'accepts proc with 1 argument' do
it 'accepts proc with 1 numbered parameter' do
expect_no_offenses('proc { _1.method }')
end

it 'accepts block with only second numeric argument' do
it 'accepts block with only second numbered parameter' do
expect_no_offenses('something { _2.first }')
end

it 'accepts Proc.new with 1 argument' do
it 'accepts Proc.new with 1 numbered parameter' do
expect_no_offenses('Proc.new { _1.method }')
end

it 'accepts ::Proc.new with 1 argument' do
it 'accepts ::Proc.new with 1 numbered parameter' do
expect_no_offenses('::Proc.new { _1.method }')
end
end
Expand Down

0 comments on commit d154c2a

Please sign in to comment.