ruby / psych

A libyaml wrapper for Ruby
MIT License
566 stars 205 forks source link

Breaking change in 5.2.0 #694

Closed tdeo closed 6 days ago

tdeo commented 2 weeks ago

I've run into the following behavior after upgrading to psych v5.2.0:

With 5.1.2

irb(main):001> require 'yaml'
=> true
irb(main):002> Gem.loaded_specs['psych'].version
=> Gem::Version.new("5.1.2")
irb(main):003> '2020-01-01'.to_yaml
=> "--- '2020-01-01'\n"

Observe how the string is quoted in the Yaml file

With 5.2.0

irb(main):001> require 'yaml'
=> true
irb(main):002> Gem.loaded_specs['psych'].version
=> Gem::Version.new("5.2.0")
irb(main):003> '2020-01-01'.to_yaml
=> "--- 2020-01-01\n"

Now the string is unquoted, which makes it invalid to be parsed back with YAML.safe_load.

I believe this behavior originates from this commit, also because I noticed that manually requiring 'date' before calling .to_yaml still yields the expected result of v5.1.2:

irb(main):001> require 'yaml'
=> true
irb(main):002> Gem.loaded_specs['psych'].version
=> Gem::Version.new("5.2.0")
irb(main):003> require 'date'
=> true
irb(main):004> '2020-01-01'.to_yaml
=> "--- '2020-01-01'\n"

I think the following error probably hints as to where the problem originates from:

irb(main):001> require 'yaml'; Psych::ClassLoader.new.date
/Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:58:in `path2class': undefined class/module Date (ArgumentError)

        path2class(name)
                   ^^^^
    from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:58:in `resolve'
    from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:50:in `find'
    from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:30:in `load'
    from /Users/thierry/.asdf/installs/ruby/3.3.5/lib/ruby/gems/3.3.0/gems/psych-5.2.0/lib/psych/class_loader.rb:42:in `date'
    from (irb):1:in `<main>'

and that actually, autoload :Date, date probably rather belongs in Psych::ClassLoader than Psych::ScalarScanner. I'll try writing a regression and submit a PR if I succeed

tdeo commented 1 week ago

@byroot did you see this issue and the related PR by any chance as you authored the commit I suspect to be responsible?

byroot commented 1 week ago

No, I'm the maintainer, but I'll have a look.

byroot commented 1 week ago

I can repro the same issue before my change.

byroot commented 1 week ago

I can repro the same issue before my change.

Nevermind, my LOAD PATH was incorrect, it's indeed a regression.