Only support dumping named, non-ivar Data

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

@@ -197,32 +197,11 @@ def visit_Psych_Nodes_Mapping o
             s
           end
 
-        when /^!ruby\/data(-with-ivars)?(?::(.*))?$/
-          data = register(o, resolve_class($2).allocate) if $2
-          members = {}
-
-          if $1 # data-with-ivars
-            ivars   = {}
-            o.children.each_slice(2) do |type, vars|
-              case accept(type)
-              when 'members'
-                revive_data_members(members, vars)
-                data ||= allocate_anon_data(o, members)
-              when 'ivars'
-                revive_hash(ivars, vars)
-              end
-            end
-            ivars.each do |ivar, v|
-              data.instance_variable_set ivar, v
-            end
-          else
-            revive_data_members(members, o)
-          end
-          data ||= allocate_anon_data(o, members)
-          values = data.members.map { |m| members[m] }
-          init_data(data, values)
-          data.freeze
-          data
+        when /^!ruby\/data?(?::(.*))?$/
+          data_class = resolve_class($1) if $1
+          members = revive_data_members(o)
+          data = (data_class || Data.define(*members.keys)).new(**members)
+          register(o, data) # replace temp with actual
 
         when /^!ruby\/object:?(.*)?$/
           name = $1 || 'Object'
@@ -372,7 +351,8 @@ def allocate_anon_data node, members
         register(node, klass.allocate)
       end
 
-      def revive_data_members hash, o
+      def revive_data_members o
+        hash = {}
         o.children.each_slice(2) do |k,v|
           name  = accept(k)
           value = accept(v)

lib/psych/visitors/yaml_tree.rb

@@ -163,41 +163,16 @@ def visit_Object o
       alias :visit_Delegator :visit_Object
 
       def visit_Data o
-        ivars = o.instance_variables
-        if ivars.empty?
-          tag = ['!ruby/data', o.class.name].compact.join(':')
-          register o, @emitter.start_mapping(nil, tag, false, Nodes::Mapping::BLOCK)
-          o.members.each do |member|
-            @emitter.scalar member.to_s, nil, nil, true, false, Nodes::Scalar::ANY
-            accept o.send member
-          end
-          @emitter.end_mapping
+        raise TypeError, "can't dump Data with ivars: #{o}" unless o.instance_variables.empty?
+        raise TypeError, "can't dump anonymous Data: #{o}" unless o.class.name
 
-        else
-          tag = ['!ruby/data-with-ivars', o.class.name].compact.join(':')
-          node = @emitter.start_mapping(nil, tag, false, Psych::Nodes::Mapping::BLOCK)
-          register(o, node)
-
-          # Dump the members
-          accept 'members'
-          @emitter.start_mapping nil, nil, true, Nodes::Mapping::BLOCK
-          o.members.each do |member|
-            @emitter.scalar member.to_s, nil, nil, true, false, Nodes::Scalar::ANY
-            accept o.send member
-          end
-          @emitter.end_mapping
-
-          # Dump the ivars
-          accept 'ivars'
-          @emitter.start_mapping nil, nil, true, Nodes::Mapping::BLOCK
-          ivars.each do |ivar|
-            accept ivar.to_s
-            accept o.instance_variable_get ivar
-          end
-          @emitter.end_mapping
-
-          @emitter.end_mapping
+        tag = ['!ruby/data', o.class.name].compact.join(':')
+        register o, @emitter.start_mapping(nil, tag, false, Nodes::Mapping::BLOCK)
+        o.members.each do |member|
+          @emitter.scalar member.to_s, nil, nil, true, false, Nodes::Scalar::ANY
+          accept o.send member
         end
+        @emitter.end_mapping
       end unless RUBY_VERSION < "3.2"
 
       def visit_Struct o

test/psych/test_coder.rb

@@ -220,9 +220,9 @@
     end
 
     def test_coder_style_map_any
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       foo = Psych.dump CustomEncode.new \
         map: {a: 1, b: 2},
         style: Psych::Nodes::Mapping::ANY,
         tag: nil
@@ -230,9 +230,9 @@
     end
 
     def test_coder_style_map_block
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       foo = Psych.dump CustomEncode.new \
         map: {a: 1, b: 2},
         style: Psych::Nodes::Mapping::BLOCK,
         tag: nil
