Refactor {Hash, Array, Enumerable, Struct , ENV}#to_h methods and extract processing of returned by a block value into a helper method

Author: andrykonchin

spec/ruby/core/array/to_h_spec.rb

@@ -45,6 +45,12 @@
       [:a, :b].to_h { |k| [k, k.to_s] }.should == { a: 'a', b: 'b' }
     end
 
+    it "passes to a block each element as a single argument" do
+      ScratchPad.record []
+      [[:a, 1], [:b, 2]].to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[[:a, 1]], [[:b, 2]]]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         [:a, :b].to_h { |k| [k, k.to_s, 1] }

spec/ruby/core/enumerable/to_h_spec.rb

@@ -53,6 +53,14 @@ def enum.each(*args)
       @enum.to_h { |k| [k, k.to_s] }.should == { a: 'a', b: 'b' }
     end
 
+    it "passes to a block each element as a single argument" do
+      enum_of_arrays = EnumerableSpecs::EachDefiner.new([:a, 1], [:b, 2])
+
+      ScratchPad.record []
+      enum_of_arrays.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[[:a, 1]], [[:b, 2]]]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         @enum.to_h { |k| [k, k.to_s, 1] }

spec/ruby/core/env/to_h_spec.rb

@@ -18,6 +18,18 @@
       ENV.to_h { |k, v| [k, v.upcase] }.should == { 'a' => "B", 'c' => "D" }
     end
 
+    it "passes to a block each pair's key and value as separate arguments" do
+      ENV.replace("a" => "b", "c" => "d")
+
+      ScratchPad.record []
+      ENV.to_h { |k, v| ScratchPad << [k, v]; [k, v] }
+      ScratchPad.recorded.sort.should == [["a", "b"], ["c", "d"]]
+
+      ScratchPad.record []
+      ENV.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [["a", "b"], ["c", "d"]]
+    end
+
     it "does not require the array elements to be strings" do
       ENV.replace("a" => "b", "c" => "d")
       ENV.to_h { |k, v| [k.to_sym, v.to_sym] }.should == { :a => :b, :c => :d }

spec/ruby/core/struct/to_h_spec.rb

@@ -21,6 +21,18 @@
       h.should == { "make" => "ford", "model" => "ranger", "year" => "" }
     end
 
+    it "passes to a block each pair's key and value as separate arguments" do
+      s = StructClasses::Ruby.new('3.2.4', 'macos')
+
+      ScratchPad.record []
+      s.to_h { |k, v| ScratchPad << [k, v]; [k, v] }
+      ScratchPad.recorded.sort.should == [[:platform, 'macos'], [:version, '3.2.4']]
+
+      ScratchPad.record []
+      s.to_h { |*args| ScratchPad << args; [args[0], args[1]] }
+      ScratchPad.recorded.sort.should == [[:platform, 'macos'], [:version, '3.2.4']]
+    end
+
     it "raises ArgumentError if block returns longer or shorter array" do
       -> do
         StructClasses::Car.new.to_h { |k, v| [k.to_s, "#{v}".downcase, 1] }

src/main/ruby/truffleruby/core/array.rb

@@ -1296,19 +1296,10 @@ def to_ary
   end
 
   def to_h
-    h = Hash.new
+    h = {}
     each_with_index do |elem, i|
       elem = yield(elem) if block_given?
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} at #{i} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "wrong array length at #{i} (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+      Truffle::HashOperations.assoc_key_value_pair_with_position(h, elem, i)
     end
     h
   end

src/main/ruby/truffleruby/core/enumerable.rb

@@ -319,16 +319,7 @@ def to_h(*args)
     each(*args) do
       elem = Primitive.single_block_arg
       elem = yield(elem) if block_given?
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+      Truffle::HashOperations.assoc_key_value_pair(h, elem)
     end
     h
   end

src/main/ruby/truffleruby/core/env.rb

@@ -286,18 +286,9 @@ def to_h
     return to_hash unless block_given?
 
     h = {}
-    each_pair do |*elem|
-      elem = yield(elem)
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+    each_pair do |k, v|
+      pair = yield(k, v)
+      Truffle::HashOperations.assoc_key_value_pair(h, pair)
     end
     h
   end

src/main/ruby/truffleruby/core/hash.rb

@@ -357,17 +357,8 @@ def to_h
     if block_given?
       h = {}
       each do |k, v|
-        elem = yield(k, v)
-        unless elem.respond_to?(:to_ary)
-          raise TypeError, "wrong element type #{Primitive.class(elem)} (expected array)"
-        end
-
-        ary = elem.to_ary
-        if ary.size != 2
-          raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-        end
-
-        h[ary[0]] = ary[1]
+        pair = yield(k, v)
+        Truffle::HashOperations.assoc_key_value_pair(h, pair)
       end
       h
     elsif instance_of? Hash

src/main/ruby/truffleruby/core/struct.rb

@@ -109,18 +109,9 @@ def select
 
   def to_h
     h = {}
-    each_pair.each_with_index do |elem, i|
-      elem = yield(elem) if block_given?
-      unless elem.respond_to?(:to_ary)
-        raise TypeError, "wrong element type #{Primitive.class(elem)} at #{i} (expected array)"
-      end
-
-      ary = elem.to_ary
-      if ary.size != 2
-        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
-      end
-
-      h[ary[0]] = ary[1]
+    each_pair do |k, v|
+      pair = block_given? ? yield(k, v) : [k, v]
+      Truffle::HashOperations.assoc_key_value_pair(h, pair)
     end
     h
   end

src/main/ruby/truffleruby/core/truffle/hash_operations.rb

@@ -17,5 +17,35 @@ def self.hash_merge(current, other)
       end
       new_hash
     end
+
+    # MRI: rb_hash_set_pair (rb_ary_to_h also contains similar logic for Array#to_h)
+    def self.assoc_key_value_pair(hash, pair)
+      ary = Truffle::Type.rb_check_convert_type pair, Array, :to_ary
+
+      unless ary
+        raise TypeError, "wrong element type #{Primitive.class(pair)} (expected array)"
+      end
+
+      if ary.size != 2
+        raise ArgumentError, "element has wrong array length (expected 2, was #{ary.size})"
+      end
+
+      hash[ary[0]] = ary[1]
+    end
+
+    # MRI: extracted from rb_ary_to_h, is similar to rb_hash_set_pair
+    def self.assoc_key_value_pair_with_position(hash, pair, index)
+      ary = Truffle::Type.rb_check_convert_type pair, Array, :to_ary
+
+      unless ary
+        raise TypeError, "wrong element type #{Primitive.class(pair)} at #{index} (expected array)"
+      end
+
+      if ary.size != 2
+        raise ArgumentError, "wrong array length at #{index} (expected 2, was #{ary.size})"
+      end
+
+      hash[ary[0]] = ary[1]
+    end
   end
 end