Skip to content

Commit

Permalink
Don't consider <=> a comparison method 🚀 [Fix #4603]
Browse files Browse the repository at this point in the history
I believe, the `<=>` operator should not be considered as a
`.comparison_method?`.

The main reason is, that this method in difference to all other
comparison methods doesn't return a boolean, but `-1`, `0` or `1`.
Thus, many assumptions for comparison methods don't apply, for example,
there isn't an inverse comparison method (like `<` is the inverse of
`>=)`, `* -1` isn't a comparison).

This is an alternative to #4603 by me.
  • Loading branch information
iGEL committed Aug 24, 2017
1 parent c243880 commit 5390f4e
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

### Bug fixes

* [#4615](https://github.com/bbatsov/rubocop/pull/4615): Don't consider `<=>` a comparison method. ([@iGEL][])
* [#4664](https://github.com/bbatsov/rubocop/pull/4664): Fix typos in Rails/HttpPositionalArguments. ([@JoeCohen][])
* [#4618](https://github.com/bbatsov/rubocop/pull/4618): Fix `Lint/FormatParameterMismatch` false positive if format string includes `%%5B` (CGI encoded left bracket). ([@barthez][])
* [#4604](https://github.com/bbatsov/rubocop/pull/4604): Fix `Style/LambdaCall` to autocorrect `obj.call` to `obj.`. ([@iGEL][])
Expand Down
5 changes: 3 additions & 2 deletions lib/rubocop/ast/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class Node < Parser::AST::Node # rubocop:disable Metrics/ClassLength
include RuboCop::AST::Sexp
extend NodePattern::Macros

COMPARISON_OPERATORS = %i[== === != <= >= > < <=>].freeze
# <=> isn't included here, because it doesn't return a boolean.
COMPARISON_OPERATORS = %i[== === != <= >= > <].freeze

TRUTHY_LITERALS = %i[str dstr xstr int float sym dsym array
hash regexp true irange erange complex
Expand Down Expand Up @@ -366,7 +367,7 @@ def immutable_literal?
case type
when :send
receiver, method_name, *args = *self
[*COMPARISON_OPERATORS, :!].include?(method_name) &&
[*COMPARISON_OPERATORS, :!, :<=>].include?(method_name) &&
receiver.send(recursive_kind) &&
args.all?(&recursive_kind)
when :begin, :pair, *OPERATOR_KEYWORDS, *COMPOSITE_LITERALS
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/ast/def_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@

describe '#comparison_method?' do
context 'with a comparison method' do
let(:source) { 'def <=>(bar); end' }
let(:source) { 'def <=(bar); end' }

it { expect(def_node.comparison_method?).to be_truthy }
end
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/ast/send_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@
context 'with a comparison method' do
let(:source) { 'foo.bar <=> :baz' }

it { expect(send_node.comparison_method?).to be_truthy }
it { expect(send_node.comparison_method?).to be_falsey }
end

context 'with a regular method' do
Expand Down
1 change: 1 addition & 0 deletions spec/rubocop/cop/style/yoda_condition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
it_behaves_like 'accepts', '[1, 2, 3] <=> [4, 5, 6]'
it_behaves_like 'accepts', '!true'
it_behaves_like 'accepts', 'not true'
it_behaves_like 'accepts', '0 <=> val'

it_behaves_like 'offense', '"foo" == bar'
it_behaves_like 'offense', 'nil == bar'
Expand Down

0 comments on commit 5390f4e

Please sign in to comment.