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.
This commit is contained in:
nick evans 2024-10-28 15:41:26 -04:00 committed by git
parent a397e4d4b0
commit 136dc52663
4 changed files with 141 additions and 9 deletions

View File

@ -197,10 +197,27 @@ module Psych
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

View File

@ -163,13 +163,41 @@ module Psych
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

69
test/psych/test_data.rb Normal file
View File

@ -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

View File

@ -35,5 +35,23 @@ module Psych
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