MaKe Hash#slice! return removed values, akin to Array [#971 state:resolved]

Signed-off-by: Frederick Cheung <frederick.cheung@gmail.com>

activesupport/lib/active_support/core_ext/hash/slice.rb

@@ -24,10 +24,17 @@ def slice(*keys)
         end
 
         # Replaces the hash with only the given keys.
+        # Returns a hash contained the removed key/value pairs
+        #   {:a => 1, :b => 2, :c => 3, :d => 4}.slice!(:a, :b) # => {:c => 3, :d =>4}
         def slice!(*keys)
-          replace(slice(*keys))
+          keys = keys.map! { |key| convert_key(key) } if respond_to?(:convert_key)
+          omit = slice(*self.keys - keys)
+          hash = slice(*keys)
+          replace(hash)
+          omit
         end
       end
     end
   end
 end
+

activesupport/test/core_ext/hash_ext_test.rb

@@ -287,10 +287,14 @@ def test_slice
     # Should return a new hash with only the given keys.
     assert_equal expected, original.slice(:a, :b)
     assert_not_equal expected, original
+  end
+
+  def test_slice_inplace
+    original = { :a => 'x', :b => 'y', :c => 10 }
+    expected = { :c => 10 }
 
     # Should replace the hash with only the given keys.
     assert_equal expected, original.slice!(:a, :b)
-    assert_equal expected, original
   end
 
   def test_slice_with_an_array_key
@@ -300,10 +304,14 @@ def test_slice_with_an_array_key
     # Should return a new hash with only the given keys when given an array key.
     assert_equal expected, original.slice([:a, :b], :c)
     assert_not_equal expected, original
+  end
+
+  def test_slice_inplace_with_an_array_key
+    original = { :a => 'x', :b => 'y', :c => 10, [:a, :b] => "an array key" }
+    expected = { :a => 'x', :b => 'y' }
 
     # Should replace the hash with only the given keys when given an array key.
     assert_equal expected, original.slice!([:a, :b], :c)
-    assert_equal expected, original
   end
 
   def test_slice_with_splatted_keys
@@ -322,11 +330,17 @@ def test_indifferent_slice
       # Should return a new hash with only the given keys.
       assert_equal expected, original.slice(*keys), keys.inspect
       assert_not_equal expected, original
+    end
+  end
 
+  def test_indifferent_slice_inplace
+    original = { :a => 'x', :b => 'y', :c => 10 }.with_indifferent_access
+    expected = { :c => 10 }.with_indifferent_access
+
+    [['a', 'b'], [:a, :b]].each do |keys|
       # Should replace the hash with only the given keys.
       copy = original.dup
       assert_equal expected, copy.slice!(*keys)
-      assert_equal expected, copy
     end
   end