Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #10864] Fix a false positive for Style/SymbolProc #10865

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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