Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #10832] Fix an incorrect autocorrect for Layout/BlockEndNewline #10835

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1 @@
* [#10832](https://github.com/rubocop/rubocop/issues/10832): Fix an incorrect autocorrect for `Layout/BlockEndNewline` when multiline block `}` is not on its own line and using heredoc argument. ([@koic][])
37 changes: 32 additions & 5 deletions lib/rubocop/cop/layout/block_end_newline.rb
Expand Up @@ -36,24 +36,51 @@ def on_block(node)
# If the end is on its own line, there is no offense
return if begins_its_line?(node.loc.end)

add_offense(node.loc.end, message: message(node)) do |corrector|
corrector.replace(delimiter_range(node), "\n#{node.loc.end.source}#{offset(node)}")
end
register_offense(node)
end

private

def register_offense(node)
add_offense(node.loc.end, message: message(node)) do |corrector|
offense_range = offense_range(node)
replacement = "\n#{offense_range.source.strip}"

if (heredoc = last_heredoc_argument(node.body))
corrector.remove(offense_range)
corrector.insert_after(heredoc.loc.heredoc_end, replacement)
else
corrector.replace(offense_range, replacement)
end
end
end

def message(node)
format(MSG, line: node.loc.end.line, column: node.loc.end.column + 1)
end

def delimiter_range(node)
def last_heredoc_argument(node)
return unless (arguments = node&.arguments)

heredoc = arguments.reverse.detect(&:heredoc?)
return heredoc if heredoc

last_heredoc_argument(node.children.first)
end

def offense_range(node)
Parser::Source::Range.new(
node.loc.expression.source_buffer,
node.children.compact.last.loc.expression.end_pos,
node.loc.expression.end_pos
end_of_method_chain(node).loc.expression.end_pos
)
end

def end_of_method_chain(node)
return node unless node.parent&.call_type?

end_of_method_chain(node.parent)
end
end
end
end
Expand Down
101 changes: 100 additions & 1 deletion spec/rubocop/cop/layout/block_end_newline_spec.rb
Expand Up @@ -27,7 +27,7 @@
RUBY
end

it 'registers an offense and corrects when multiline block } is not on its own line' do
it 'registers an offense and corrects when multiline block `}` is not on its own line' do
expect_offense(<<~RUBY)
test {
foo }
Expand Down Expand Up @@ -55,4 +55,103 @@
}
RUBY
end

it 'registers an offense and corrects when multiline block `}` is not on its own line ' \
'and using method chain' do
expect_offense(<<~RUBY)
test {
foo }.bar.baz
^ Expression at 2, 7 should be on its own line.
RUBY

expect_correction(<<~RUBY)
test {
foo
}.bar.baz
RUBY
end

it 'registers an offense and corrects when multiline block `}` is not on its own line ' \
'and using heredoc argument' do
expect_offense(<<~RUBY)
test {
foo(<<~EOS) }
^ Expression at 2, 15 should be on its own line.
Heredoc text.
EOS
RUBY

expect_correction(<<~RUBY)
test {
foo(<<~EOS)
Heredoc text.
EOS
}
RUBY
end

it 'registers an offense and corrects when multiline block `}` is not on its own line ' \
'and using multiple heredoc arguments' do
expect_offense(<<~RUBY)
test {
foo(<<~FIRST, <<~SECOND) }
^ Expression at 2, 28 should be on its own line.
Heredoc text.
FIRST
Heredoc text.
SECOND
RUBY

expect_correction(<<~RUBY)
test {
foo(<<~FIRST, <<~SECOND)
Heredoc text.
FIRST
Heredoc text.
SECOND
}
RUBY
end

it 'registers an offense and corrects when multiline block `}` is not on its own line ' \
'and using heredoc argument with method chain' do
expect_offense(<<~RUBY)
test {
foo(<<~EOS).bar }
^ Expression at 2, 19 should be on its own line.
Heredoc text.
EOS
RUBY

expect_correction(<<~RUBY)
test {
foo(<<~EOS).bar
Heredoc text.
EOS
}
RUBY
end

it 'registers an offense and corrects when multiline block `}` is not on its own line ' \
'and using multiple heredoc argument method chain' do
expect_offense(<<~RUBY)
test {
foo(<<~FIRST).bar(<<~SECOND) }
^ Expression at 2, 32 should be on its own line.
Heredoc text.
FIRST
Heredoc text.
SECOND
RUBY

expect_correction(<<~RUBY)
test {
foo(<<~FIRST).bar(<<~SECOND)
Heredoc text.
FIRST
Heredoc text.
SECOND
}
RUBY
end
end