Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YAML Serialization fail if there's a class in a module with the same module name. #12290

Closed
hugopl opened this issue Jul 17, 2022 · 2 comments · Fixed by #12537
Closed

YAML Serialization fail if there's a class in a module with the same module name. #12290

hugopl opened this issue Jul 17, 2022 · 2 comments · Fixed by #12537
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:serialization

Comments

@hugopl
Copy link
Contributor

hugopl commented Jul 17, 2022

The serialization macro code doesn't use the :: code when expanding the type, so the code above fails to compile:

require "yaml"

module Foo
  class Foo
  end
  
  class Serialized
    include YAML::Serializable
    @foo = ""
  end
end

Foo::Serialized.from_yaml("")

Error:

There was a problem expanding macro 'macro_140364593214112'

Code in macro 'included'

 9 | ctx.read_alias(node, {{@type}}) do |obj|
                          ^
Called macro defined in macro 'included'

 9 | ctx.read_alias(node, {{@type}}) do |obj|

Which expanded to:

 > 1 | Foo::Serialized
       ^--------------
Error: undefined constant Foo::Serialized

If the class Foo is commented out it works as expected, the problem does not happen on JSON serialization.

@hugopl hugopl added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jul 17, 2022
@asterite
Copy link
Member

I just tried it and if we replace \{{@type}} with self, all YAML specs pass. Maybe that's the solution? 🤔

@hugopl
Copy link
Contributor Author

hugopl commented Sep 28, 2022

It does indeed, I can't think a situation that this change could cause a change in the current API behavior, but I wouldn't be surprised if @HertzDevil appears with a corner case that shows that 😅

I added a test case for this and created a PR, locally it passed all std_spec tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:serialization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants