Refactor HashWithIndifferentAccess not to rely on tap as much

Author: byroot

activesupport/lib/active_support/hash_with_indifferent_access.rb

@@ -278,9 +278,7 @@ def fetch_values(*indices, &block)
     #   hash[:a][:c] # => "c"
     #   dup[:a][:c]  # => "c"
     def dup
-      self.class.new(self).tap do |new_hash|
-        set_defaults(new_hash)
-      end
+      copy_defaults(self.class.new(self))
     end
 
     # This method has the same semantics of +update+, except it does not
@@ -368,12 +366,12 @@ def transform_keys(hash = NOT_GIVEN, &block)
     def transform_keys!(hash = NOT_GIVEN, &block)
       if NOT_GIVEN.equal?(hash)
         if block_given?
-          replace(transform_keys(&block).tap { |h| set_defaults(h) })
+          replace(copy_defaults(transform_keys(&block)))
         else
           return to_enum(:transform_keys!)
         end
       else
-        replace(transform_keys(hash, &block).tap { |h| set_defaults(h) })
+        replace(copy_defaults(transform_keys(hash, &block)))
       end
 
       self
@@ -397,8 +395,7 @@ def compact
     def to_hash
       copy = Hash[self]
       copy.transform_values! { |v| convert_value_to_hash(v) }
-      set_defaults(copy)
-      copy
+      copy_defaults(copy)
     end
 
     def to_proc
@@ -438,12 +435,13 @@ def convert_value_to_hash(value)
       end
 
 
-      def set_defaults(target)
+      def copy_defaults(target)
         if default_proc
           target.default_proc = default_proc.dup
         else
           target.default = default
         end
+        target
       end
 
       def update_with_single_argument(other_hash, block)

activesupport/test/hash_with_indifferent_access_test.rb

@@ -555,15 +555,20 @@ def test_indifferent_transform_keys_bang
 
     hash_with_default = Hash.new(:a)
     hash = ActiveSupport::HashWithIndifferentAccess.new(hash_with_default).transform_keys!(&:to_s)
-    assert_equal hash_with_default.default, hash.default
+    assert_equal :a, hash.default
+    assert_equal :a, hash_with_default.default
+
     hash = ActiveSupport::HashWithIndifferentAccess.new(hash_with_default).transform_keys! { |k| k.to_s }
-    assert_equal hash_with_default.default, hash.default
+    assert_equal :a, hash.default
+    assert_equal :a, hash_with_default.default
 
     hash_with_default_proc = Hash.new { |h, k| h[k] = :b }
+    default_proc = hash_with_default_proc.default_proc
+
     hash = ActiveSupport::HashWithIndifferentAccess.new(hash_with_default_proc).transform_keys!(&:to_s)
-    assert_equal hash_with_default_proc.default_proc, hash.default_proc
+    assert_equal default_proc, hash.default_proc
     hash = ActiveSupport::HashWithIndifferentAccess.new(hash_with_default_proc).transform_keys! { |k| k.to_s }
-    assert_equal hash_with_default_proc.default_proc, hash.default_proc
+    assert_equal default_proc, hash.default_proc
   end
 
   def test_indifferent_deep_transform_keys_bang