From db955e334fa71a410ef8cda0445f138970a2745d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 23 Feb 2021 21:59:01 +0900 Subject: [PATCH] [Fix #9525] Add `AllowMethodsWithArguments` option to `Style/SymbolProc` ## Summary Fixes #9525. This PR adds `AllowMethodsWithArguments` option to `Style/SymbolProc`. ### `AllowMethodsWithArguments: false` (default) ```ruby # bad something.do_something(foo) { |o| o.bar } # good something.do_something(foo, &:bar) ``` ### `AllowMethodsWithArguments: true` ```ruby # good something.do_something(foo) { |o| o.bar } ``` If user prefer a style that allows block for method with arguments, the user can set `true` to `AllowMethodsWithArguments`. ## Other Information It may be possible to consider defaulting to `AllowMethodsWithArguments: true`, but at the time of this patch `Style/SymbolProc` cop respect keeping the existing behavior. --- ...s_arguments_option_to_style_symbol_proc.md | 1 + config/default.yml | 1 + lib/rubocop/cop/style/symbol_proc.rb | 19 +++++++ spec/rubocop/cop/style/symbol_proc_spec.rb | 55 ++++++++++++++++--- 4 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 changelog/new_add_allow_if_method_has_arguments_option_to_style_symbol_proc.md diff --git a/changelog/new_add_allow_if_method_has_arguments_option_to_style_symbol_proc.md b/changelog/new_add_allow_if_method_has_arguments_option_to_style_symbol_proc.md new file mode 100644 index 00000000000..47ee23f2633 --- /dev/null +++ b/changelog/new_add_allow_if_method_has_arguments_option_to_style_symbol_proc.md @@ -0,0 +1 @@ +* [#9525](https://github.com/rubocop-hq/rubocop/issues/9525): Add `AllowMethodsWithArguments` option to `Style/SymbolProc`. ([@koic][]) diff --git a/config/default.yml b/config/default.yml index 1799e31e9d2..34dae8e6502 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4588,6 +4588,7 @@ Style/SymbolProc: Safe: false VersionAdded: '0.26' VersionChanged: '1.5' + AllowMethodsWithArguments: false # A list of method names to be ignored by the check. # The names should be fairly unique, otherwise you'll end up ignoring lots of code. IgnoredMethods: diff --git a/lib/rubocop/cop/style/symbol_proc.rb b/lib/rubocop/cop/style/symbol_proc.rb index 32b1185cb66..220a07c5f53 100644 --- a/lib/rubocop/cop/style/symbol_proc.rb +++ b/lib/rubocop/cop/style/symbol_proc.rb @@ -5,6 +5,9 @@ module Cop module Style # Use symbols as procs when possible. # + # If you prefer a style that allows block for method with arguments, + # please set `true` to `AllowMethodsWithArguments`. + # # @example # # bad # something.map { |s| s.upcase } @@ -12,6 +15,17 @@ module Style # # # good # something.map(&:upcase) + # + # @example AllowMethodsWithArguments: false (default) + # # bad + # something.do_something(foo) { |o| o.bar } + # + # # good + # something.do_something(foo, &:bar) + # + # @example AllowMethodsWithArguments: true + # # good + # something.do_something(foo) { |o| o.bar } class SymbolProc < Base include RangeHelp include IgnoredMethods @@ -42,6 +56,7 @@ def on_block(node) return if proc_node?(dispatch_node) return if %i[lambda proc].include?(dispatch_node.method_name) return if ignored_method?(dispatch_node.method_name) + return if allow_if_method_has_argument?(node) return if node.block_type? && destructuring_block_argument?(arguments_node) register_offense(node, method_name, dispatch_node.method_name) @@ -103,6 +118,10 @@ def begin_pos_for_replacement(node) node.loc.begin.begin_pos end end + + def allow_if_method_has_argument?(node) + !!cop_config.fetch('AllowMethodsWithArguments', false) && !node.arguments.count.zero? + end end end end diff --git a/spec/rubocop/cop/style/symbol_proc_spec.rb b/spec/rubocop/cop/style/symbol_proc_spec.rb index 8ba7e56b14c..abe7562ad93 100644 --- a/spec/rubocop/cop/style/symbol_proc_spec.rb +++ b/spec/rubocop/cop/style/symbol_proc_spec.rb @@ -142,16 +142,53 @@ expect(&run).not_to raise_error end - context 'when `super` has arguments' do - it 'registers an offense' do - expect_offense(<<~RUBY) - super(one, two) { |x| x.test } - ^^^^^^^^^^^^^^ Pass `&:test` as an argument to `super` instead of a block. - RUBY + context 'when `AllowMethodsWithArguments: true`' do + let(:cop_config) { { 'AllowMethodsWithArguments' => true } } + + context 'when method has arguments' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + do_something(one, two) { |x| x.test } + RUBY + end + end - expect_correction(<<~RUBY) - super(one, two, &:test) - RUBY + context 'when `super` has arguments' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + super(one, two) { |x| x.test } + RUBY + end + end + end + + context 'when `AllowMethodsWithArguments: false`' do + let(:cop_config) { { 'AllowMethodsWithArguments' => false } } + + context 'when method has arguments' do + it 'registers an offense' do + expect_offense(<<~RUBY) + do_something(one, two) { |x| x.test } + ^^^^^^^^^^^^^^ Pass `&:test` as an argument to `do_something` instead of a block. + RUBY + + expect_correction(<<~RUBY) + do_something(one, two, &:test) + RUBY + end + end + + context 'when `super` has arguments' do + it 'registers an offense' do + expect_offense(<<~RUBY) + super(one, two) { |x| x.test } + ^^^^^^^^^^^^^^ Pass `&:test` as an argument to `super` instead of a block. + RUBY + + expect_correction(<<~RUBY) + super(one, two, &:test) + RUBY + end end end