diff --git a/changelog/fix_a_false_positive_for_style_symbol_proc_when_using_hash_reject.md b/changelog/fix_a_false_positive_for_style_symbol_proc_when_using_hash_reject.md new file mode 100644 index 00000000000..10fa96ac73a --- /dev/null +++ b/changelog/fix_a_false_positive_for_style_symbol_proc_when_using_hash_reject.md @@ -0,0 +1 @@ +* [#10864](https://github.com/rubocop/rubocop/issues/10864): Fix a false positive for `Style/SymbolProc` when using `Hash#reject`. ([@koic][]) diff --git a/lib/rubocop/cop/style/symbol_proc.rb b/lib/rubocop/cop/style/symbol_proc.rb index dc56c3ca981..5808194a59d 100644 --- a/lib/rubocop/cop/style/symbol_proc.rb +++ b/lib/rubocop/cop/style/symbol_proc.rb @@ -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) @@ -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) @@ -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 diff --git a/spec/rubocop/cop/style/symbol_proc_spec.rb b/spec/rubocop/cop/style/symbol_proc_spec.rb index e1dc91d3b01..59d5162beee 100644 --- a/spec/rubocop/cop/style/symbol_proc_spec.rb +++ b/spec/rubocop/cop/style/symbol_proc_spec.rb @@ -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 } } @@ -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. @@ -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