summaryloggraphiclog1graphiclog2commitcommitdifftreerefseditfork
rawpatchinlineside by side (parent: a397e4d)
rawpatchinlineside by side (parent: a397e4d)
| author | nick evans <[email protected]> | |
| Mon, 28 Oct 2024 19:41:26 +0000 (28 15:41 -0400) | ||
| committer | git <[email protected]> | |
| Thu, 1 May 2025 17:52:14 +0000 (1 17:52 +0000) |
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.
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.
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
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
diff --git a/test/psych/test_data.rb b/test/psych/test_data.rb
--- /dev/null
+++ b/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
+
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