From b1510ddcd2b1ce8f3cda662d802cb825df2aa35c Mon Sep 17 00:00:00 2001 From: Buildkite Date: Sun, 28 Apr 2019 10:17:45 -0500 Subject: [PATCH] Fix edge case bugs for HeredocArgumentClosingParenthesis --- .../heredoc_argument_closing_parenthesis.rb | 41 +++---- ...redoc_argument_closing_parenthesis_spec.rb | 110 +++++++++++++++++- 2 files changed, 122 insertions(+), 29 deletions(-) diff --git a/lib/rubocop/cop/layout/heredoc_argument_closing_parenthesis.rb b/lib/rubocop/cop/layout/heredoc_argument_closing_parenthesis.rb index 3c3f78fcc32..d2ed19b2ad5 100644 --- a/lib/rubocop/cop/layout/heredoc_argument_closing_parenthesis.rb +++ b/lib/rubocop/cop/layout/heredoc_argument_closing_parenthesis.rb @@ -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 @@ -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) @@ -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) @@ -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? @@ -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) diff --git a/spec/rubocop/cop/layout/heredoc_argument_closing_parenthesis_spec.rb b/spec/rubocop/cop/layout/heredoc_argument_closing_parenthesis_spec.rb index 6855c56a36d..c67e0dcfdfe 100644 --- a/spec/rubocop/cop/layout/heredoc_argument_closing_parenthesis_spec.rb +++ b/spec/rubocop/cop/layout/heredoc_argument_closing_parenthesis_spec.rb @@ -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 @@ -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