@@ -240,9 +240,9 @@
     end
 
     def test_coder_style_map_flow
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       foo = Psych.dump CustomEncode.new \
         map: { a: 1, b: 2 },
         style: Psych::Nodes::Mapping::FLOW,
         tag: nil

test/psych/test_data.rb

@@ -1,67 +1,35 @@
 # frozen_string_literal: true
 require_relative 'helper'
 
-class PsychDataWithIvar < Data.define(:foo)
-  attr_reader :bar
-  def initialize(**)
-    @bar = 'hello'
-    super
-  end
-end unless RUBY_VERSION < "3.2"
+PsychData = Data.define(:foo) unless RUBY_VERSION < "3.2"
 
 module Psych
   class TestData < TestCase
-    class SelfReferentialData < Data.define(:foo)
-      attr_accessor :ref
-      def initialize(foo:)
-        @ref = self
-        super
-      end
-    end unless RUBY_VERSION < "3.2"
-
     def setup
       omit "Data requires ruby >= 3.2" if RUBY_VERSION < "3.2"
     end
 
     # TODO: move to another test?
     def test_dump_data
-      assert_equal <<~eoyml, Psych.dump(PsychDataWithIvar["bar"])
-        --- !ruby/data-with-ivars:PsychDataWithIvar
-        members:
-          foo: bar
-        ivars:
-          "@bar": hello
+      assert_equal <<~eoyml, Psych.dump(PsychData["bar"])
+        --- !ruby/data:PsychData
+        foo: bar
       eoyml
     end
 
-    def test_self_referential_data
-      circular = SelfReferentialData.new("foo")
-
-      loaded = Psych.unsafe_load(Psych.dump(circular))
-      assert_instance_of(SelfReferentialData, loaded.ref)
-
-      assert_equal(circular, loaded)
-      assert_same(loaded, loaded.ref)
-    end
-
     def test_roundtrip
-      thing = PsychDataWithIvar.new("bar")
+      thing = PsychData.new("bar")
       data = Psych.unsafe_load(Psych.dump(thing))
 
-      assert_equal "hello", data.bar
       assert_equal "bar",   data.foo
     end
 
     def test_load
       obj = Psych.unsafe_load(<<~eoyml)
-        --- !ruby/data-with-ivars:PsychDataWithIvar
-        members:
-          foo: bar
-        ivars:
-          "@bar": hello
+        --- !ruby/data:PsychData
+        foo: bar
       eoyml
 
-      assert_equal "hello", obj.bar
       assert_equal "bar",   obj.foo
     end
   end

test/psych/test_date_time.rb

