Skip to content

Commit

Permalink
Fix an incorrect auto-correct for Style/MultilineTernaryOperator
Browse files Browse the repository at this point in the history
Follow up rubocop#8661.

This PR fixes an incorrect auto-correct for `Style/MultilineTernaryOperator`
when returning a multiline ternary operator expression.
Using `break`, `next`, or method call also causes the same syntax error as `return`.

And this PR will change these auto-correct to single-line ternary operator as valid syntax.
  • Loading branch information
koic committed Apr 15, 2022
1 parent a4821e2 commit 69ed7d5
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 14 deletions.
@@ -0,0 +1 @@
* [#10537](https://github.com/rubocop/rubocop/pull/10537): Fix an incorrect auto-correct for `Style/MultilineTernaryOperator` when returning a multiline ternary operator expression with `break`, `next`, or method call. ([@koic][])
44 changes: 33 additions & 11 deletions lib/rubocop/cop/style/multiline_ternary_operator.rb
Expand Up @@ -6,7 +6,8 @@ module Style
# This cop checks for multi-line ternary op expressions.
#
# NOTE: `return if ... else ... end` is syntax error. If `return` is used before
# multiline ternary operator expression, it cannot be auto-corrected.
# multiline ternary operator expression, it will be auto-corrected to single-line
# ternary operator. The same is true for `break`, `next`, and method call.
#
# @example
# # bad
Expand All @@ -18,27 +19,50 @@ module Style
# b :
# c
#
# return cond ?
# b :
# c
#
# # good
# a = cond ? b : c
# a = if cond
# b
# else
# c
# end
#
# return cond ? b : c
#
class MultilineTernaryOperator < Base
extend AutoCorrector

MSG = 'Avoid multi-line ternary operators, use `if` or `unless` instead.'
MSG_IF = 'Avoid multi-line ternary operators, use `if` or `unless` instead.'
MSG_SINGLE_LINE = 'Avoid multi-line ternary operators, use single-line instead.'
SINGLE_LINE_TYPES = %i[return break next send].freeze

def on_if(node)
return unless offense?(node)

add_offense(node) do |corrector|
# `return if ... else ... end` is syntax error. If `return` is used before
# multiline ternary operator expression, it cannot be auto-corrected.
next unless offense?(node) && !node.parent.return_type?
message = enforce_single_line_ternary_operator?(node) ? MSG_SINGLE_LINE : MSG_IF

add_offense(node, message: message) do |corrector|
next unless offense?(node)

corrector.replace(node, replacement(node))
end
end

private

corrector.replace(node, <<~RUBY.chop)
def offense?(node)
node.ternary? && node.multiline?
end

def replacement(node)
if enforce_single_line_ternary_operator?(node)
"#{node.condition.source} ? #{node.if_branch.source} : #{node.else_branch.source}"
else
<<~RUBY.chop
if #{node.condition.source}
#{node.if_branch.source}
else
Expand All @@ -48,10 +72,8 @@ def on_if(node)
end
end

private

def offense?(node)
node.ternary? && node.multiline?
def enforce_single_line_ternary_operator?(node)
SINGLE_LINE_TYPES.include?(node.parent.type)
end
end
end
Expand Down
47 changes: 44 additions & 3 deletions spec/rubocop/cop/style/multiline_ternary_operator_spec.rb
Expand Up @@ -51,15 +51,56 @@
RUBY
end

it 'register an offense and does not auto-correct when returning a multiline ternary operator expression' do
it 'register an offense and corrects when returning a multiline ternary operator expression with `return`' do
expect_offense(<<~RUBY)
return cond ?
^^^^^^ Avoid multi-line ternary operators, use `if` or `unless` instead.
^^^^^^ Avoid multi-line ternary operators, use single-line instead.
foo :
bar
RUBY

expect_no_corrections
expect_correction(<<~RUBY)
return cond ? foo : bar
RUBY
end

it 'register an offense and corrects when returning a multiline ternary operator expression with `break`' do
expect_offense(<<~RUBY)
break cond ?
^^^^^^ Avoid multi-line ternary operators, use single-line instead.
foo :
bar
RUBY

expect_correction(<<~RUBY)
break cond ? foo : bar
RUBY
end

it 'register an offense and corrects when returning a multiline ternary operator expression with `next`' do
expect_offense(<<~RUBY)
next cond ?
^^^^^^ Avoid multi-line ternary operators, use single-line instead.
foo :
bar
RUBY

expect_correction(<<~RUBY)
next cond ? foo : bar
RUBY
end

it 'register an offense and corrects when returning a multiline ternary operator expression with method call' do
expect_offense(<<~RUBY)
do_something cond ?
^^^^^^ Avoid multi-line ternary operators, use single-line instead.
foo :
bar
RUBY

expect_correction(<<~RUBY)
do_something cond ? foo : bar
RUBY
end

it 'accepts a single line ternary operator expression' do
Expand Down

0 comments on commit 69ed7d5

Please sign in to comment.