From eca7d75e65e221cce939d68d023f1f7259b1d5ce Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 7 Aug 2020 20:07:19 +0900 Subject: [PATCH] Add new `Lint/IdentityComparison` cop Follow https://github.com/rubocop-hq/ruby-style-guide/pull/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. --- CHANGELOG.md | 1 + config/default.yml | 6 +++ docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_lint.adoc | 32 ++++++++++++ lib/rubocop.rb | 1 + lib/rubocop/cop/lint/identity_comparison.rb | 50 +++++++++++++++++++ lib/rubocop/cop/style/empty_literal.rb | 2 +- .../cop/lint/identity_comparison_spec.rb | 34 +++++++++++++ 8 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/cop/lint/identity_comparison.rb create mode 100644 spec/rubocop/cop/lint/identity_comparison_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0418fdc84..fbf7a49a484 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/default.yml b/config/default.yml index 949168569b2..90f86fbfcc7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 03add164fdb..16ba9faf3d4 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -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] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 68124625536..5945219f994 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -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 |=== diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 52147beac19..e81061cdc77 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' diff --git a/lib/rubocop/cop/lint/identity_comparison.rb b/lib/rubocop/cop/lint/identity_comparison.rb new file mode 100644 index 00000000000..89257355858 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/style/empty_literal.rb b/lib/rubocop/cop/style/empty_literal.rb index 4eb485188ba..32107fc2889 100644 --- a/lib/rubocop/cop/style/empty_literal.rb +++ b/lib/rubocop/cop/style/empty_literal.rb @@ -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 diff --git a/spec/rubocop/cop/lint/identity_comparison_spec.rb b/spec/rubocop/cop/lint/identity_comparison_spec.rb new file mode 100644 index 00000000000..8f3e7b7056b --- /dev/null +++ b/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