Skip to content

Commit

Permalink
Add new Lint/IdentityComparison cop
Browse files Browse the repository at this point in the history
Follow rubocop/ruby-style-guide#835.

This PR adds new `Lint/IdentityComparison` cop.

```ruby
# bad
foo.object_id == bar.object_id

# good
foo.equal?(bar)
```

The cop prefers `equal?` over `==` when comparing `object_id`.
`Object#equal?` is provided to compare objects for identity, and in contrast
`Object#==` is provided for the purpose of doing value comparison.
  • Loading branch information
koic authored and bbatsov committed Sep 11, 2020
1 parent 9bc7908 commit eca7d75
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@

* New option `--cache-root` and support for the `RUBOCOP_CACHE_ROOT` environment variable. Both can be used to override the `AllCops: CacheRootDirectory` config, especially in a CI setting. ([@sascha-wolf][])
* [#8582](https://github.com/rubocop-hq/rubocop/issues/8582): Add new `Layout/BeginEndAlignment` cop. ([@koic][])
* [#8699](https://github.com/rubocop-hq/rubocop/pull/8699): Add new `Lint/IdentityComparison` cop. ([@koic][])

### Bug fixes

Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Expand Up @@ -1548,6 +1548,12 @@ Lint/HeredocMethodCallPosition:
StyleGuide: '#heredoc-method-calls'
VersionAdded: '0.68'

Lint/IdentityComparison:
Description: 'Prefer `equal?` over `==` when comparing `object_id`.'
Enabled: pending
StyleGuide: '#identity-comparison'
VersionAdded: '0.91'

Lint/ImplicitStringConcatenation:
Description: >-
Checks for adjacent string literals on the same line, which
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -216,6 +216,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintfloatoutofrange[Lint/FloatOutOfRange]
* xref:cops_lint.adoc#lintformatparametermismatch[Lint/FormatParameterMismatch]
* xref:cops_lint.adoc#lintheredocmethodcallposition[Lint/HeredocMethodCallPosition]
* xref:cops_lint.adoc#lintidentitycomparison[Lint/IdentityComparison]
* xref:cops_lint.adoc#lintimplicitstringconcatenation[Lint/ImplicitStringConcatenation]
* xref:cops_lint.adoc#lintineffectiveaccessmodifier[Lint/IneffectiveAccessModifier]
* xref:cops_lint.adoc#lintinheritexception[Lint/InheritException]
Expand Down
32 changes: 32 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -1600,6 +1600,38 @@ the receiver of the call is a HEREDOC.

* https://rubystyle.guide#heredoc-method-calls

== Lint/IdentityComparison

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| 0.91
| -
|===

Prefer `equal?` over `==` when comparing `object_id`.

`Object#equal?` is provided to compare objects for identity, and in contrast
`Object#==` is provided for the purpose of doing value comparison.

=== Examples

[source,ruby]
----
# bad
foo.object_id == bar.object_id
# good
foo.equal?(bar)
----

=== References

* https://rubystyle.guide#identity-comparison

== Lint/ImplicitStringConcatenation

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -278,6 +278,7 @@
require_relative 'rubocop/cop/lint/float_out_of_range'
require_relative 'rubocop/cop/lint/format_parameter_mismatch'
require_relative 'rubocop/cop/lint/heredoc_method_call_position'
require_relative 'rubocop/cop/lint/identity_comparison'
require_relative 'rubocop/cop/lint/implicit_string_concatenation'
require_relative 'rubocop/cop/lint/inherit_exception'
require_relative 'rubocop/cop/lint/ineffective_access_modifier'
Expand Down
50 changes: 50 additions & 0 deletions lib/rubocop/cop/lint/identity_comparison.rb
@@ -0,0 +1,50 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
#
# Prefer `equal?` over `==` when comparing `object_id`.
#
# `Object#equal?` is provided to compare objects for identity, and in contrast
# `Object#==` is provided for the purpose of doing value comparison.
#
# @example
# # bad
# foo.object_id == bar.object_id
#
# # good
# foo.equal?(bar)
#
class IdentityComparison < Base
extend AutoCorrector

MSG = 'Use `equal?` instead `==` when comparing `object_id`.'

def on_send(node)
return unless compare_between_object_id_by_double_equal?(node)

add_offense(node) do |corrector|
receiver = node.receiver.receiver.source
argument = node.first_argument.receiver.source
replacement = "#{receiver}.equal?(#{argument})"

corrector.replace(node, replacement)
end
end

private

def compare_between_object_id_by_double_equal?(node)
return false unless node.method?(:==)

object_id_method?(node.receiver) && object_id_method?(node.first_argument)
end

def object_id_method?(node)
node.send_type? && node.method?(:object_id)
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/empty_literal.rb
Expand Up @@ -71,7 +71,7 @@ def first_argument_unparenthesized?(node)
parent = node.parent
return false unless parent && %i[send super zsuper].include?(parent.type)

node.object_id == parent.arguments.first.object_id &&
node.equal?(parent.arguments.first) &&
!parentheses?(node.parent)
end

Expand Down
34 changes: 34 additions & 0 deletions spec/rubocop/cop/lint/identity_comparison_spec.rb
@@ -0,0 +1,34 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Lint::IdentityComparison, :config do
subject(:cop) { described_class.new(config) }

it 'registers an offense and corrects when using `==` for comparison between `object_id`s' do
expect_offense(<<~RUBY)
foo.object_id == bar.object_id
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `equal?` instead `==` when comparing `object_id`.
RUBY

expect_correction(<<~RUBY)
foo.equal?(bar)
RUBY
end

it 'does not register an offense when using `==` for comparison between `object_id` and other' do
expect_no_offenses(<<~RUBY)
foo.object_id == bar.do_something
RUBY
end

it 'does not register an offense when a receiver that is not `object_id` uses `==`' do
expect_no_offenses(<<~RUBY)
foo.do_something == bar.object_id
RUBY
end

it 'does not register an offense when using `==`' do
expect_no_offenses(<<~RUBY)
foo.equal(bar)
RUBY
end
end

0 comments on commit eca7d75

Please sign in to comment.