Skip to content

Commit

Permalink
[Fix #9520] Fix an incorrect auto-correct for Style/MultipleComparison
Browse files Browse the repository at this point in the history
Fixes #9520.

This PR fixes an incorrect auto-correct for `Style/MultipleComparison`
when comparing a variable with multiple items in `if` and `elsif` conditions.
  • Loading branch information
koic authored and bbatsov committed Feb 19, 2021
1 parent 8bd7679 commit d198e92
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
@@ -0,0 +1 @@
* [#9520](https://github.com/rubocop-hq/rubocop/issues/9520): Fix an incorrect auto-correct for `Style/MultipleComparison` when comparing a variable with multiple items in `if` and `elsif` conditions. ([@koic][])
18 changes: 16 additions & 2 deletions lib/rubocop/cop/style/multiple_comparison.rb
Expand Up @@ -47,11 +47,12 @@ class MultipleComparison < Base
'in a conditional, use `Array#include?` instead.'

def on_new_investigation
@compared_elements = []
@allowed_method_comparison = false
@last_comparison = nil
end

def on_or(node)
reset_comparison if switch_comparison?(node)

root_of_or_node = root_of_or_node(node)

return unless node == root_of_or_node
Expand All @@ -64,6 +65,8 @@ def on_or(node)

corrector.replace(node, prefer_method)
end

@last_comparison = node
end

private
Expand Down Expand Up @@ -131,6 +134,17 @@ def root_of_or_node(or_node)
end
end

def switch_comparison?(node)
return true if @last_comparison.nil?

@last_comparison.descendants.none? { |descendant| descendant == node }
end

def reset_comparison
@compared_elements = []
@allowed_method_comparison = false
end

def allow_method_comparison?
cop_config.fetch('AllowMethodComparison', true)
end
Expand Down
22 changes: 22 additions & 0 deletions spec/rubocop/cop/style/multiple_comparison_spec.rb
Expand Up @@ -96,6 +96,28 @@ def foo(x)
RUBY
end

it 'registers an offense and corrects when `a` is compared twice in `if` and `elsif` conditions' do
expect_offense(<<~RUBY)
def foo(a)
if a == 'foo' || a == 'bar'
^^^^^^^^^^^^^^^^^^^^^^^^ Avoid comparing a variable with multiple items in a conditional, use `Array#include?` instead.
elsif a == 'baz' || a == 'qux'
^^^^^^^^^^^^^^^^^^^^^^^^ Avoid comparing a variable with multiple items in a conditional, use `Array#include?` instead.
elsif a == 'quux'
end
end
RUBY

expect_correction(<<~RUBY)
def foo(a)
if ['foo', 'bar'].include?(a)
elsif ['baz', 'qux'].include?(a)
elsif a == 'quux'
end
end
RUBY
end

it 'does not register an offense for comparing multiple literal strings' do
expect_no_offenses(<<~RUBY)
if "a" == "a" || "a" == "c"
Expand Down

0 comments on commit d198e92

Please sign in to comment.