Skip to content

Commit

Permalink
Fix numblock regressions in omit_parentheses Style/MethodCallWithAr…
Browse files Browse the repository at this point in the history
…gsParentheses

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.
  • Loading branch information
gsamokovarov committed Jan 24, 2024
1 parent b4080d8 commit fc2d927
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions 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][])
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions spec/rubocop/cop/style/method_call_with_args_parentheses_spec.rb
Expand Up @@ -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)
[
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit fc2d927

Please sign in to comment.