Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Lint/IdentityComparison cop #8699

Merged
merged 1 commit into from Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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