Merge pull request #16149 from sferik/refactor-transform_values

Refactor Hash#transform_values and Hash#transform_values!

4 files changed, 50 insertions, 2 deletions

diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb
index 8657f34be2..f4105f66b0 100644
--- a/activesupport/lib/active_support/core_ext/hash/keys.rb
+++ b/activesupport/lib/active_support/core_ext/hash/keys.rb
@@ -6,7 +6,8 @@ class Hash
   #  hash.transform_keys{ |key| key.to_s.upcase }
   #  # => {"NAME"=>"Rob", "AGE"=>"28"}
   def transform_keys
-    result = {}
+    return enum_for(:transform_keys) unless block_given?
+    result = self.class.new
     each_key do |key|
       result[yield(key)] = self[key]
     end
@@ -16,6 +17,7 @@ class Hash
   # Destructively convert all keys using the block operations.
   # Same as transform_keys but modifies +self+.
   def transform_keys!
+    return enum_for(:transform_keys!) unless block_given?
     keys.each do |key|
       self[yield(key)] = delete(key)
     end
diff --git a/activesupport/lib/active_support/core_ext/hash/transform_values.rb b/activesupport/lib/active_support/core_ext/hash/transform_values.rb
index 6ff7e91212..e9bcce761f 100644
--- a/activesupport/lib/active_support/core_ext/hash/transform_values.rb
+++ b/activesupport/lib/active_support/core_ext/hash/transform_values.rb
@@ -4,7 +4,8 @@ class Hash
   #
   #   { a: 1, b: 2, c: 3 }.transform_values { |x| x * 2 }
   #   # => { a: 2, b: 4, c: 6 }
-  def transform_values(&block)
+  def transform_values
+    return enum_for(:transform_values) unless block_given?
     result = self.class.new
     each do |key, value|
       result[key] = yield(value)
@@ -14,6 +15,7 @@ class Hash
 
   # Destructive +transform_values+
   def transform_values!
+    return enum_for(:transform_values!) unless block_given?
     each do |key, value|
       self[key] = yield(value)
     end
diff --git a/activesupport/test/core_ext/hash/transform_keys_test.rb b/activesupport/test/core_ext/hash/transform_keys_test.rb
new file mode 100644
index 0000000000..956fd7ae97
--- /dev/null
+++ b/activesupport/test/core_ext/hash/transform_keys_test.rb
@@ -0,0 +1,32 @@
+require 'abstract_unit'
+require 'active_support/core_ext/hash/transform_keys'
+
+class TransformKeysTest < ActiveSupport::TestCase
+  test "transform_keys returns a new hash with the keys computed from the block" do
+    original = { a: 'a', b: 'b' }
+    mapped = original.transform_keys { |k| "#{k}!".to_sym }
+
+    assert_equal({ a: 'a', b: 'b' }, original)
+    assert_equal({ a!: 'a', b!: 'b' }, mapped)
+  end
+
+  test "transform_keys! modifies the keys of the original" do
+    original = { a: 'a', b: 'b' }
+    mapped = original.transform_keys! { |k| "#{k}!".to_sym }
+
+    assert_equal({ a!: 'a', b!: 'b' }, original)
+    assert_same original, mapped
+  end
+
+  test "transform_keys returns an Enumerator if no block is given" do
+    original = { a: 'a', b: 'b' }
+    enumerator = original.transform_keys
+    assert_equal Enumerator, enumerator.class
+  end
+
+  test "transform_keys is chainable with Enumerable methods" do
+    original = { a: 'a', b: 'b' }
+    mapped = original.transform_keys.with_index { |k, i| [k, i].join.to_sym }
+    assert_equal({ a0: 'a', b1: 'b' }, mapped)
+  end
+end
diff --git a/activesupport/test/core_ext/hash/transform_values_test.rb b/activesupport/test/core_ext/hash/transform_values_test.rb
index 4449c94701..45ed11fef7 100644
--- a/activesupport/test/core_ext/hash/transform_values_test.rb
+++ b/activesupport/test/core_ext/hash/transform_values_test.rb
@@ -46,4 +46,16 @@ class TransformValuesTest < ActiveSupport::TestCase
     assert_equal 'a!', mapped[:a]
     assert_nil mapped[:b]
   end
+
+  test "transform_values returns an Enumerator if no block is given" do
+    original = { a: 'a', b: 'b' }
+    enumerator = original.transform_values
+    assert_equal Enumerator, enumerator.class
+  end
+
+  test "transform_values is chainable with Enumerable methods" do
+    original = { a: 'a', b: 'b' }
+    mapped = original.transform_values.with_index { |v, i| [v, i].join }
+    assert_equal({ a: 'a0', b: 'b1' }, mapped)
+  end
 end