Make Hash#shift return nil for empty hash

Fixes [Bug #16908]

NEWS.md

@@ -51,6 +51,11 @@ Note that each entry is kept to a minimum, see links for details.
 
 Note: We're only listing outstanding class updates.
 
+* Hash
+    * Hash#shift now always returns nil if the hash is
+      empty, instead of returning the default value or
+      calling the default proc. [[Bug #16908]]
+
 * Module
     * Module.used_refinements has been added. [[Feature #14332]]
     * Module#refinements has been added. [[Feature #12737]]
@@ -137,6 +142,7 @@ The following deprecated APIs are removed.
 [Feature #15231]: https://bugs.ruby-lang.org/issues/15231
 [Bug #15928]:     https://bugs.ruby-lang.org/issues/15928
 [Feature #16131]: https://bugs.ruby-lang.org/issues/16131
+[Bug #16908]:     https://bugs.ruby-lang.org/issues/16908
 [Feature #17351]: https://bugs.ruby-lang.org/issues/17351
 [Feature #17391]: https://bugs.ruby-lang.org/issues/17391
 [Bug #17545]:     https://bugs.ruby-lang.org/issues/17545

hash.c

@@ -2445,7 +2445,7 @@ shift_i_safe(VALUE key, VALUE value, VALUE arg)
 
 /*
  *  call-seq:
- *    hash.shift -> [key, value] or default_value
+ *    hash.shift -> [key, value] or nil
  *
  *  Removes the first hash entry
  *  (see {Entry Order}[#class-Hash-label-Entry+Order]);
@@ -2454,8 +2454,7 @@ shift_i_safe(VALUE key, VALUE value, VALUE arg)
  *    h.shift # => [:foo, 0]
  *    h # => {:bar=>1, :baz=>2}
  *
- *  Returns the default value if the hash is empty
+ *  Returns nil if the hash is empty.
- *  (see {Default Values}[#class-Hash-label-Default+Values]).
  */
 
 static VALUE
@@ -2494,7 +2493,7 @@ rb_hash_shift(VALUE hash)
 	    }
 	}
     }
-    return rb_hash_default_value(hash, Qnil);
+    return Qnil;
 }
 
 static int

spec/ruby/core/hash/shift_spec.rb

@@ -30,23 +30,45 @@ describe "Hash#shift" do
     h.should == {}
   end
 
-  it "calls #default with nil if the Hash is empty" do
+  ruby_version_is '3.2' do
-    h = {}
+    it "returns nil if the Hash is empty" do
-    def h.default(key)
+      h = {}
-      key.should == nil
+      def h.default(key)
-      :foo
+        raise
+      end
+      h.shift.should == nil
+    end
+  end
+
+  ruby_version_is ''...'3.2' do
+    it "calls #default with nil if the Hash is empty" do
+      h = {}
+      def h.default(key)
+        key.should == nil
+        :foo
+      end
+      h.shift.should == :foo
     end
-    h.shift.should == :foo
   end
 
   it "returns nil from an empty hash" do
     {}.shift.should == nil
   end
 
-  it "returns (computed) default for empty hashes" do
+  ruby_version_is '3.2' do
-    Hash.new(5).shift.should == 5
+    it "returns nil for empty hashes with defaults and default procs" do
-    h = Hash.new { |*args| args }
+      Hash.new(5).shift.should == nil
-    h.shift.should == [h, nil]
+      h = Hash.new { |*args| args }
+      h.shift.should == nil
+    end
+  end
+
+  ruby_version_is ''...'3.2' do
+    it "returns (computed) default for empty hashes" do
+      Hash.new(5).shift.should == 5
+      h = Hash.new { |*args| args }
+      h.shift.should == [h, nil]
+    end
   end
 
   it "preserves Hash invariants when removing the last item" do

test/ruby/test_hash.rb

@@ -1048,14 +1048,14 @@ class TestHash < Test::Unit::TestCase
     h = @cls.new {|hh, k| :foo }
     h[1] = 2
     assert_equal([1, 2], h.shift)
-    assert_equal(:foo, h.shift)
+    assert_nil(h.shift)
-    assert_equal(:foo, h.shift)
+    assert_nil(h.shift)
 
     h = @cls.new(:foo)
     h[1] = 2
     assert_equal([1, 2], h.shift)
-    assert_equal(:foo, h.shift)
+    assert_nil(h.shift)
-    assert_equal(:foo, h.shift)
+    assert_nil(h.shift)
 
     h =@cls[1=>2]
     h.each { assert_equal([1, 2], h.shift) }
@@ -1066,7 +1066,7 @@ class TestHash < Test::Unit::TestCase
     def h.default(k = nil)
       super.upcase
     end
-    assert_equal("FOO", h.shift)
+    assert_nil(h.shift)
   end
 
   def test_reject_bang2