@@ -87,7 +87,7 @@ def test_alias_with_time
     end
 
     def test_overwritten_to_s
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
       s = Psych.dump(Date.new(2023, 9, 2), permitted_classes: [Date])
       assert_separately(%W[-rpsych -rdate - #{s}], "#{<<~"begin;"}\n#{<<~'end;'}")
       class Date

test/psych/test_encoding.rb

@@ -119,7 +119,7 @@
     end
 
     def test_emit_alias
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @emitter.start_stream Psych::Parser::UTF8
       @emitter.start_document [], [], true
@@ -153,9 +153,9 @@
       @emitter.end_mapping
       @emitter.end_document false
       @emitter.end_stream
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse @buffer.string
       assert_encodings @utf8, @handler.strings
       assert_equal [foo, bar], @handler.strings
     end
@@ -173,9 +173,9 @@
       @emitter.end_sequence
       @emitter.end_document false
       @emitter.end_stream
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse @buffer.string
       assert_encodings @utf8, @handler.strings
       assert_equal [foo, bar], @handler.strings
     end
@@ -191,9 +191,9 @@
       @emitter.scalar 'foo', nil, nil, true, false, Nodes::Scalar::ANY
       @emitter.end_document false
       @emitter.end_stream
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse @buffer.string
       assert_encodings @utf8, @handler.strings
       assert_equal key, @handler.strings[1]
     end
@@ -268,7 +268,7 @@
     end
 
     def test_dump_non_ascii_string_to_file
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       Tempfile.create(['utf8', 'yml'], :encoding => 'UTF-8') do |t|
         h = {'one' => 'いち'}

test/psych/test_object_references.rb

@@ -31,11 +31,6 @@ def test_struct_has_references
       assert_reference_trip Struct.new(:foo).new(1)
     end
 
-    def test_data_has_references
-      omit "Data requires ruby >= 3.2" if RUBY_VERSION < "3.2"
-      assert_reference_trip Data.define(:foo).new(1)
-    end
-
     def assert_reference_trip obj
       yml = Psych.dump([obj, obj])
       assert_match(/\*-?\d+/, yml)

test/psych/test_parser.rb

@@ -85,11 +85,11 @@
 
     def test_line_numbers
       assert_equal 0, @parser.mark.line
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse "---\n- hello\n- world"
       line_calls = @handler.marks.map(&:line).zip(@handler.calls.map(&:first))
       assert_equal [
                     [0, :event_location],
                     [0, :start_stream],
                     [0, :event_location],
@@ -112,11 +112,11 @@
 
     def test_column_numbers
       assert_equal 0, @parser.mark.column
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse "---\n- hello\n- world"
       col_calls = @handler.marks.map(&:column).zip(@handler.calls.map(&:first))
       assert_equal [
                     [0, :event_location],
                     [0, :start_stream],
                     [3, :event_location],
@@ -139,11 +139,11 @@
 
     def test_index_numbers
       assert_equal 0, @parser.mark.index
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse "---\n- hello\n- world"
       idx_calls = @handler.marks.map(&:index).zip(@handler.calls.map(&:first))
       assert_equal [
                     [0, :event_location],
                     [0, :start_stream],
                     [3, :event_location],
@@ -358,7 +358,7 @@
     end
 
     def test_event_location
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       @parser.parse "foo:\n" \
                     "  barbaz: [1, 2]"
@@ -367,7 +367,7 @@
         [event[0], location[1]]
       end
 
       assert_equal [
                      [:start_stream, [0, 0, 0, 0]],
                      [:start_document, [0, 0, 0, 0]],
                      [:start_mapping, [0, 0, 0, 0]],

test/psych/test_safe_load.rb

@@ -124,28 +124,6 @@ def test_data_depends_on_sym
       end
     end
 
-    def test_anon_data
-      omit "Data requires ruby >= 3.2" if RUBY_VERSION < "3.2"
-      assert Psych.safe_load(<<-eoyml, permitted_classes: [Data, Symbol])
---- !ruby/data
-  foo: bar
-      eoyml
-
-      assert_raise(Psych::DisallowedClass) do
-        Psych.safe_load(<<-eoyml, permitted_classes: [Data])
---- !ruby/data
-  foo: bar
-        eoyml
-      end
-
-      assert_raise(Psych::DisallowedClass) do
-        Psych.safe_load(<<-eoyml, permitted_classes: [Symbol])
---- !ruby/data
-  foo: bar
-        eoyml
-      end
-    end
-
     def test_safe_load_default_fallback
       assert_nil Psych.safe_load("")
     end

test/psych/test_serialize_subclasses.rb

@@ -38,12 +38,11 @@ def test_struct_subclass
 
     class DataSubclass < Data.define(:foo)
       def initialize(foo:)
-        @bar = "hello #{foo}"
         super(foo: foo)
       end
 
       def == other
-        super(other) && @bar == other.instance_eval{ @bar }
+        super(other)
       end
     end unless RUBY_VERSION < "3.2"
 

test/psych/test_string.rb

@@ -51,7 +51,7 @@ def test_doublequotes_when_there_is_a_single
     end
 
     def test_single_quote_when_matching_date
-      pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
+      # pend "Failing on JRuby" if RUBY_PLATFORM =~ /java/
 
       lib = File.expand_path("../../../lib", __FILE__)
       assert_separately(["-I", lib, "-r", "psych"], __FILE__, __LINE__ + 1, <<~'RUBY')

test/psych/visitors/test_yaml_tree.rb

@@ -80,20 +80,6 @@ def test_data
         assert_cycle D.new('bar')
       end
 
-      def test_data_anon
-        omit "Data requires ruby >= 3.2" if RUBY_VERSION < "3.2"
-        d = Data.define(:foo).new('bar')
-        obj =  Psych.unsafe_load(Psych.dump(d))
-        assert_equal d.foo, obj.foo
-      end
-
-      def test_data_override_method
-        omit "Data requires ruby >= 3.2" if RUBY_VERSION < "3.2"
-        d = Data.define(:method).new('override')
-        obj =  Psych.unsafe_load(Psych.dump(d))
-        assert_equal d.method, obj.method
-      end
-
       def test_exception
         ex = Exception.new 'foo'
         loaded = Psych.unsafe_load(Psych.dump(ex))