Skip to content

Commit

Permalink
Fix edge case bugs for HeredocArgumentClosingParenthesis
Browse files Browse the repository at this point in the history
  • Loading branch information
Buildkite authored and bbatsov committed Apr 28, 2019
1 parent 184c2a1 commit b1510dd
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 29 deletions.
41 changes: 18 additions & 23 deletions lib/rubocop/cop/layout/heredoc_argument_closing_parenthesis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ class HeredocArgumentClosingParenthesis < Cop

STRING_TYPES = %i[str dstr xstr].freeze
def on_send(node)
heredoc = extract_heredoc_argument(node)
return unless heredoc
heredoc_arg = extract_heredoc_argument(node)
return unless heredoc_arg

outermost_send = outermost_send_with_parens(heredoc)
outermost_send = outermost_send_on_same_line(heredoc_arg)
return unless outermost_send
return unless outermost_send.loc.end
return unless heredoc.first_line != outermost_send.loc.end.line
return unless heredoc_arg.first_line != outermost_send.loc.end.line

add_offense(outermost_send, location: :end)
end
Expand Down Expand Up @@ -111,28 +111,22 @@ def autocorrect(node)

private

def outermost_send_with_parens(heredoc)
def outermost_send_on_same_line(heredoc)
previous = heredoc
current = previous.parent
until containing_send?(current, previous, heredoc)
until send_missing_closing_parens?(current, previous, heredoc)
previous = current
current = current.parent
return unless previous && current
end

while containing_send?(current, previous, heredoc)
outermost_with_parens = current if send_with_parens?(current)
previous = current
current = current.parent
end

outermost_with_parens
current
end

def containing_send?(parent, child, heredoc)
def send_missing_closing_parens?(parent, child, heredoc)
send_node?(parent) &&
parent.arguments.include?(child) &&
parent.first_line == heredoc.last_line
parent.loc.begin &&
parent.loc.end.line != heredoc.last_line
end

def send_node?(node)
Expand All @@ -141,10 +135,6 @@ def send_node?(node)
node.send_type? || node.csend_type?
end

def send_with_parens?(node)
!node.loc.end.nil?
end

def extract_heredoc_argument(node)
node.arguments.find do |arg_node|
extract_heredoc(arg_node)
Expand All @@ -153,9 +143,7 @@ def extract_heredoc_argument(node)

def extract_heredoc(node)
return node if heredoc_node?(node)
if node.send_type? && heredoc_node?(node.receiver)
return node.receiver
end
return node.receiver if single_line_send_with_heredoc_receiver?(node)

return unless node.hash_type?

Expand All @@ -169,6 +157,13 @@ def heredoc_node?(node)
node && STRING_TYPES.include?(node.type) && node.heredoc?
end

def single_line_send_with_heredoc_receiver?(node)
return false unless node.send_type?
return false unless heredoc_node?(node.receiver)

node.receiver.location.heredoc_end.end_pos > node.source_range.end_pos
end

# Closing parenthesis helpers.

def fix_closing_parenthesis(node, corrector)
Expand Down
110 changes: 104 additions & 6 deletions spec/rubocop/cop/layout/heredoc_argument_closing_parenthesis_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,32 @@
RUBY
end

context 'double case new line' do
it 'ignores' do
context 'invocation after the HEREDOC' do
it 'ignores tr' do
expect_no_offenses(<<-RUBY.strip_indent)
foo(
<<-SQL, <<-NOSQL
foo
<<-SQL.tr("z", "t"))
baz
SQL
bar
NOSQL
RUBY
end

it 'ignores random call' do
expect_no_offenses(<<-RUBY.strip_indent)
description(
<<-TEXT.foo)
foobarbaz
TEXT
RUBY
end

it 'ignores random call after' do
expect_no_offenses(<<-RUBY.strip_indent)
description(
<<-TEXT
foobarbaz
TEXT
.foo
)
RUBY
end
Expand Down Expand Up @@ -473,5 +490,86 @@
RUBY
end
end

context 'complex incorrect case with multiple calls' do
it 'detects and fixes the first' do
expect_offense(<<-RUBY.strip_indent)
query.order(Arel.sql(<<-SQL,
foo
SQL
))
^ Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening.
RUBY

expect_correction(<<-RUBY.strip_indent)
query.order(Arel.sql(<<-SQL)
foo
SQL
)
RUBY
end

it 'detects and fixes the second' do
expect_offense(<<-RUBY.strip_indent)
query.order(Arel.sql(<<-SQL)
foo
SQL
)
^ Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening.
RUBY

expect_correction(<<-RUBY.strip_indent)
query.order(Arel.sql(<<-SQL))
foo
SQL
RUBY
end
end

context 'complex incorrect case with multiple calls' do
it 'detects and fixes the first' do
expect_offense(<<-RUBY.strip_indent)
query.joins({
foo: []
}).order(Arel.sql(<<-SQL),
bar
SQL
)
^ Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening.
RUBY

expect_correction(<<-RUBY.strip_indent)
query.joins({
foo: []
}).order(Arel.sql(<<-SQL))
bar
SQL
RUBY
end
end

context 'double case new line' do
it 'detects and fixes' do
expect_offense(<<-RUBY.strip_indent)
foo(
<<-SQL, <<-NOSQL
foo
SQL
bar
NOSQL
)
^ Put the closing parenthesis for a method call with a HEREDOC parameter on the same line as the HEREDOC opening.
RUBY

expect_correction(<<-RUBY.strip_indent)
foo(
<<-SQL, <<-NOSQL)
foo
SQL
bar
NOSQL
RUBY
end
end
end
end

0 comments on commit b1510dd

Please sign in to comment.