Skip to content

Commit

Permalink
[Fix rubocop#7705] Fix Style/OneLineConditional to handle if/then/e…
Browse files Browse the repository at this point in the history
…lsif/then/else and add AlwaysCorrectToMultiline config option
  • Loading branch information
Dmytro Savochkin committed Aug 24, 2020
1 parent 7836c61 commit 4f8bf2b
Show file tree
Hide file tree
Showing 6 changed files with 521 additions and 133 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -23,6 +23,7 @@
* [#8524](https://github.com/rubocop-hq/rubocop/issues/8524): Fix `Layout/EmptyLinesAroundClassBody` and `Layout/EmptyLinesAroundModuleBody` to correctly handle an access modifier as a first child. ([@dsavochkin][])
* [#8518](https://github.com/rubocop-hq/rubocop/issues/8518): Fix `Lint/ConstantResolution` cop reporting offense for `module` and `class` definitions. ([@tejasbubane][])
* [#8158](https://github.com/rubocop-hq/rubocop/issues/8158): Fix `Style/MultilineWhenThen` cop to correctly handle cases with multiline body. ([@dsavochkin][])
* [#7705](https://github.com/rubocop-hq/rubocop/issues/7705): Fix `Style/OneLineConditional` cop to handle if/then/elsif/then/else/end cases. Add `AlwaysCorrectToMultiline` config option to this cop to always convert offenses to the multi-line form (false by default). ([@Lykos][], [@dsavochkin][])

### Changes

Expand Down Expand Up @@ -4804,3 +4805,4 @@
[@wcmonty]: https://github.com/wcmonty
[@nguyenquangminh0711]: https://github.com/nguyenquangminh0711
[@chocolateboy]: https://github.com/chocolateboy
[@Lykos]: https://github.com/Lykos
7 changes: 4 additions & 3 deletions config/default.yml
Expand Up @@ -3636,12 +3636,13 @@ Style/NumericPredicate:

Style/OneLineConditional:
Description: >-
Favor the ternary operator(?:) over
if/then/else/end constructs.
Favor the ternary operator (?:) or multi-line constructs over
single-line if/then/else/end constructs.
StyleGuide: '#ternary-operator'
Enabled: true
AlwaysCorrectToMultiline: false
VersionAdded: '0.9'
VersionChanged: '0.38'
VersionChanged: '0.90'

Style/OptionHash:
Description: "Don't use option hashes when you can use keyword arguments."
Expand Down
38 changes: 28 additions & 10 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -6665,33 +6665,51 @@ bar.baz > 0
| Yes
| Yes
| 0.9
| 0.38
| 0.90
|===

TODO: Make configurable.
Checks for uses of if/then/else/end on a single line.
Checks for uses of if/then/else/end constructs on a single line.
AlwaysCorrectToMultiline config option can be set to true to auto-convert all offenses to
multi-line constructs. When AlwaysCorrectToMultiline is false (default case) the
auto-correct will first try converting them to ternary operators.

=== Examples

[source,ruby]
----
# bad
if foo then boo else doo end
unless foo then boo else goo end
if foo then bar else baz end
# bad
unless foo then baz else bar end
# good
foo ? bar : baz
# good
foo ? boo : doo
boo if foo
if foo then boo end
bar if foo
# good
if foo then bar end
# good
if foo
boo
bar
else
doo
baz
end
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| AlwaysCorrectToMultiline
| `false`
| Boolean
|===

=== References

* https://rubystyle.guide#ternary-operator
Expand Down
84 changes: 67 additions & 17 deletions lib/rubocop/cop/style/one_line_conditional.rb
Expand Up @@ -3,34 +3,45 @@
module RuboCop
module Cop
module Style
# TODO: Make configurable.
# Checks for uses of if/then/else/end on a single line.
# Checks for uses of if/then/else/end constructs on a single line.
# AlwaysCorrectToMultiline config option can be set to true to auto-convert all offenses to
# multi-line constructs. When AlwaysCorrectToMultiline is false (default case) the
# auto-correct will first try converting them to ternary operators.
#
# @example
# # bad
# if foo then boo else doo end
# unless foo then boo else goo end
# if foo then bar else baz end
#
# # bad
# unless foo then baz else bar end
#
# # good
# foo ? bar : baz
#
# # good
# foo ? boo : doo
# boo if foo
# if foo then boo end
# bar if foo
#
# # good
# if foo then bar end
#
# # good
# if foo
# boo
# bar
# else
# doo
# baz
# end
class OneLineConditional < Base
include ConfigurableEnforcedStyle
include OnNormalIfUnless
extend AutoCorrector

MSG = 'Favor the ternary operator (`?:`) ' \
'over `%<keyword>s/then/else/end` constructs.'
MSG = 'Favor the ternary operator (`?:`) or multi-line constructs ' \
'over single-line `%<keyword>s/then/else/end` constructs.'

def on_normal_if_unless(node)
return unless node.single_line? && node.else_branch
return unless node.single_line?
return unless node.else_branch
return if node.elsif?

message = message(node)
add_offense(node, message: message) do |corrector|
Expand All @@ -45,16 +56,55 @@ def message(node)
end

def replacement(node)
return to_ternary(node) unless node.parent
if always_multiline? || cannot_replace_to_ternary?(node)
multiline_replacement(node)
else
replaced_node = ternary_replacement(node)
return replaced_node unless node.parent
return "(#{replaced_node})" if %i[and or].include?(node.parent.type)
return "(#{replaced_node})" if node.parent.send_type? && node.parent.operator_method?

replaced_node
end
end

return "(#{to_ternary(node)})" if %i[and or].include?(node.parent.type)
def always_multiline?
@config.for_cop('Style/OneLineConditional')['AlwaysCorrectToMultiline']
end

def cannot_replace_to_ternary?(node)
node.elsif_conditional?
end

return "(#{to_ternary(node)})" if node.parent.send_type? && node.parent.operator_method?
def multiline_replacement(node, indentation = nil)
indentation = ' ' * node.source_range.column if indentation.nil?
if_branch_source = node.if_branch&.source || 'nil'
elsif_indentation = indentation if node.respond_to?(:elsif?) && node.elsif?
if_branch = <<~RUBY
#{elsif_indentation}#{node.keyword} #{node.condition.source}
#{indentation}#{branch_body_indentation}#{if_branch_source}
RUBY
else_branch = else_branch_to_multiline(node.else_branch, indentation)
if_branch + else_branch
end

def else_branch_to_multiline(else_branch, indentation)
if else_branch.if_type? && else_branch.elsif?
multiline_replacement(else_branch, indentation)
else
<<~RUBY.chomp
#{indentation}else
#{indentation}#{branch_body_indentation}#{else_branch.source}
#{indentation}end
RUBY
end
end

to_ternary(node)
def branch_body_indentation
' ' * (@config.for_cop('Layout/IndentationWidth')['Width'] || 2)
end

def to_ternary(node)
def ternary_replacement(node)
condition, if_branch, else_branch = *node

"#{expr_replacement(condition)} ? " \
Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cli_spec.rb
Expand Up @@ -175,8 +175,8 @@ def and_with_args
"if it's surely a splat operator, or add a whitespace to the " \
'right of the `*` if it should be a multiplication.',
"#{abs('example.rb')}:4:1: C: Style/OneLineConditional: " \
'Favor the ternary operator (`?:`) over `if/then/else/end` ' \
'constructs.',
'Favor the ternary operator (`?:`) or multi-line constructs over ' \
'single-line `if/then/else/end` constructs.',
''].join("\n"))
end
end
Expand Down

0 comments on commit 4f8bf2b

Please sign in to comment.