diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d6b6222fe2..eac5ebfe063 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/default.yml b/config/default.yml index 4dc2688dd18..e137d8fe264 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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.' diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 07387d7d540..b3752a7e447 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -6010,9 +6010,9 @@ end | Enabled | Yes -| No +| Yes | 0.49 -| - +| 1.1 |=== This cop checks against comparing a variable with multiple items, where diff --git a/lib/rubocop/cop/style/multiple_comparison.rb b/lib/rubocop/cop/style/multiple_comparison.rb index fd5e839960a..825c4085f33 100644 --- a/lib/rubocop/cop/style/multiple_comparison.rb +++ b/lib/rubocop/cop/style/multiple_comparison.rb @@ -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) @@ -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 @@ -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) diff --git a/spec/rubocop/cop/style/multiple_comparison_spec.rb b/spec/rubocop/cop/style/multiple_comparison_spec.rb index 77158b55355..c188bced9e1 100644 --- a/spec/rubocop/cop/style/multiple_comparison_spec.rb +++ b/spec/rubocop/cop/style/multiple_comparison_spec.rb @@ -14,7 +14,7 @@ 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" @@ -22,9 +22,16 @@ 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" @@ -32,9 +39,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 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" @@ -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" @@ -54,9 +75,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 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) @@ -64,6 +92,12 @@ def foo(x) ^^^^^^^^^^^^^^^^^^^^^^^^^^ 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