Eliminate support for anon Data or Data with ivars

This commit removes support for the following features:

* Dumping an anonymous Data. This is unsupported for the same
  reasons as dumping anonymous modules or classes: there's no way
  to reference the proper Data class, and defining a new anonymous
  Data for every such element would not round-trip properly.
* Dumping a Data with instance variables. Instance variables on a
  Data can only be set during the initialize chain when first
  constructed, and such a chain is custom for each such Data class.
  As there is no way to modify a Data after constructed and no way
  to accurately invoke the custom initialize method, all Data with
  instance variables are rejected.

Data is also no longer registered while loading, since it's not
possible to have a self-referential Data instance without a custom
initialize that sets an instance variable to self. The combination
of these removed features simplifies Data dumping and loading and
eliminates all allocate+initialize hacks.

Author: headius

lib/psych/visitors/to_ruby.rb

@@ -197,30 +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)
-          data.__send__ :initialize, **members
-          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'
@@ -370,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_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_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_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/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))