Skip to content

Commit

Permalink
Fix #8820 autocorrection for Style/IfWithSemicolon
Browse files Browse the repository at this point in the history
This change fixes autocorrection for Style/IfWithSemicolon when elsif present.

In case of one of more elsif conditions present, the autocorrector will create a full if, elsif, else structure.
  • Loading branch information
adrian-rivera authored and bbatsov committed Nov 30, 2020
1 parent 708e46e commit a05c2e6
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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
1 change: 1 addition & 0 deletions 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][])
43 changes: 39 additions & 4 deletions lib/rubocop/cop/style/if_with_semicolon.rb
Expand Up @@ -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 %<expr>s;` - use `if/else` instead.'
MSG_TERNARY = 'Do not use `if %<expr>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
Expand Down
55 changes: 54 additions & 1 deletion spec/rubocop/cop/style/if_with_semicolon_spec.rb
Expand Up @@ -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)
Expand All @@ -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

0 comments on commit a05c2e6

Please sign in to comment.