Skip to content

Commit

Permalink
[Fix #9902] Fix an incorrect auto-correct for Style/BlockDelimiters
Browse files Browse the repository at this point in the history
Fixes #9902

This PR fixes an incorrect auto-correct for `Style/BlockDelimiters`
when there is a comment after the closing brace.
It prevents `Style/CommentedKeyword` from removing source code
comment after `end` keyword.
  • Loading branch information
koic authored and bbatsov committed Jul 6, 2021
1 parent e758a0d commit cfdb0a1
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 1 deletion.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Expand Up @@ -13,7 +13,7 @@ InternalAffairs/NodeDestructuring:
# Offense count: 49
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 186
Max: 196

# Offense count: 228
# Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods.
Expand Down
@@ -0,0 +1 @@
* [#9902](https://github.com/rubocop/rubocop/issues/9902): Fix an incorrect auto-correct for `Style/BlockDelimiters` when there is a comment after the closing brace. ([@koic][])
15 changes: 15 additions & 0 deletions lib/rubocop/cop/style/block_delimiters.rb
Expand Up @@ -135,6 +135,7 @@ module Style
class BlockDelimiters < Base
include ConfigurableEnforcedStyle
include IgnoredMethods
include RangeHelp
extend AutoCorrector

ALWAYS_BRACES_MESSAGE = 'Prefer `{...}` over `do...end` for blocks.'
Expand Down Expand Up @@ -231,6 +232,11 @@ def replace_braces_with_do_end(corrector, loc)
corrector.insert_before(e, ' ') unless whitespace_before?(e)
corrector.insert_after(b, ' ') unless whitespace_after?(b)
corrector.replace(b, 'do')

if (comment = processed_source.comment_at_line(e.line))
move_comment_before_block(corrector, comment, loc.node, e)
end

corrector.replace(e, 'end')
end

Expand All @@ -252,6 +258,15 @@ def whitespace_after?(range, length = 1)
/\s/.match?(range.source_buffer.source[range.begin_pos + length, 1])
end

def move_comment_before_block(corrector, comment, block_node, closing_brace)
range = range_between(closing_brace.end_pos, comment.loc.expression.end_pos)

corrector.remove(range_with_surrounding_space(range: range, side: :right))
corrector.insert_after(closing_brace, "\n")

corrector.insert_before(block_node, "#{comment.text}\n")
end

def get_blocks(node, &block)
case node.type
when :block
Expand Down
22 changes: 22 additions & 0 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -196,6 +196,28 @@ def batch
RUBY
end

it 'corrects `EnforcedStyle: line_count_based` of `Style/BlockDelimiters` with `Style/CommentedKeyword` and `Layout/BlockEndNewline`' do
create_file('.rubocop.yml', <<~YAML)
Style/BlockDelimiters:
EnforcedStyle: line_count_based
YAML
source = <<~RUBY
foo {
bar } # This comment should be kept.
RUBY
create_file('example.rb', source)
expect(cli.run([
'--auto-correct',
'--only', 'Style/BlockDelimiters,Style/CommentedKeyword,Layout/BlockEndNewline'
])).to eq(0)
expect(File.read('example.rb')).to eq(<<~RUBY)
# This comment should be kept.
foo do
bar
end
RUBY
end

it 'corrects `EnforcedStyle: require_parentheses` of `Style/MethodCallWithArgsParentheses` with `Style/NestedParenthesizedCalls`' do
create_file('.rubocop.yml', <<~YAML)
Style/MethodCallWithArgsParentheses:
Expand Down
29 changes: 29 additions & 0 deletions spec/rubocop/cop/style/block_delimiters_spec.rb
Expand Up @@ -354,6 +354,35 @@
RUBY
end

it 'registers an offense when there is a comment after the closing brace and block body is not empty' do
expect_offense(<<~RUBY)
baz.map { |x|
^ Avoid using `{...}` for multi-line blocks.
foo(x) } # comment
RUBY

expect_correction(<<~RUBY)
# comment
baz.map do |x|
foo(x) end
RUBY
end

it 'registers an offense when there is a comment after the closing brace and block body is empty' do
expect_offense(<<~RUBY)
baz.map { |x|
^ Avoid using `{...}` for multi-line blocks.
} # comment
RUBY

expect_correction(<<~RUBY)
# comment
baz.map do |x|
end
RUBY
end

This comment has been minimized.

Copy link
@medoalmdar

medoalmdar Aug 20, 2021

spec/rubocop/cop/style/block_delimiters_spec.rb


it 'accepts braces if do-end would change the meaning' do
expect_no_offenses(<<~RUBY)
scope :foo, lambda { |f|
Expand Down

0 comments on commit cfdb0a1

Please sign in to comment.