Skip to content

Commit

Permalink
[Fix rubocop#5619] - Do not register an offense when in InverseMethod…
Browse files Browse the repository at this point in the history
…s when determining class hierarchy
  • Loading branch information
rrosenblum authored and bbatsov committed Mar 13, 2018
1 parent 4c34f33 commit 9da2633
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 22 additions & 5 deletions lib/rubocop/cop/style/inverse_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 `%<inverse>s` instead of inverting `%<method>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

Expand All @@ -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,
Expand All @@ -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|
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions manual/cops_style.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions spec/rubocop/cop/style/inverse_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 9da2633

Please sign in to comment.