Skip to content

Commit

Permalink
Add new Lint/HashCompareByIdentity cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Sep 29, 2020
1 parent 14ad258 commit cb06e4a
Show file tree
Hide file tree
Showing 10 changed files with 127 additions and 8 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### New features

* [#8796](https://github.com/rubocop-hq/rubocop/pull/8796): Add new `Lint/HashCompareByIdentity` cop. ([@fatkodima][])

## 0.92.0 (2020-09-25)

### New features
Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -1546,6 +1546,13 @@ Lint/FormatParameterMismatch:
Enabled: true
VersionAdded: '0.33'

Lint/HashCompareByIdentity:
Description: 'Prefer using `Hash#compare_by_identity` than using `object_id` for keys.'
StyleGuide: '#identity-comparison'
Enabled: pending
Safe: false
VersionAdded: '0.93'

Lint/HeredocMethodCallPosition:
Description: >-
Checks for the ordering of a method call where
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#lintfloatcomparison[Lint/FloatComparison]
* xref:cops_lint.adoc#lintfloatoutofrange[Lint/FloatOutOfRange]
* xref:cops_lint.adoc#lintformatparametermismatch[Lint/FormatParameterMismatch]
* xref:cops_lint.adoc#linthashcomparebyidentity[Lint/HashCompareByIdentity]
* xref:cops_lint.adoc#lintheredocmethodcallposition[Lint/HeredocMethodCallPosition]
* xref:cops_lint.adoc#lintidentitycomparison[Lint/IdentityComparison]
* xref:cops_lint.adoc#lintimplicitstringconcatenation[Lint/ImplicitStringConcatenation]
Expand Down
36 changes: 36 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -1620,6 +1620,42 @@ format('Unnumbered format: %s and numbered: %2$s', a_value, another)
format('Numbered format: %1$s and numbered %2$s', a_value, another)
----

== Lint/HashCompareByIdentity

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

| Pending
| No
| No
| 0.93
| -
|===

Prefer using `Hash#compare_by_identity` than using `object_id` for hash keys.

This cop is marked as unsafe as a hash possibly can contain other keys
besides `object_id`s.

=== Examples

[source,ruby]
----
# bad
hash = {}
hash[foo.object_id] = :bar
hash.key?(baz.object_id)
# good
hash = {}.compare_by_identity
hash[foo] = :bar
hash.key?(baz)
----

=== References

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

== Lint/HeredocMethodCallPosition

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -279,6 +279,7 @@
require_relative 'rubocop/cop/lint/float_comparison'
require_relative 'rubocop/cop/lint/float_out_of_range'
require_relative 'rubocop/cop/lint/format_parameter_mismatch'
require_relative 'rubocop/cop/lint/hash_compare_by_identity'
require_relative 'rubocop/cop/lint/heredoc_method_call_position'
require_relative 'rubocop/cop/lint/identity_comparison'
require_relative 'rubocop/cop/lint/implicit_string_concatenation'
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/correctors/line_break_corrector.rb
Expand Up @@ -52,8 +52,8 @@ def remove_semicolon(node, corrector)
end

def semicolon(node)
@semicolon ||= {}
@semicolon[node.object_id] ||= processed_source.tokens_within(node).find(&:semicolon?)
@semicolon ||= {}.compare_by_identity
@semicolon[node] ||= processed_source.tokens_within(node).find(&:semicolon?)
end
end
end
Expand Down
37 changes: 37 additions & 0 deletions lib/rubocop/cop/lint/hash_compare_by_identity.rb
@@ -0,0 +1,37 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Lint
# Prefer using `Hash#compare_by_identity` than using `object_id` for hash keys.
#
# This cop is marked as unsafe as a hash possibly can contain other keys
# besides `object_id`s.
#
# @example
# # bad
# hash = {}
# hash[foo.object_id] = :bar
# hash.key?(baz.object_id)
#
# # good
# hash = {}.compare_by_identity
# hash[foo] = :bar
# hash.key?(baz)
#
class HashCompareByIdentity < Base
RESTRICT_ON_SEND = %i[key? has_key? fetch [] []=].freeze

MSG = 'Use `Hash#compare_by_identity` instead of using `object_id` for keys.'

def_node_matcher :id_as_hash_key?, <<~PATTERN
(send _ {:key? :has_key? :fetch :[] :[]=} (send _ :object_id) ...)
PATTERN

def on_send(node)
add_offense(node) if id_as_hash_key?(node)
end
end
end
end
end
4 changes: 2 additions & 2 deletions lib/rubocop/result_cache.rb
Expand Up @@ -209,8 +209,8 @@ def relevant_options_digest(options)
# The external dependency checksums are cached per RuboCop team so that
# the checksums don't need to be recomputed for each file.
def team_checksum(team)
@checksum_by_team ||= {}
@checksum_by_team[team.object_id] ||= team.external_dependency_checksum
@checksum_by_team ||= {}.compare_by_identity
@checksum_by_team[team] ||= team.external_dependency_checksum
end

# We combine team and options into a single "context" checksum to avoid
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/runner.rb
Expand Up @@ -324,8 +324,8 @@ def mobilize_team(processed_source)
end

def mobilized_cop_classes(config)
@mobilized_cop_classes ||= {}
@mobilized_cop_classes[config.object_id] ||= begin
@mobilized_cop_classes ||= {}.compare_by_identity
@mobilized_cop_classes[config] ||= begin
cop_classes = Cop::Registry.all

OptionsValidator.new(@options).validate_cop_options
Expand Down Expand Up @@ -399,8 +399,8 @@ def get_processed_source(file)
# otherwise dormant team that can be used for config- and option-
# level caching in ResultCache.
def standby_team(config)
@team_by_config ||= {}
@team_by_config[config.object_id] ||=
@team_by_config ||= {}.compare_by_identity
@team_by_config[config] ||=
Cop::Team.mobilize(mobilized_cop_classes(config), config, @options)
end
end
Expand Down
33 changes: 33 additions & 0 deletions spec/rubocop/cop/lint/hash_compare_by_identity_spec.rb
@@ -0,0 +1,33 @@
# frozen_string_literal: true

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

it 'registers an offense when using hash methods with `object_id` on receiver as a key' do
expect_offense(<<~RUBY)
hash.key?(foo.object_id)
^^^^^^^^^^^^^^^^^^^^^^^^ Use `Hash#compare_by_identity` instead of using `object_id` for keys.
hash.has_key?(foo.object_id)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Hash#compare_by_identity` instead of using `object_id` for keys.
hash[foo.object_id] = bar
^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Hash#compare_by_identity` instead of using `object_id` for keys.
hash[foo.object_id]
^^^^^^^^^^^^^^^^^^^ Use `Hash#compare_by_identity` instead of using `object_id` for keys.
hash.fetch(foo.object_id, 42)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Hash#compare_by_identity` instead of using `object_id` for keys.
RUBY
end

it 'registers an offense when using hash method with `object_id` as a key' do
expect_offense(<<~RUBY)
hash.key?(object_id)
^^^^^^^^^^^^^^^^^^^^ Use `Hash#compare_by_identity` instead of using `object_id` for keys.
RUBY
end

it 'does not register an offense for hash methods without `object_id` as key' do
expect_no_offenses(<<~RUBY)
hash.key?(foo)
RUBY
end
end

0 comments on commit cb06e4a

Please sign in to comment.