Skip to content

Commit

Permalink
[Fix #13299] Fix Style/BlockDelimiters to always accept braces when…
Browse files Browse the repository at this point in the history
… an operator method argument is chained

Update `Style/BlockDelimiters` to not register an offense when braces are necessary to avoid changing precedence. Previously when an operator method was encountered, it was automatically evaluated (ie. not marked as ignored), but this can result in block delimiters being changed that affects block binding.

Now operator methods are evaluated for nodes to ignore, other than if it only has a single argument which is of block type.

As well, I added the missing `expect_corrections` tests for offenses without it.
  • Loading branch information
dvandersluis authored and bbatsov committed Dec 12, 2024
1 parent 522950a commit fd13dab
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#13299](https://github.com/rubocop/rubocop/issues/13299): Fix `Style/BlockDelimiters` to always accept braces when an operator method argument is chained. ([@dvandersluis][])
9 changes: 8 additions & 1 deletion lib/rubocop/cop/style/block_delimiters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ def self.autocorrect_incompatible_with
def on_send(node)
return unless node.arguments?
return if node.parenthesized?
return if node.operator_method? || node.assignment_method?
return if node.assignment_method?
return if single_argument_operator_method?(node)

node.arguments.each do |arg|
get_blocks(arg) do |block|
Expand Down Expand Up @@ -501,6 +502,12 @@ def begin_required?(block_node)
# `begin`...`end` when changing `do-end` to `{}`.
block_node.each_child_node(:rescue, :ensure).any? && !block_node.single_line?
end

def single_argument_operator_method?(node)
return false unless node.operator_method?

node.arguments.one? && node.first_argument.block_type?
end
end
end
end
Expand Down
158 changes: 128 additions & 30 deletions spec/rubocop/cop/style/block_delimiters_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,25 @@
RUBY
end

it 'accepts a multi-line block with chained method as an argument to an operator method' do
expect_no_offenses(<<~RUBY)
'Some text: %s' %
%w[foo bar].map { |v|
v.upcase
}.join(', ')
RUBY
end

it 'accepts a multi-line block with chained method as an argument to an operator method' \
'inside a kwarg' do
expect_no_offenses(<<~RUBY)
foo :bar, baz: 'Some text: %s' %
%w[foo bar].map { |v|
v.upcase
}.join(', ')
RUBY
end

context 'Ruby >= 2.7', :ruby27 do
it 'accepts a multi-line numblock' do
expect_no_offenses(<<~RUBY)
Expand All @@ -59,6 +78,25 @@
} + [0], 1
RUBY
end

it 'accepts a multi-line numblock with chained method as an argument to an operator method' do
expect_no_offenses(<<~RUBY)
foo %
%w[bar baz].map {
_1.upcase
}.join(', ')
RUBY
end

it 'accepts a multi-line numblock with chained method as an argument to an operator method' \
'inside a kwarg' do
expect_no_offenses(<<~RUBY)
foo :bar, baz: 'Some text: %s' %
%w[foo bar].map {
_1.upcase
}.join(', ')
RUBY
end
end
end
end
Expand All @@ -69,6 +107,10 @@
each do |x| end
^^ Prefer `{...}` over `do...end` for single-line blocks.
RUBY

expect_correction(<<~RUBY)
each { |x| }
RUBY
end

it 'accepts a single line block with braces' do
Expand All @@ -88,6 +130,10 @@
each do _1 end
^^ Prefer `{...}` over `do...end` for single-line blocks.
RUBY

expect_correction(<<~RUBY)
each { _1 }
RUBY
end

it 'accepts a single line numblock with braces' do
Expand Down Expand Up @@ -166,6 +212,12 @@
x
}
RUBY

expect_correction(<<~RUBY)
each do |x|
x
end
RUBY
end

it 'registers an offense for a multi-line block with do-end if the return value is assigned' do
Expand All @@ -175,6 +227,12 @@
x
end
RUBY

expect_correction(<<~RUBY)
foo = map { |x|
x
}
RUBY
end

it 'registers an offense for a multi-line block with do-end if the ' \
Expand All @@ -185,6 +243,12 @@
x
end)
RUBY

