Skip to content

Commit

Permalink
Merge pull request #8951 from koic/support_autocorrection_for_style_m…
Browse files Browse the repository at this point in the history
…ultiple_comparison

Support auto-correction for `Style/MultipleComparison`
  • Loading branch information
koic committed Oct 27, 2020
2 parents 21e4062 + 07e431c commit 8c02b2f
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@
* [#8934](https://github.com/rubocop-hq/rubocop/pull/8934): Add new `Style/SwapValues` cop. ([@fatkodima][])
* [#7549](https://github.com/rubocop-hq/rubocop/issues/7549): Add new `Style/ArgumentsForwarding` cop. ([@koic][])
* [#8859](https://github.com/rubocop-hq/rubocop/issues/8859): Add new `Lint/UnmodifiedReduceAccumulator` cop. ([@dvandersluis][])
* [#8951](https://github.com/rubocop-hq/rubocop/pull/8951): Support auto-correction for `Style/MultipleComparison`. ([@koic][])

### Bug fixes

Expand Down
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -3563,6 +3563,7 @@ Style/MultipleComparison:
use Array#include? instead.
Enabled: true
VersionAdded: '0.49'
VersionChanged: '1.1'

Style/MutableConstant:
Description: 'Do not assign mutable objects to constants.'
Expand Down
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -6010,9 +6010,9 @@ end

| Enabled
| Yes
| No
| Yes
| 0.49
| -
| 1.1
|===

This cop checks against comparing a variable with multiple items, where
Expand Down
27 changes: 21 additions & 6 deletions lib/rubocop/cop/style/multiple_comparison.rb
Expand Up @@ -17,24 +17,37 @@ module Style
# foo if ['a', 'b', 'c'].include?(a)
# foo if a == b.lightweight || a == b.heavyweight
class MultipleComparison < Base
extend AutoCorrector

MSG = 'Avoid comparing a variable with multiple items ' \
'in a conditional, use `Array#include?` instead.'

def on_new_investigation
@compared_elements = []
end

def on_or(node)
root_of_or_node = root_of_or_node(node)

return unless node == root_of_or_node
return unless nested_variable_comparison?(root_of_or_node)

add_offense(node)
add_offense(node) do |corrector|
elements = @compared_elements.join(', ')
prefer_method = "[#{elements}].include?(#{variables_in_node(node).first})"

corrector.replace(node, prefer_method)
end
end

private

def_node_matcher :simple_double_comparison?, '(send $lvar :== $lvar)'
def_node_matcher :simple_comparison?, <<~PATTERN
{(send $lvar :== !send)
(send !send :== $lvar)}
def_node_matcher :simple_comparison_lhs?, <<~PATTERN
(send $lvar :== $!send)
PATTERN
def_node_matcher :simple_comparison_rhs?, <<~PATTERN
(send $!send :== $lvar)
PATTERN

def nested_variable_comparison?(node)
Expand All @@ -57,9 +70,11 @@ def variables_in_simple_node(node)
simple_double_comparison?(node) do |var1, var2|
return [variable_name(var1), variable_name(var2)]
end
simple_comparison?(node) do |var|
if (var, obj = simple_comparison_lhs?(node)) || (obj, var = simple_comparison_rhs?(node))
@compared_elements << obj.source
return [variable_name(var)]
end

[]
end

Expand All @@ -76,7 +91,7 @@ def nested_comparison?(node)
end

def comparison?(node)
simple_comparison?(node) || nested_comparison?(node)
simple_comparison_lhs?(node) || simple_comparison_rhs?(node) || nested_comparison?(node)
end

def root_of_or_node(or_node)
Expand Down
44 changes: 39 additions & 5 deletions spec/rubocop/cop/style/multiple_comparison_spec.rb
Expand Up @@ -14,27 +14,41 @@
RUBY
end

it 'registers an offense when `a` is compared twice' do
it 'registers an offense and corrects when `a` is compared twice' do
expect_offense(<<~RUBY)
a = "a"
if a == "a" || a == "b"
^^^^^^^^^^^^^^^^^^^^ Avoid comparing a variable with multiple items in a conditional, use `Array#include?` instead.
print a
end
RUBY

expect_correction(<<~RUBY)
a = "a"
if ["a", "b"].include?(a)
print a
end
RUBY
end

it 'registers an offense when `a` is compared three times' do
it 'registers an offense and corrects when `a` is compared three times' do
expect_offense(<<~RUBY)
a = "a"
if a == "a" || a == "b" || a == "c"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid comparing a variable with multiple items in a conditional, use `Array#include?` instead.
print a
end
RUBY

expect_correction(<<~RUBY)
a = "a"
if ["a", "b", "c"].include?(a)
print a
end
RUBY
end

it 'registers an offense when `a` is compared three times on the right ' \
it 'registers an offense and corrects when `a` is compared three times on the right ' \
'hand side' do
expect_offense(<<~RUBY)
a = "a"
Expand All @@ -43,9 +57,16 @@
print a
end
RUBY

expect_correction(<<~RUBY)
a = "a"
if ["a", "b", "c"].include?(a)
print a
end
RUBY
end

it 'registers an offense when `a` is compared three times, once on the ' \
it 'registers an offense and corrects when `a` is compared three times, once on the ' \
'righthand side' do
expect_offense(<<~RUBY)
a = "a"
Expand All @@ -54,16 +75,29 @@
print a
end
RUBY

expect_correction(<<~RUBY)
a = "a"
if ["a", "b", "c"].include?(a)
print a
end
RUBY
end

it 'registers an offense when multiple comparison is not ' \
it 'registers an offense and corrects when multiple comparison is not ' \
'part of a conditional' do
expect_offense(<<~RUBY)
def foo(x)
x == 1 || x == 2 || x == 3
^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid comparing a variable with multiple items in a conditional, use `Array#include?` instead.
end
RUBY

expect_correction(<<~RUBY)
def foo(x)
[1, 2, 3].include?(x)
end
RUBY
end

it 'does not register an offense and corrects when using multiple method calls' do
Expand Down

0 comments on commit 8c02b2f

Please sign in to comment.