Skip to content

Commit

Permalink
Fix an incorrect auto-correct for Style/MultilineTernaryOperator
Browse files Browse the repository at this point in the history
This PR fixes the following incorrect auto-correct for
`Style/MultilineTernaryOperator` when returning
a multiline ternary operator expression.

```console
% cat example.rb
return distance_in_minutes == 0 ?
       locale.t(:less_than_x_minutes, count: 1) :
       locale.t(:x_minutes, count: distance_in_minutes)

% bundle exec rubocop -a --only Style/MultilineTernaryOperator
(snip)

Inspecting 2 files
C.

Offenses:

example.rb:1:8: C: [Corrected] Style/MultilineTernaryOperator: Avoid
multi-line ternary operators, use if or unless instead.
return distance_in_minutes == 0 ? ...
       ^^^^^^^^^^^^^^^^^^^^^^^^^^

2 files inspected, 1 offense detected, 1 offense corrected

% cat example.rb
return if distance_in_minutes == 0
  locale.t(:less_than_x_minutes, count: 1)
else
  locale.t(:x_minutes, count: distance_in_minutes)
end

% ruby -c example.rb
example.rb:3: syntax error, unexpected `else', expecting end-of-input
```

I found it with the following code.
https://github.com/rails/rails/blob/v6.0.3.2/actionview/lib/action_view/helpers/date_helper.rb#L109-L111
  • Loading branch information
koic committed Sep 13, 2020
1 parent 5ff83df commit 8a1776f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -32,6 +32,7 @@
* [#8626](https://github.com/rubocop-hq/rubocop/issues/8626): Fix false negatives for `Lint/UselessMethodDefinition`. ([@marcandre][])
* [#8698](https://github.com/rubocop-hq/rubocop/pull/8698): Fix cache to avoid encoding exception. ([@marcandre][])
* [#8704](https://github.com/rubocop-hq/rubocop/issues/8704): Fix an error for `Lint/AmbiguousOperator` when using safe navigation operator with a unary operator. ([@koic][])
* [#8661](https://github.com/rubocop-hq/rubocop/pull/8661): Fix an incorrect auto-correct for `Style/MultilineTernaryOperator` when returning a multiline ternary operator expression. ([@koic][])

### Changes

Expand Down
3 changes: 3 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -5757,6 +5757,9 @@ end

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.

=== Examples

[source,ruby]
Expand Down
15 changes: 14 additions & 1 deletion lib/rubocop/cop/style/multiline_ternary_operator.rb
Expand Up @@ -5,6 +5,9 @@ module Cop
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.
#
# @example
# # bad
# a = cond ?
Expand All @@ -29,9 +32,13 @@ class MultilineTernaryOperator < Base
'use `if` or `unless` instead.'

def on_if(node)
return unless node.ternary? && node.multiline?
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?

corrector.replace(node, <<~RUBY.chop)
if #{node.condition.source}
#{node.if_branch.source}
Expand All @@ -41,6 +48,12 @@ def on_if(node)
RUBY
end
end

private

def offense?(node)
node.ternary? && node.multiline?
end
end
end
end
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/style/multiline_ternary_operator_spec.rb
Expand Up @@ -53,6 +53,17 @@
RUBY
end

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

expect_no_corrections
end

it 'accepts a single line ternary operator expression' do
expect_no_offenses('a = cond ? b : c')
end
Expand Down

0 comments on commit 8a1776f

Please sign in to comment.