From dbc2714c24921e78011b1495a74d40e32e63d8c0 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 26 Oct 2020 01:25:03 +0900 Subject: [PATCH] Accept method calls for `Style/MultipleComparison` This PR accepts comparisons of multiple method calls for `Style/MultipleComparison` to avoid unnecessary method calls. ```ruby # also good foo if a == b.lightweight || a == b.heavyweight ``` This change can reduce RuboCop's own disable comment. --- CHANGELOG.md | 1 + config/default.yml | 2 +- docs/modules/ROOT/pages/cops_lint.adoc | 2 +- docs/modules/ROOT/pages/cops_style.adoc | 2 ++ lib/rubocop/cop/layout/extra_spacing.rb | 3 +-- lib/rubocop/cop/style/multiple_comparison.rb | 6 ++++-- spec/rubocop/cop/style/multiple_comparison_spec.rb | 9 +++++++++ 7 files changed, 19 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a2f67f9cfa..86b8fd77343 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ * [#8920](https://github.com/rubocop-hq/rubocop/pull/8920): Remove Capybara's `save_screenshot` from `Lint/Debugger`. ([@ybiquitous][]) * [#8919](https://github.com/rubocop-hq/rubocop/issues/8919): Require RuboCop AST to 1.0.1 or higher. ([@koic][]) +* [#8939](https://github.com/rubocop-hq/rubocop/pull/8939): Accept comparisons of multiple method calls for `Style/MultipleComparison`. ([@koic][]) ## 1.0.0 (2020-10-21) diff --git a/config/default.yml b/config/default.yml index f77265b10ae..83e0d180829 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1653,7 +1653,7 @@ Lint/MultipleComparison: Description: "Use `&&` operator to compare multiple values." Enabled: true VersionAdded: '0.47' - VersionChanged: '0.77' + VersionChanged: '1.1' 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 e2857c5f618..21a8f2daf68 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2275,7 +2275,7 @@ named captures. | Yes | Yes | 0.47 -| 0.77 +| 1.1 |=== In math and Python, we can use `x < y < z` style comparison to compare diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index f28dc3807c9..792724f17b1 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -6017,6 +6017,7 @@ 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. === Examples @@ -6029,6 +6030,7 @@ foo if a == 'a' || a == 'b' || a == 'c' # good a = 'a' foo if ['a', 'b', 'c'].include?(a) +foo if a == b.lightweight || a == b.heavyweight ---- == Style/MutableConstant diff --git a/lib/rubocop/cop/layout/extra_spacing.rb b/lib/rubocop/cop/layout/extra_spacing.rb index b6d0a0fc16a..8ac6a702b99 100644 --- a/lib/rubocop/cop/layout/extra_spacing.rb +++ b/lib/rubocop/cop/layout/extra_spacing.rb @@ -56,8 +56,7 @@ def aligned_locations(locs) aligned = Set[locs.first.line, locs.last.line] locs.each_cons(3) do |before, loc, after| col = loc.column - aligned << loc.line if col == before.column || # rubocop:disable Style/MultipleComparison - col == after.column + aligned << loc.line if col == before.column || col == after.column end aligned end diff --git a/lib/rubocop/cop/style/multiple_comparison.rb b/lib/rubocop/cop/style/multiple_comparison.rb index f1af84551d1..fd5e839960a 100644 --- a/lib/rubocop/cop/style/multiple_comparison.rb +++ b/lib/rubocop/cop/style/multiple_comparison.rb @@ -5,6 +5,7 @@ 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. # # @example # # bad @@ -14,6 +15,7 @@ module Style # # good # a = 'a' # foo if ['a', 'b', 'c'].include?(a) + # foo if a == b.lightweight || a == b.heavyweight class MultipleComparison < Base MSG = 'Avoid comparing a variable with multiple items ' \ 'in a conditional, use `Array#include?` instead.' @@ -31,8 +33,8 @@ def on_or(node) def_node_matcher :simple_double_comparison?, '(send $lvar :== $lvar)' def_node_matcher :simple_comparison?, <<~PATTERN - {(send $lvar :== _) - (send _ :== $lvar)} + {(send $lvar :== !send) + (send !send :== $lvar)} PATTERN def nested_variable_comparison?(node) diff --git a/spec/rubocop/cop/style/multiple_comparison_spec.rb b/spec/rubocop/cop/style/multiple_comparison_spec.rb index 612a2fd3b10..77158b55355 100644 --- a/spec/rubocop/cop/style/multiple_comparison_spec.rb +++ b/spec/rubocop/cop/style/multiple_comparison_spec.rb @@ -66,6 +66,15 @@ 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"