From fc2d927083971bd3196b336545372b1ad4c53d30 Mon Sep 17 00:00:00 2001 From: Genadi Samokovarov Date: Wed, 24 Jan 2024 17:50:00 +0200 Subject: [PATCH] Fix numblock regressions in `omit_parentheses` Style/MethodCallWithArgsParentheses My last change was trying to fix an edge case where RuboCop wanted us to remove parentheses in the following example, but removing the parens was resulting in a `SyntaxError`: ```ruby AnyCable.middleware.use( Class.new(AnyCable::Middleware) do ^^^^^^^^^^^^^^^^^^^^^^ Omit parentheses for method calls with arguments. pass end ) ``` As it often happens, though, I broke a few other cases where we now want to remove parens, while the removal can result in ambiguous code: **Case 1** ```ruby Foo::Bar.find(pending.things.map { _1['code'] }) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Omit parentheses for method calls with arguments. ``` While we can remove the parentheses in *Case 1*, the author has to be well aware of the difference between the do/end and braced blocks method bounding semantics. Allowing the author to put the parens can remove this ambiguity and we used to allow it. **Case 2** ```ruby [a, b].map { _1.call 'something' }.uniq.join(' - ') ^^^^^^^ Omit parentheses for method calls with arguments. ``` If we set `AllowParenthesesInChaining: true`, we should allow parentheses in chained calls. However, this is broken in current RuboCop. Both of the issues were caused by a 'refactoring' that forgot to check argument calls or chaining with `numblocks` specifically. --- ...numblock_regressions_in_omit_parentheses.md | 1 + .../omit_parentheses.rb | 2 +- .../method_call_with_args_parentheses_spec.rb | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_numblock_regressions_in_omit_parentheses.md diff --git a/changelog/fix_numblock_regressions_in_omit_parentheses.md b/changelog/fix_numblock_regressions_in_omit_parentheses.md new file mode 100644 index 000000000000..39394afe206e --- /dev/null +++ b/changelog/fix_numblock_regressions_in_omit_parentheses.md @@ -0,0 +1 @@ +* [#12648](https://github.com/rubocop/rubocop/pull/12648): Fix numblock regressions in `omit_parentheses` `Style/MethodCallWithArgsParentheses`. ([@gsamokovarov][]) diff --git a/lib/rubocop/cop/style/method_call_with_args_parentheses/omit_parentheses.rb b/lib/rubocop/cop/style/method_call_with_args_parentheses/omit_parentheses.rb index 965c5a33e481..b48a80b2883a 100644 --- a/lib/rubocop/cop/style/method_call_with_args_parentheses/omit_parentheses.rb +++ b/lib/rubocop/cop/style/method_call_with_args_parentheses/omit_parentheses.rb @@ -132,7 +132,7 @@ def call_with_ambiguous_arguments?(node) # rubocop:disable Metrics/PerceivedComp call_in_match_pattern?(node) || hash_literal_in_arguments?(node) || node.descendants.any? do |n| - n.forwarded_args_type? || n.block_type? || + n.forwarded_args_type? || n.block_type? || n.numblock_type? || ambiguous_literal?(n) || logical_operator?(n) end end diff --git a/spec/rubocop/cop/style/method_call_with_args_parentheses_spec.rb b/spec/rubocop/cop/style/method_call_with_args_parentheses_spec.rb index d6f78bc82874..774eeb21c6e8 100644 --- a/spec/rubocop/cop/style/method_call_with_args_parentheses_spec.rb +++ b/spec/rubocop/cop/style/method_call_with_args_parentheses_spec.rb @@ -771,6 +771,18 @@ def seatle_style arg: default(42) expect_no_offenses('foo(1) { 2 }') end + it 'accepts parens around argument values with blocks' do + expect_no_offenses(<<~RUBY) + Foo::Bar.find(pending.things.map { |t| t['code'] }.first) + RUBY + end + + it 'accepts parens around argument values with numblocks', :ruby27 do + expect_no_offenses(<<~RUBY) + Foo::Bar.find(pending.things.map { _1['code'] }.first) + RUBY + end + it 'accepts parens in array literal calls with blocks' do expect_no_offenses(<<~RUBY) [ @@ -1018,6 +1030,12 @@ def seatle_style arg: default(42) expect_no_offenses('foo().bar(3).wait 4') end + it 'accept parens when previously chained sends have numblocks', :ruby27 do + expect_no_offenses(<<~RUBY) + [a, b].map { _1.call 'something' }.uniq.join(' - ') + RUBY + end + it 'accepts parens in the last call if any previous calls with parentheses' do expect_no_offenses('foo().bar(3).quux.wait(4)') end