Add support for Data objects with ivars

This sets the ivars _before_ calling initialize, which feels wrong.  But
Data doesn't give us any mechanism for setting the members other than 1)
initialize, or 2) drop down into the C API.  Since initialize freezes
the object, we need to set the ivars before that.  I think this is a
reasonable compromise—if users need better handling, they can implement
their own `encode_with` and `init_with`.  But it will lead to unhappy
surprises for some users.

Alternatively, we could use the C API, similarly to Marshal.  Psych _is_
already using the C API for path2class and build_exception.  This would
be the least surprising behavior for users, I think.

Author: nevans

lib/psych/visitors/to_ruby.rb

@@ -197,10 +197,27 @@ def visit_Psych_Nodes_Mapping o
             s
           end
 
-        when /^!ruby\/data(?::(.*))?$/
-          data = register(o, resolve_class($1).allocate) if $1
+        when /^!ruby\/data(-with-ivars)?(?::(.*))?$/
+          data = register(o, resolve_class($2).allocate) if $2
           members = {}
-          revive_data_members(members, o)
+
+          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)
           data.send(:initialize, **members)
           data

lib/psych/visitors/yaml_tree.rb

@@ -163,13 +163,41 @@ def visit_Object o
       alias :visit_Delegator :visit_Object
 
       def visit_Data o
-        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
+        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
+
+        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
         end
-        @emitter.end_mapping
       end
 
       def visit_Struct o

test/psych/test_data.rb

@@ -0,0 +1,69 @@
+# 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"
+
+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
+      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")
+      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
+      eoyml
+
+      assert_equal "hello", obj.bar
+      assert_equal "bar",   obj.foo
+    end
+  end
+end
+

test/psych/test_serialize_subclasses.rb

@@ -35,5 +35,23 @@ def test_struct_subclass
       so = StructSubclass.new('foo', [1,2,3])
       assert_equal so, Psych.unsafe_load(Psych.dump(so))
     end
+
+    class DataSubclass < Data.define(:foo)
+      def initialize(foo:)
+        @bar = "hello #{foo}"
+        super(foo: foo)
+      end
+
+      def == other
+        super(other) && @bar == other.instance_eval{ @bar }
+      end
+    end unless RUBY_VERSION < "3.2"
+
+    def test_data_subclass
+      omit "Data requires ruby >= 3.2" if RUBY_VERSION < "3.2"
+      so = DataSubclass.new('foo')
+      assert_equal so, Psych.unsafe_load(Psych.dump(so))
+    end
+
   end
 end