From d198e92da90b6d43afdf08f5135dc11b4a417280 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 17 Feb 2021 01:11:52 +0900 Subject: [PATCH] [Fix #9520] Fix an incorrect auto-correct for `Style/MultipleComparison` Fixes #9520. This PR fixes an incorrect auto-correct for `Style/MultipleComparison` when comparing a variable with multiple items in `if` and `elsif` conditions. --- ...tocorrect_for_style_multiple_comparison.md | 1 + lib/rubocop/cop/style/multiple_comparison.rb | 18 +++++++++++++-- .../cop/style/multiple_comparison_spec.rb | 22 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 changelog/fix_incorrect_autocorrect_for_style_multiple_comparison.md diff --git a/changelog/fix_incorrect_autocorrect_for_style_multiple_comparison.md b/changelog/fix_incorrect_autocorrect_for_style_multiple_comparison.md new file mode 100644 index 00000000000..14d2fe71bfa --- /dev/null +++ b/changelog/fix_incorrect_autocorrect_for_style_multiple_comparison.md @@ -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][]) diff --git a/lib/rubocop/cop/style/multiple_comparison.rb b/lib/rubocop/cop/style/multiple_comparison.rb index 7477bb81677..31a1096702c 100644 --- a/lib/rubocop/cop/style/multiple_comparison.rb +++ b/lib/rubocop/cop/style/multiple_comparison.rb @@ -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 @@ -64,6 +65,8 @@ def on_or(node) corrector.replace(node, prefer_method) end + + @last_comparison = node end private @@ -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 diff --git a/spec/rubocop/cop/style/multiple_comparison_spec.rb b/spec/rubocop/cop/style/multiple_comparison_spec.rb index d1aa67a709b..86efb09e9ec 100644 --- a/spec/rubocop/cop/style/multiple_comparison_spec.rb +++ b/spec/rubocop/cop/style/multiple_comparison_spec.rb @@ -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"