diff --git a/CHANGELOG.md b/CHANGELOG.md index c3d8c2a7a826..cba0ea43ec25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * [#5630](https://github.com/bbatsov/rubocop/issues/5630): Fix false positive for `Style/FormatStringToken` when using placeholder arguments in `format` method. ([@koic][]) * [#5651](https://github.com/bbatsov/rubocop/pull/5651): Fix NoMethodError when specified config file that does not exist. ([@onk][]) * [#5647](https://github.com/bbatsov/rubocop/pull/5647): Fix encoding method of RuboCop::MagicComment::SimpleComment. ([@htwroclau][]) +* [#5619](https://github.com/bbatsov/rubocop/issues/5619): Do not register an offense in `Style/InverseMethods` when comparing constants with `<`, `>`, `<=`, or `>=`. If the code is being used to determine class hierarchy, the correction might not be accurate. ([@rrosenblum][]) ### Changes diff --git a/lib/rubocop/cop/style/inverse_methods.rb b/lib/rubocop/cop/style/inverse_methods.rb index 1a57b1c11983..7709cd126a01 100644 --- a/lib/rubocop/cop/style/inverse_methods.rb +++ b/lib/rubocop/cop/style/inverse_methods.rb @@ -27,18 +27,21 @@ module Style # foo != bar # foo == bar # !!('foo' =~ /^\w+$/) + # !(foo.class < Numeric) # Checking class hierarchy is allowed class InverseMethods < Cop include IgnoredNode MSG = 'Use `%s` instead of inverting `%s`.'.freeze + CLASS_COMPARISON_METHODS = %i[<= >= < >].freeze EQUALITY_METHODS = %i[== != =~ !~ <= >= < >].freeze NEGATED_EQUALITY_METHODS = %i[!= !~].freeze + CAMEL_CASE = /[A-Z]+[a-z]+/ def_node_matcher :inverse_candidate?, <<-PATTERN { - (send $(send (...) $_ ...) :!) - (send (block $(send (...) $_) ...) :!) - (send (begin $(send (...) $_ ...)) :!) + (send $(send $(...) $_ $...) :!) + (send (block $(send $(...) $_) $...) :!) + (send (begin $(send $(...) $_ $...)) :!) } PATTERN @@ -52,8 +55,9 @@ class InverseMethods < Cop def on_send(node) return if part_of_ignored_node?(node) - inverse_candidate?(node) do |_method_call, method| + inverse_candidate?(node) do |_method_call, lhs, method, rhs| return unless inverse_methods.key?(method) + return if possible_class_hierarchy_check?(lhs, rhs, method) return if negated?(node) add_offense(node, @@ -78,7 +82,7 @@ def on_block(node) end def autocorrect(node) - method_call, method = inverse_candidate?(node) + method_call, _lhs, method, _rhs = inverse_candidate?(node) if method_call && method lambda do |corrector| @@ -143,6 +147,19 @@ def end_parentheses(node, method_call) method_call.loc.expression.end_pos, node.loc.expression.end_pos) end + + # When comparing classes, `!(Integer < Numeric)` is not the same as + # `Integer > Numeric`. + def possible_class_hierarchy_check?(lhs, rhs, method) + CLASS_COMPARISON_METHODS.include?(method) && + (camel_case_constant?(lhs) || + (rhs.size == 1 && + camel_case_constant?(rhs.first))) + end + + def camel_case_constant?(node) + node.const_type? && node.source =~ CAMEL_CASE + end end end end diff --git a/manual/cops_style.md b/manual/cops_style.md index da62289c1044..0e6b66f5c62d 100644 --- a/manual/cops_style.md +++ b/manual/cops_style.md @@ -2508,6 +2508,7 @@ foo.any? { |f| f.even? } foo != bar foo == bar !!('foo' =~ /^\w+$/) +!(foo.class < Numeric) # Checking class hierarchy is allowed ``` ### Configurable attributes diff --git a/spec/rubocop/cop/style/inverse_methods_spec.rb b/spec/rubocop/cop/style/inverse_methods_spec.rb index 3d403020b4d5..f30e173376a8 100644 --- a/spec/rubocop/cop/style/inverse_methods_spec.rb +++ b/spec/rubocop/cop/style/inverse_methods_spec.rb @@ -150,6 +150,36 @@ end end + it 'allows comparing camel case constants on the right' do + expect_no_offenses(<<-RUBY.strip_indent) + klass = self.class + !(klass < BaseClass) + RUBY + end + + it 'allows comparing camel case constants on the left' do + expect_no_offenses(<<-RUBY.strip_indent) + klass = self.class + !(BaseClass < klass) + RUBY + end + + it 'registers an offense for comparing snake case constants on the right' do + expect_offense(<<-RUBY.strip_indent) + klass = self.class + !(klass < FOO_BAR) + ^^^^^^^^^^^^^^^^^^ Use `>=` instead of inverting `<`. + RUBY + end + + it 'registers an offense for comparing snake case constants on the left' do + expect_offense(<<-RUBY.strip_indent) + klass = self.class + !(FOO_BAR < klass) + ^^^^^^^^^^^^^^^^^^ Use `>=` instead of inverting `<`. + RUBY + end + context 'inverse blocks' do { select: :reject, reject: :select,