diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f11ffba641..6a50aba28ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -4804,3 +4805,4 @@ [@wcmonty]: https://github.com/wcmonty [@nguyenquangminh0711]: https://github.com/nguyenquangminh0711 [@chocolateboy]: https://github.com/chocolateboy +[@Lykos]: https://github.com/Lykos diff --git a/config/default.yml b/config/default.yml index 06c22411915..8027c697130 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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." diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 47b8a071f9f..cff2ee51ff8 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -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 diff --git a/lib/rubocop/cop/style/one_line_conditional.rb b/lib/rubocop/cop/style/one_line_conditional.rb index fee70f5c9c9..56c90a44471 100644 --- a/lib/rubocop/cop/style/one_line_conditional.rb +++ b/lib/rubocop/cop/style/one_line_conditional.rb @@ -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 `%s/then/else/end` constructs.' + MSG = 'Favor the ternary operator (`?:`) or multi-line constructs ' \ + 'over single-line `%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| @@ -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)} ? " \ diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 16b9e17f83e..fedf5a3513c 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -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 diff --git a/spec/rubocop/cop/style/one_line_conditional_spec.rb b/spec/rubocop/cop/style/one_line_conditional_spec.rb index 58f2df65018..3f6351ae95c 100644 --- a/spec/rubocop/cop/style/one_line_conditional_spec.rb +++ b/spec/rubocop/cop/style/one_line_conditional_spec.rb @@ -1,161 +1,478 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Style::OneLineConditional do - subject(:cop) { described_class.new } + subject(:cop) { described_class.new(config) } - context 'one line if/then/else/end' do - it 'registers an offense' do + let(:config) { RuboCop::Config.new(config_data) } + let(:config_data) { cop_config_data } + let(:cop_config_data) do + { + 'Style/OneLineConditional' => { + 'AlwaysCorrectToMultiline' => always_correct_to_multiline + } + } + end + let(:if_offense_message) do + 'Favor the ternary operator (`?:`) or multi-line constructs over single-line ' \ + '`if/then/else/end` constructs.' + end + let(:unless_offense_message) do + 'Favor the ternary operator (`?:`) or multi-line constructs over single-line ' \ + '`unless/then/else/end` constructs.' + end + + context 'when AlwaysCorrectToMultiline is false' do + let(:always_correct_to_multiline) { false } + + it 'registers and corrects an offense with ternary operator for if/then/else/end' do expect_offense(<<~RUBY) if cond then run else dont end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} RUBY + expect_correction(<<~RUBY) cond ? run : dont RUBY end - it 'allows empty else' do + it 'does not register an offense for if/then/else/end with empty else' do expect_no_offenses('if cond then run else end') end - end - context 'one line if/then/else/end when `then` branch has no body' do - it 'registers an offense' do + it 'registers and corrects an offense with ternary operator for if/then/else/end when ' \ + '`then` without body' do expect_offense(<<~RUBY) if cond then else dont end - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. + ^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} RUBY + expect_correction(<<~RUBY) cond ? nil : dont RUBY end - end - it 'allows one line if/then/end' do - expect_no_offenses('if cond then run end') - end + it 'does not register an offense for if/then/end' do + expect_no_offenses('if cond then run end') + end - context 'one line unless/then/else/end' do - it 'registers an offense' do + it 'registers and corrects an offense with ternary operator for unless/then/else/end' do expect_offense(<<~RUBY) unless cond then run else dont end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `unless/then/else/end` constructs. + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{unless_offense_message} RUBY + expect_correction(<<~RUBY) cond ? dont : run RUBY end - it 'allows empty else' do + it 'does not register an offense for unless/then/else/end with empty else' do expect_no_offenses('unless cond then run else end') end - end - it 'allows one line unless/then/end' do - expect_no_offenses('unless cond then run end') - end + it 'does not register an offense for unless/then/end' do + expect_no_offenses('unless cond then run end') + end + + %w[| ^ & <=> == === =~ > >= < <= << >> + - * / % ** ~ ! != !~ && ||].each do |operator| + it 'registers and corrects an offense with ternary operator and adding parentheses ' \ + 'when if/then/else/end is preceded by an operator' do + expect_offense(<<~RUBY, operator: operator) + a %{operator} if cond then run else dont end + _{operator} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + a #{operator} (cond ? run : dont) + RUBY + end + end - %w[| ^ & <=> == === =~ > >= < <= << >> + - * / % ** ~ ! != !~ - && ||].each do |operator| - it 'parenthesizes the expression if it is preceded by an operator' do - expect_offense(<<~RUBY, operator: operator) - a %{operator} if cond then run else dont end - _{operator} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. + shared_examples 'if/then/else/end with constructs changing precedence' do |expr| + it 'registers and corrects an offense with ternary operator and adding parentheses inside ' \ + "for if/then/else/end with `#{expr}` constructs inside inner branches" do + expect_offense(<<~RUBY, expr: expr) + if %{expr} then %{expr} else %{expr} end + ^^^^{expr}^^^^^^^{expr}^^^^^^^{expr}^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + (#{expr}) ? (#{expr}) : (#{expr}) + RUBY + end + end + + it_behaves_like 'if/then/else/end with constructs changing precedence', 'puts 1' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'defined? :A' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'yield a' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'super b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'not a' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a and b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a or b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a = b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a ? b : c' + + it 'registers and corrects an offense with ternary operator and adding parentheses for ' \ + 'if/then/else/end that contains method calls with unparenthesized arguments' do + expect_offense(<<~RUBY) + if check 1 then run 2 else dont_run 3 end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} RUBY + expect_correction(<<~RUBY) - a #{operator} (cond ? run : dont) + (check 1) ? (run 2) : (dont_run 3) RUBY end - end - shared_examples 'changed precedence' do |expr| - it "adds parentheses around `#{expr}`" do - expect_offense(<<~RUBY, expr: expr) - if %{expr} then %{expr} else %{expr} end - ^^^^{expr}^^^^^^^{expr}^^^^^^^{expr}^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. + it 'registers and corrects an offense with ternary operator without adding parentheses for ' \ + 'if/then/else/end that contains method calls with parenthesized arguments' do + expect_offense(<<~RUBY) + if a(0) then puts(1) else yield(2) end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} RUBY expect_correction(<<~RUBY) - (#{expr}) ? (#{expr}) : (#{expr}) + a(0) ? puts(1) : yield(2) RUBY end - end - it_behaves_like 'changed precedence', 'puts 1' - it_behaves_like 'changed precedence', 'defined? :A' - it_behaves_like 'changed precedence', 'yield a' - it_behaves_like 'changed precedence', 'super b' - it_behaves_like 'changed precedence', 'not a' - it_behaves_like 'changed precedence', 'a and b' - it_behaves_like 'changed precedence', 'a or b' - it_behaves_like 'changed precedence', 'a = b' - it_behaves_like 'changed precedence', 'a ? b : c' - - it 'does not parenthesize expressions when they do not contain method ' \ - 'calls with unparenthesized arguments' do - expect_offense(<<~RUBY) - if a(0) then puts(1) else yield(2) end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. - RUBY - - expect_correction(<<~RUBY) - a(0) ? puts(1) : yield(2) - RUBY - end + it 'registers and corrects an offense with ternary operator without adding parentheses for ' \ + 'if/then/else/end that contains unparenthesized operator method calls' do + expect_offense(<<~RUBY) + if 0 + 0 then 1 + 1 else 2 + 2 end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY - it 'does not parenthesize expressions when they contain unparenthesized ' \ - 'operator method calls' do - expect_offense(<<~RUBY) - if 0 + 0 then 1 + 1 else 2 + 2 end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. - RUBY + expect_correction(<<~RUBY) + 0 + 0 ? 1 + 1 : 2 + 2 + RUBY + end - expect_correction(<<~RUBY) - 0 + 0 ? 1 + 1 : 2 + 2 - RUBY - end + shared_examples 'if/then/else/end with keyword' do |keyword| + it 'registers and corrects an offense with ternary operator when one of the branches of ' \ + "if/then/else/end contains `#{keyword}` keyword" do + expect_offense(<<~RUBY, keyword: keyword) + if true then %{keyword} else 7 end + ^^^^^^^^^^^^^^{keyword}^^^^^^^^^^^ #{if_offense_message} + RUBY - it 'does not break when one of the branches contains a retry keyword' do - expect_offense(<<~RUBY) - if true then retry else 7 end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. - RUBY + expect_correction(<<~RUBY) + true ? #{keyword} : 7 + RUBY + end + end - expect_correction(<<~RUBY) - true ? retry : 7 - RUBY - end + it_behaves_like 'if/then/else/end with keyword', 'retry' + it_behaves_like 'if/then/else/end with keyword', 'break' + it_behaves_like 'if/then/else/end with keyword', 'self' + it_behaves_like 'if/then/else/end with keyword', 'raise' - it 'does not break when one of the branches contains a break keyword' do - expect_offense(<<~RUBY) - if true then break else 7 end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. - RUBY + it 'registers and corrects an offense with ternary operator when one of the branches of ' \ + 'if/then/else/end contains `next` keyword' do + expect_offense(<<~RUBY) + map { |line| if line.match(/^\s*#/) || line.strip.empty? then next else line end } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY - expect_correction(<<~RUBY) - true ? break : 7 - RUBY - end + expect_correction(<<~RUBY) + map { |line| (line.match(/^\s*#/) || line.strip.empty?) ? next : line } + RUBY + end + + it 'registers and corrects an offense with multi-line construct for ' \ + 'if-then-elsif-then-else-end' do + expect_offense(<<~RUBY) + if cond1 then run elsif cond2 then maybe else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY - it 'does not break when one of the branches contains a self keyword' do - expect_offense(<<~RUBY) - if true then self else 7 end - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. - RUBY + expect_correction(<<~RUBY) + if cond1 + run + elsif cond2 + maybe + else + dont + end + RUBY + end - expect_correction(<<~RUBY) - true ? self : 7 - RUBY + it 'registers and corrects an offense with multi-line construct for ' \ + 'if-then-elsif-then-elsif-then-else-end' do + expect_offense(<<~RUBY) + if cond1 then run elsif cond2 then maybe elsif cond3 then perhaps else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if cond1 + run + elsif cond2 + maybe + elsif cond3 + perhaps + else + dont + end + RUBY + end end - it 'does not break when one of the branches contains `next` keyword' do - expect_offense(<<~RUBY) - map{ |line| if line.match(/^\s*#/) || line.strip.empty? then next else line end } - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Favor the ternary operator (`?:`) over `if/then/else/end` constructs. - RUBY + context 'when AlwaysCorrectToMultiline is true' do + let(:always_correct_to_multiline) { true } + + it 'registers and corrects an offense with multi-line construct for if/then/else/end' do + expect_offense(<<~RUBY) + if cond then run else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if cond + run + else + dont + end + RUBY + end + + it 'does not register an offense for if/then/else/end with empty else' do + expect_no_offenses('if cond then run else end') + end + + it 'registers and corrects an offense with multi-line construct for if/then/else/end when ' \ + '`then` without body' do + expect_offense(<<~RUBY) + if cond then else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if cond + nil + else + dont + end + RUBY + end + + it 'does not register an offense for if/then/end' do + expect_no_offenses('if cond then run end') + end + + it 'registers and corrects an offense with multi-line construct for unless/then/else/end' do + expect_offense(<<~RUBY) + unless cond then run else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{unless_offense_message} + RUBY + + expect_correction(<<~RUBY) + unless cond + run + else + dont + end + RUBY + end + + it 'does not register an offense for unless/then/else/end with empty else' do + expect_no_offenses('unless cond then run else end') + end + + it 'does not register an offense for unless/then/end' do + expect_no_offenses('unless cond then run end') + end + + %w[| ^ & <=> == === =~ > >= < <= << >> + - * / % ** ~ ! != !~ && ||].each do |operator| + it 'registers and corrects an offense with multi-line construct without adding parentheses ' \ + 'when if/then/else/end is preceded by an operator' do + expect_offense(<<~RUBY, operator: operator) + a %{operator} if cond then run else dont end + _{operator} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + a #{operator} if cond + #{' ' * operator.length} run + #{' ' * operator.length} else + #{' ' * operator.length} dont + #{' ' * operator.length} end + RUBY + end + end + + shared_examples 'if/then/else/end with constructs changing precedence' do |expr| + it 'registers and corrects an offense with multi-line construct without adding ' \ + "parentheses for if/then/else/end with `#{expr}` constructs inside inner branches" do + expect_offense(<<~RUBY, expr: expr) + if %{expr} then %{expr} else %{expr} end + ^^^^{expr}^^^^^^^{expr}^^^^^^^{expr}^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if #{expr} + #{expr} + else + #{expr} + end + RUBY + end + end + + it_behaves_like 'if/then/else/end with constructs changing precedence', 'puts 1' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'defined? :A' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'yield a' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'super b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'not a' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a and b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a or b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a = b' + it_behaves_like 'if/then/else/end with constructs changing precedence', 'a ? b : c' + + it 'registers and corrects an offense with multi-line construct without adding parentheses ' \ + 'for if/then/else/end that contains method calls with unparenthesized arguments' do + expect_offense(<<~RUBY) + if check 1 then run 2 else dont_run 3 end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if check 1 + run 2 + else + dont_run 3 + end + RUBY + end + + it 'registers and corrects an offense with multi-line construct without adding parentheses for ' \ + 'if/then/else/end that contains method calls with parenthesized arguments' do + expect_offense(<<~RUBY) + if a(0) then puts(1) else yield(2) end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if a(0) + puts(1) + else + yield(2) + end + RUBY + end + + it 'registers and corrects an offense with multi-line construct without adding parentheses for ' \ + 'if/then/else/end that contains unparenthesized operator method calls' do + expect_offense(<<~RUBY) + if 0 + 0 then 1 + 1 else 2 + 2 end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if 0 + 0 + 1 + 1 + else + 2 + 2 + end + RUBY + end + + shared_examples 'if/then/else/end with keyword' do |keyword| + it 'registers and corrects an offense with multi-line construct when one of the branches ' \ + "of if/then/else/end contains `#{keyword}` keyword" do + expect_offense(<<~RUBY, keyword: keyword) + if true then %{keyword} else 7 end + ^^^^^^^^^^^^^^{keyword}^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if true + #{keyword} + else + 7 + end + RUBY + end + end + + it_behaves_like 'if/then/else/end with keyword', 'retry' + it_behaves_like 'if/then/else/end with keyword', 'break' + it_behaves_like 'if/then/else/end with keyword', 'self' + it_behaves_like 'if/then/else/end with keyword', 'raise' + + it 'registers and corrects an offense with multi-line construct when one of the branches of ' \ + 'if/then/else/end contains `next` keyword' do + expect_offense(<<~RUBY) + map { |line| if line.match(/^\s*#/) || line.strip.empty? then next else line end } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + map { |line| if line.match(/^\s*#/) || line.strip.empty? + next + else + line + end } + RUBY + end + + it 'registers and corrects an offense with multi-line construct for ' \ + 'if-then-elsif-then-else-end' do + expect_offense(<<~RUBY) + if cond1 then run elsif cond2 then maybe else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if cond1 + run + elsif cond2 + maybe + else + dont + end + RUBY + end + + it 'registers and corrects an offense with multi-line construct for ' \ + 'if-then-elsif-then-elsif-then-else-end' do + expect_offense(<<~RUBY) + if cond1 then run elsif cond2 then maybe elsif cond3 then perhaps else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY - expect_correction(<<~RUBY) - map{ |line| (line.match(/^ *#/) || line.strip.empty?) ? next : line } - RUBY + expect_correction(<<~RUBY) + if cond1 + run + elsif cond2 + maybe + elsif cond3 + perhaps + else + dont + end + RUBY + end + + context 'when IndentationWidth differs from default' do + let(:config_data) do + cop_config_data.merge('Layout/IndentationWidth' => { 'Width' => 4 }) + end + + it 'registers and corrects an offense with multi-line construct for if/then/else/end' do + expect_offense(<<~RUBY) + if cond then run else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{if_offense_message} + RUBY + + expect_correction(<<~RUBY) + if cond + run + else + dont + end + RUBY + end + end end end