Lint/HashCompareByIdentity

Hash#compare_by_identity is intentionally not used since we only need to keep
track of object ids we've seen before, based on object_id. If we were to use
compare_by_identity, then we'd need to store entire objects in memory.

Author: joshcooper

.rubocop_todo.yml

@@ -590,11 +590,6 @@ Lint/EnsureReturn:
   Exclude:
     - 'lib/puppet/configurer.rb'
 
-Lint/HashCompareByIdentity:
-  Exclude:
-    - 'lib/puppet/pops/serialization/serializer.rb'
-    - 'lib/puppet/pops/types/recursion_guard.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: standard_error, runtime_error

lib/puppet/pops/serialization/serializer.rb

@@ -16,6 +16,8 @@ class Serializer
     # @option options [Boolean] :type_by_reference `true` if Object types are serialized by name only.
     # @api public
     def initialize(writer, options = EMPTY_HASH)
+      # Hash#compare_by_identity is intentionally not used since we only need to map
+      # object ids to their size and we don't want to store entire objects in memory
       @written = {}
       @writer = writer
       @options = options
@@ -37,7 +39,7 @@ def write(value)
       when :default
         @writer.write(Extension::Default::INSTANCE)
       else
-        index = @written[value.object_id]
+        index = @written[value.object_id] # rubocop:disable Lint/HashCompareByIdentity
         if index.nil?
           write_tabulated_first_time(value)
         else
@@ -76,7 +78,7 @@ def start_object(attr_count)
     end
 
     def push_written(value)
-      @written[value.object_id] = @written.size
+      @written[value.object_id] = @written.size # rubocop:disable Lint/HashCompareByIdentity
     end
 
     # Write the start of a sensitive object

lib/puppet/pops/types/recursion_guard.rb

@@ -8,6 +8,10 @@ module Types
 #
 # All comparisons are made using the `object_id` of the instance rather than the instance itself.
 #
+# Hash#compare_by_identity is intentionally not used since we only need to keep
+# track of object ids we've seen before, based on object_id, so we don't store
+# entire objects in memory.
+#
 # @api private
 class RecursionGuard
   attr_reader :state
@@ -25,14 +29,14 @@ def initialize
   # @param instance [Object] the instance to check
   # @return [Integer] the resulting state
   def recursive_this?(instance)
-    instance_variable_defined?(:@recursive_this_map) && @recursive_this_map.has_key?(instance.object_id)
+    instance_variable_defined?(:@recursive_this_map) && @recursive_this_map.has_key?(instance.object_id) # rubocop:disable Lint/HashCompareByIdentity
   end
 
   # Checks if recursion was detected for the given argument in the 'that' context
   # @param instance [Object] the instance to check
   # @return [Integer] the resulting state
   def recursive_that?(instance)
-    instance_variable_defined?(:@recursive_that_map) && @recursive_that_map.has_key?(instance.object_id)
+    instance_variable_defined?(:@recursive_that_map) && @recursive_that_map.has_key?(instance.object_id) # rubocop:disable Lint/HashCompareByIdentity
   end
 
   # Add the given argument as 'this' invoke the given block with the resulting state