From 2ddbc78664d960886c05511b4a74c3c848ed1a2f Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Thu, 14 Feb 2019 01:12:37 -0500 Subject: [PATCH] [Fix #6493] `Layout/FirstMethodArgumentLineBreak` for multi-line single argument --- CHANGELOG.md | 2 ++ lib/rubocop/ast/node.rb | 5 +++- .../first_method_argument_line_break.rb | 14 +++++++++- .../cop/mixin/first_element_line_break.rb | 8 ++++++ .../first_method_argument_line_break_spec.rb | 27 +++++++++++++++++++ 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f080bd7ea6f2..0302ce965b88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Changes * [#5977](https://github.com/rubocop-hq/rubocop/issues/5977): Warn for Performance Cops. ([@koic][]) +* [#6493](https://github.com/rubocop-hq/rubocop/issues/6493): Fix `Layout/FirstMethodArgumentLineBreak` for multi-line single argument. ([@marcotc][]) +* [#6699](https://github.com/rubocop-hq/rubocop/issues/6699): Fix infinite loop for `Layout/IndentationWidth` and `Layout/IndentationConsistency` when bad modifier indentation before good method definition. ([@koic][]) ## 0.66.0 (2019-03-18) diff --git a/lib/rubocop/ast/node.rb b/lib/rubocop/ast/node.rb index 5941484e6186..2d23d4ee25de 100644 --- a/lib/rubocop/ast/node.rb +++ b/lib/rubocop/ast/node.rb @@ -464,7 +464,10 @@ def chained? end def argument? - parent && parent.send_type? && parent.arguments.include?(self) + # TODO: Adds support safe navigator calls. + # TODO: Discuss with maintainers if this should be applied 'everywhere' + parent && (parent.send_type? || parent.csend_type?) && + parent.arguments.include?(self) end def numeric_type? diff --git a/lib/rubocop/cop/layout/first_method_argument_line_break.rb b/lib/rubocop/cop/layout/first_method_argument_line_break.rb index 62b931c0527c..1c247bf761f8 100644 --- a/lib/rubocop/cop/layout/first_method_argument_line_break.rb +++ b/lib/rubocop/cop/layout/first_method_argument_line_break.rb @@ -46,7 +46,19 @@ def on_send(node) alias on_csend on_send def autocorrect(node) - EmptyLineCorrector.insert_before(node) + lambda do |corrector| + first_arg = if node.argument? + node + else + # In case of keyword arguments + node.parent + end + + block_start_col = first_arg.parent.source_range.column + + corrector.insert_before(first_arg.source_range, + "\n#{' ' * block_start_col}") + end end end end diff --git a/lib/rubocop/cop/mixin/first_element_line_break.rb b/lib/rubocop/cop/mixin/first_element_line_break.rb index 8a6ac90bec9a..a87f0f3e6378 100644 --- a/lib/rubocop/cop/mixin/first_element_line_break.rb +++ b/lib/rubocop/cop/mixin/first_element_line_break.rb @@ -21,6 +21,14 @@ def method_uses_parens?(node, limit) end def check_children_line_break(node, children, start = node) + return if children.empty? + + return add_offense(children.first) if children.first.multiline? + + check_multiple_children_line_break(children, start) + end + + def check_multiple_children_line_break(children, start) return if children.size < 2 line = start.first_line diff --git a/spec/rubocop/cop/layout/first_method_argument_line_break_spec.rb b/spec/rubocop/cop/layout/first_method_argument_line_break_spec.rb index d26e9aff7ebc..5838c121579b 100644 --- a/spec/rubocop/cop/layout/first_method_argument_line_break_spec.rb +++ b/spec/rubocop/cop/layout/first_method_argument_line_break_spec.rb @@ -95,6 +95,33 @@ end end + context 'single arg spanning multiple lines' do + it 'detects the offense' do + expect_offense(<<-RUBY.strip_indent) + foo([ + ^ Add a line break before the first argument of a multi-line method argument list. + ]) + RUBY + end + + it 'autocorrects the offense' do + new_source = autocorrect_source(<<-RUBY.strip_indent) + begin + foo([ + ]) + end + RUBY + + expect(new_source).to eq(<<-RUBY.strip_indent) + begin + foo( + [ + ]) + end + RUBY + end + end + it 'ignores arguments listed on a single line' do expect_no_offenses('foo(bar, baz, bing)') end