expect_correction(<<~RUBY)
puts (map { |x|
x
})
RUBY
end

it 'registers an offense for a multi-line block with do-end if the ' \
Expand All @@ -195,6 +259,12 @@
x
end
RUBY

expect_correction(<<~RUBY)
foo.bar = map { |x|
x
}
RUBY
end

it 'accepts a multi-line block with do-end if it is the return value of its scope' do
Expand Down Expand Up @@ -338,21 +408,6 @@
RUBY
end

it 'autocorrects do-end to {} if it is a functional block' do
expect_offense(<<~RUBY)
foo = map do |x|
^^ Prefer `{...}` over `do...end` for functional blocks.
x
end
RUBY

expect_correction(<<~RUBY)
foo = map { |x|
x
}
RUBY
end

it 'autocorrects do-end to {} with appropriate spacing' do
expect_offense(<<~RUBY)
foo = map do|x|
Expand All @@ -368,21 +423,6 @@
RUBY
end

it 'autocorrects do-end to {} if it is a functional block and does not change the meaning' do
expect_offense(<<~RUBY)
puts (map do |x|
^^ Prefer `{...}` over `do...end` for functional blocks.
x
end)
RUBY

expect_correction(<<~RUBY)
puts (map { |x|
x
})
RUBY
end

it 'autocorrects do-end with `rescue` to {} if it is a functional block' do
expect_offense(<<~RUBY)
x = map do |a|
Expand Down Expand Up @@ -473,6 +513,11 @@
^ Avoid using `{...}` for multi-line blocks.
}
RUBY

expect_correction(<<~RUBY)
each do |x|
end
RUBY
end

it 'registers an offense when combined with attribute assignment' do
Expand All @@ -481,6 +526,11 @@
^ Avoid using `{...}` for multi-line blocks.
}
RUBY

expect_correction(<<~RUBY)
foo.bar = baz.map do |x|
end
RUBY
end

it 'registers an offense when there is a comment after the closing brace and block body is not empty' do
Expand Down Expand Up @@ -603,6 +653,16 @@
# ...
})
RUBY

expect_correction(<<~RUBY)
scope :foo, (lambda do |f|
where(condition: "value")
end)
expect { something }.to(raise_error(ErrorClass) do |error|
# ...
end)
RUBY
end

it 'can handle special method names such as []= and done?' do
Expand All @@ -616,6 +676,16 @@
e.nil?
}
RUBY

expect_correction(<<~RUBY)
h2[k2] = Hash.new do |h3,k3|
h3[k3] = 0
end
x = done? list.reject { |e|
e.nil?
}
RUBY
end

it 'autocorrects { and } to do and end' do
Expand Down Expand Up @@ -802,6 +872,14 @@
}
]
RUBY

expect_correction(<<~RUBY)
Hash[
{foo: :bar}.map do |k, v|
[k, v]
end
]
RUBY
end

context 'when there are braces around a multi-line block' do
Expand All @@ -811,6 +889,11 @@
^ Prefer `do...end` for multi-line blocks without chaining.
}
RUBY

expect_correction(<<~RUBY)
each do |x|
end
RUBY
end

it 'registers an offense when combined with attribute assignment' do
Expand All @@ -819,6 +902,11 @@
^ Prefer `do...end` for multi-line blocks without chaining.
}
RUBY

expect_correction(<<~RUBY)
foo.bar = baz.map do |x|
end
RUBY
end

it 'allows when the block is being chained' do
Expand Down Expand Up @@ -926,6 +1014,11 @@
^^ Prefer `{...}` over `do...end` for blocks.
end
RUBY

expect_correction(<<~RUBY)
each { |x|
}
RUBY
end

it 'does not autocorrect do-end if {} would change the meaning' do
Expand Down Expand Up @@ -965,6 +1058,11 @@
^^ Prefer `{...}` over `do...end` for blocks.
end
RUBY

expect_correction(<<~RUBY)
foo.bar = baz.map { |x|
}
RUBY
end

it 'accepts a multi-line functional block with do-end if it is an ignored method' do
Expand Down

0 comments on commit fd13dab

Please sign in to comment.