Merge pull request #765 from nevans/replace-c-ext-init_data-with-Data-initialize-bind_call

Replace C extension `ToRuby#init_data` with `Data#initialize` bind_call

Author: tenderlove

.github/workflows/test.yml

@@ -26,12 +26,11 @@ jobs:
         ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }}
         os: [ ubuntu-latest, macos-latest, windows-latest ]
         include:
-          # jruby is broken with "undefined method 'init_data'"
-          # https://github.com/ruby/psych/actions/runs/15434465445/job/43438083198?pr=734
+          # jruby-10.0.2.0 does not support Data instances with ivars
           - { os: windows-latest, ruby: jruby-head }
           - { os: macos-latest, ruby: jruby-head }
           - { os: ubuntu-latest, ruby: jruby-head }
-          # Needs truffleruby-head for rb_struct_initialize() (which truffleruby 25.0 does not have)
+          # Needs truffleruby-head for (base) Data#initialize (which truffleruby 25.0 does not have)
           - { os: ubuntu-latest, ruby: truffleruby-head }
           - { os: macos-latest, ruby: truffleruby-head }
           - { os: windows-latest, ruby: ucrt }
@@ -59,7 +58,6 @@ jobs:
       - name: Run test
         id: test
         run: rake
-        continue-on-error: ${{ matrix.ruby == 'jruby-head' }}
       - name: Install gem
         run: rake install
         if: ${{ steps.test.outcome == 'success' }}

ext/psych/psych_to_ruby.c

@@ -28,12 +28,6 @@ static VALUE path2class(VALUE self, VALUE path)
     return rb_path_to_class(path);
 }
 
-static VALUE init_data(VALUE self, VALUE data, VALUE values)
-{
-    rb_struct_initialize(data, values);
-    return data;
-}
-
 void Init_psych_to_ruby(void)
 {
     VALUE psych     = rb_define_module("Psych");
@@ -43,7 +37,6 @@ void Init_psych_to_ruby(void)
     VALUE visitor   = rb_define_class_under(visitors, "Visitor", rb_cObject);
     cPsychVisitorsToRuby = rb_define_class_under(visitors, "ToRuby", visitor);
 
-    rb_define_private_method(cPsychVisitorsToRuby, "init_data", init_data, 2);
     rb_define_private_method(cPsychVisitorsToRuby, "build_exception", build_exception, 2);
     rb_define_private_method(class_loader, "path2class", path2class, 1);
 }

lib/psych/visitors/to_ruby.rb

@@ -12,6 +12,10 @@ module Visitors
     ###
     # This class walks a YAML AST, converting each node to Ruby
     class ToRuby < Psych::Visitors::Visitor
+      unless RUBY_VERSION < "3.2"
+        DATA_INITIALIZE = Data.instance_method(:initialize)
+      end
+
       def self.create(symbolize_names: false, freeze: false, strict_integer: false, parse_symbols: true)
         class_loader = ClassLoader.new
         scanner      = ScalarScanner.new class_loader, strict_integer: strict_integer, parse_symbols: parse_symbols
@@ -219,8 +223,7 @@ def visit_Psych_Nodes_Mapping o
             revive_data_members(members, o)
           end
           data ||= allocate_anon_data(o, members)
-          values = data.members.map { |m| members[m] }
-          init_data(data, values)
+          DATA_INITIALIZE.bind_call(data, **members)
           data.freeze
           data
 

test/psych/test_data.rb

@@ -64,6 +64,31 @@ def test_load
       assert_equal "hello", obj.bar
       assert_equal "bar",   obj.foo
     end
+
+    def test_members_must_be_identical
+      TestData.const_set :D, Data.define(:a, :b)
+      d = Psych.dump(TestData::D.new(1, 2))
+
+      # more members
+      TestData.send :remove_const, :D
+      TestData.const_set :D, Data.define(:a, :b, :c)
+      e = assert_raise(ArgumentError) { Psych.unsafe_load d }
+      assert_equal 'missing keyword: :c', e.message
+
+      # less members
+      TestData.send :remove_const, :D
+      TestData.const_set :D, Data.define(:a)
+      e = assert_raise(ArgumentError) { Psych.unsafe_load d }
+      assert_equal 'unknown keyword: :b', e.message
+
+      # completely different members
+      TestData.send :remove_const, :D
+      TestData.const_set :D, Data.define(:foo, :bar)
+      e = assert_raise(ArgumentError) { Psych.unsafe_load d }
+      assert_equal 'unknown keywords: :a, :b', e.message
+    ensure
+      TestData.send :remove_const, :D
+    end
   end
 end