diff --git a/CHANGELOG.md b/CHANGELOG.md index eac5ebfe063..5bfe1eea1b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * [#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][]) +* [#8953](https://github.com/rubocop-hq/rubocop/pull/8953): Add `AllowMethodComparison` option for `Lint/MultipleComparison`. ([@koic][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 04e88c1a1a3..443d9a82fbd 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1659,6 +1659,7 @@ Lint/MultipleComparison: Enabled: true VersionAdded: '0.47' VersionChanged: '1.1' + AllowMethodComparison: true Lint/NestedMethodDefinition: Description: 'Do not use nested method definitions.' diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index d252df494e1..3bda42c5cae 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2338,6 +2338,16 @@ x < y && y < z 10 <= x && x <= 20 ---- +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| AllowMethodComparison +| `true` +| Boolean +|=== + == Lint/NestedMethodDefinition |=== diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index a5de9fd3e37..5485bcf56cc 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -6075,7 +6075,8 @@ end This cop checks against comparing a variable with multiple items, where `Array#include?` could be used instead to avoid code repetition. -It accepts comparisons of multiple method calls to avoid unnecessary method calls. +It accepts comparisons of multiple method calls to avoid unnecessary method calls +by default. It can be configured by `AllowMethodComparison` option. === Examples @@ -6091,6 +6092,25 @@ foo if ['a', 'b', 'c'].include?(a) foo if a == b.lightweight || a == b.heavyweight ---- +==== AllowMethodComparison: true (default) + +[source,ruby] +---- +# good +foo if a == b.lightweight || a == b.heavyweight +---- + +==== AllowMethodComparison: false + +[source,ruby] +---- +# bad +foo if a == b.lightweight || a == b.heavyweight + +# good +foo if [b.lightweight, b.heavyweight].include?(a) +---- + == Style/MutableConstant |=== diff --git a/lib/rubocop/cop/style/multiple_comparison.rb b/lib/rubocop/cop/style/multiple_comparison.rb index 825c4085f33..c1e4816db34 100644 --- a/lib/rubocop/cop/style/multiple_comparison.rb +++ b/lib/rubocop/cop/style/multiple_comparison.rb @@ -5,7 +5,8 @@ module Cop module Style # This cop checks against comparing a variable with multiple items, where # `Array#include?` could be used instead to avoid code repetition. - # It accepts comparisons of multiple method calls to avoid unnecessary method calls. + # It accepts comparisons of multiple method calls to avoid unnecessary method calls + # by default. It can be configured by `AllowMethodComparison` option. # # @example # # bad @@ -16,6 +17,17 @@ module Style # a = 'a' # foo if ['a', 'b', 'c'].include?(a) # foo if a == b.lightweight || a == b.heavyweight + # + # @example AllowMethodComparison: true (default) + # # good + # foo if a == b.lightweight || a == b.heavyweight + # + # @example AllowMethodComparison: false + # # bad + # foo if a == b.lightweight || a == b.heavyweight + # + # # good + # foo if [b.lightweight, b.heavyweight].include?(a) class MultipleComparison < Base extend AutoCorrector @@ -44,10 +56,10 @@ def on_or(node) def_node_matcher :simple_double_comparison?, '(send $lvar :== $lvar)' def_node_matcher :simple_comparison_lhs?, <<~PATTERN - (send $lvar :== $!send) + (send $lvar :== $_) PATTERN def_node_matcher :simple_comparison_rhs?, <<~PATTERN - (send $!send :== $lvar) + (send $_ :== $lvar) PATTERN def nested_variable_comparison?(node) @@ -71,6 +83,8 @@ def variables_in_simple_node(node) return [variable_name(var1), variable_name(var2)] end if (var, obj = simple_comparison_lhs?(node)) || (obj, var = simple_comparison_rhs?(node)) + return [] if allow_method_comparison? && obj.send_type? + @compared_elements << obj.source return [variable_name(var)] end @@ -103,6 +117,10 @@ def root_of_or_node(or_node) or_node end end + + def allow_method_comparison? + cop_config.fetch('AllowMethodComparison', true) + end end end end diff --git a/spec/rubocop/cop/style/multiple_comparison_spec.rb b/spec/rubocop/cop/style/multiple_comparison_spec.rb index c188bced9e1..2a091668e84 100644 --- a/spec/rubocop/cop/style/multiple_comparison_spec.rb +++ b/spec/rubocop/cop/style/multiple_comparison_spec.rb @@ -1,10 +1,8 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Style::MultipleComparison do +RSpec.describe RuboCop::Cop::Style::MultipleComparison, :config do subject(:cop) { described_class.new(config) } - let(:config) { RuboCop::Config.new } - it 'does not register an offense for comparing an lvar' do expect_no_offenses(<<~RUBY) a = "a" @@ -100,15 +98,6 @@ def foo(x) RUBY end - it 'does not register an offense and corrects when using multiple method calls' do - expect_no_offenses(<<~RUBY) - col = loc.column - if col == before.column || col == after.column - do_something - end - RUBY - end - it 'does not register an offense for comparing multiple literal strings' do expect_no_offenses(<<~RUBY) if "a" == "a" || "a" == "c" @@ -174,4 +163,38 @@ def foo(x) end RUBY end + + context 'when `AllowMethodComparison: true`' do + let(:cop_config) { { 'AllowMethodComparison' => true } } + + it 'does not register an offense when using multiple method calls' do + expect_no_offenses(<<~RUBY) + col = loc.column + if col == before.column || col == after.column + do_something + end + RUBY + end + end + + context 'when `AllowMethodComparison: false`' do + let(:cop_config) { { 'AllowMethodComparison' => false } } + + it 'registers an offense and corrects when using multiple method calls' do + expect_offense(<<~RUBY) + col = loc.column + if col == before.column || col == after.column + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid comparing a variable with multiple items in a conditional, use `Array#include?` instead. + do_something + end + RUBY + + expect_correction(<<~RUBY) + col = loc.column + if [before.column, after.column].include?(col) + do_something + end + RUBY + end + end end