diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d42748dba1..32dc04f25fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5178,3 +5178,4 @@ [@HeroProtagonist]: https://github.com/HeroProtagonist [@piotrmurach]: https://github.com/piotrmurach [@javierav]: https://github.com/javierav +[@adrian-rivera]: https://github.com/adrian-rivera diff --git a/changelog/fix_if_with_semicolon_correction.md b/changelog/fix_if_with_semicolon_correction.md new file mode 100644 index 00000000000..ffd5a6c620d --- /dev/null +++ b/changelog/fix_if_with_semicolon_correction.md @@ -0,0 +1 @@ +* [#8820](https://github.com/rubocop-hq/rubocop/issues/8820): Fixes `IfWithSemicolon` autocorrection when `elsif` is present. ([@adrian-rivera][], [@dvandersluis][]) diff --git a/lib/rubocop/cop/style/if_with_semicolon.rb b/lib/rubocop/cop/style/if_with_semicolon.rb index ff3676ca78f..1577563cdff 100644 --- a/lib/rubocop/cop/style/if_with_semicolon.rb +++ b/lib/rubocop/cop/style/if_with_semicolon.rb @@ -17,26 +17,61 @@ class IfWithSemicolon < Base include OnNormalIfUnless extend AutoCorrector - MSG = 'Do not use if x; Use the ternary operator instead.' + MSG_IF_ELSE = 'Do not use `if %s;` - use `if/else` instead.' + MSG_TERNARY = 'Do not use `if %s;` - use a ternary operator instead.' def on_normal_if_unless(node) return unless node.else_branch + return if node.parent&.if_type? beginning = node.loc.begin return unless beginning&.is?(';') - add_offense(node) do |corrector| - corrector.replace(node, correct_to_ternary(node)) + message = node.else_branch.if_type? ? MSG_IF_ELSE : MSG_TERNARY + + add_offense(node, message: format(message, expr: node.condition.source)) do |corrector| + corrector.replace(node, autocorrect(node)) end end private - def correct_to_ternary(node) + def autocorrect(node) + return correct_elsif(node) if node.else_branch.if_type? + else_code = node.else_branch ? node.else_branch.source : 'nil' "#{node.condition.source} ? #{node.if_branch.source} : #{else_code}" end + + def correct_elsif(node) + <<~RUBY.chop + if #{node.condition.source} + #{node.if_branch.source} + #{build_else_branch(node.else_branch).chop} + end + RUBY + end + + def build_else_branch(second_condition) + result = <<~RUBY + elsif #{second_condition.condition.source} + #{second_condition.if_branch.source} + RUBY + + if second_condition.else_branch + result += if second_condition.else_branch.if_type? + build_else_branch(second_condition.else_branch) + else + <<~RUBY + else + #{second_condition.else_branch.source} + RUBY + end + end + + result + end end end end diff --git a/spec/rubocop/cop/style/if_with_semicolon_spec.rb b/spec/rubocop/cop/style/if_with_semicolon_spec.rb index 11c6ce62de5..2959d8e6a2a 100644 --- a/spec/rubocop/cop/style/if_with_semicolon_spec.rb +++ b/spec/rubocop/cop/style/if_with_semicolon_spec.rb @@ -6,7 +6,7 @@ it 'registers an offense and corrects for one line if/;/end' do expect_offense(<<~RUBY) if cond; run else dont end - ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use if x; Use the ternary operator instead. + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `if cond;` - use a ternary operator instead. RUBY expect_correction(<<~RUBY) @@ -28,4 +28,57 @@ class Hash end if RUBY_VERSION < "1.8.7" RUBY end + + context 'when elsif is present' do + it 'accepts without `else` branch' do + expect_offense(<<~RUBY) + if cond; run elsif cond2; run2 end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `if cond;` - use `if/else` instead. + RUBY + + expect_correction(<<~RUBY) + if cond + run + elsif cond2 + run2 + end + RUBY + end + + it 'accepts second elsif block' do + expect_offense(<<~RUBY) + if cond; run elsif cond2; run2 elsif cond3; run3 else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `if cond;` - use `if/else` instead. + RUBY + + expect_correction(<<~RUBY) + if cond + run + elsif cond2 + run2 + elsif cond3 + run3 + else + dont + end + RUBY + end + + it 'accepts with `else` branch' do + expect_offense(<<~RUBY) + if cond; run elsif cond2; run2 else dont end + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `if cond;` - use `if/else` instead. + RUBY + + expect_correction(<<~RUBY) + if cond + run + elsif cond2 + run2 + else + dont + end + RUBY + end + end end