From 8a1776fe2e862c01323fd6bbfd001e411a8f56df Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 8 Sep 2020 00:16:24 +0900 Subject: [PATCH] Fix an incorrect auto-correct for `Style/MultilineTernaryOperator` 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 --- CHANGELOG.md | 1 + docs/modules/ROOT/pages/cops_style.adoc | 3 +++ .../cop/style/multiline_ternary_operator.rb | 15 ++++++++++++++- .../cop/style/multiline_ternary_operator_spec.rb | 11 +++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dd0a103146..e16628a2302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 8a1a4ffbee1..4de9a927951 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -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] diff --git a/lib/rubocop/cop/style/multiline_ternary_operator.rb b/lib/rubocop/cop/style/multiline_ternary_operator.rb index 1b55cd54ec2..dc679f4d87f 100644 --- a/lib/rubocop/cop/style/multiline_ternary_operator.rb +++ b/lib/rubocop/cop/style/multiline_ternary_operator.rb @@ -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 ? @@ -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} @@ -41,6 +48,12 @@ def on_if(node) RUBY end end + + private + + def offense?(node) + node.ternary? && node.multiline? + end end end end diff --git a/spec/rubocop/cop/style/multiline_ternary_operator_spec.rb b/spec/rubocop/cop/style/multiline_ternary_operator_spec.rb index ddd4bcdac0c..2190c842f5d 100644 --- a/spec/rubocop/cop/style/multiline_ternary_operator_spec.rb +++ b/spec/rubocop/cop/style/multiline_ternary_operator_spec.rb @@ -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