annotation Ann
end
module UsingAnn
def method
{% for ivar in @type.instance_vars %}
{{ ivar.annotation(::Ann)[0].resolve }}
{% end %}
end
end
class Foo
include UsingAnn
@[Ann(Bar)]
getter foo : String?
module Bar
end
end
Foo.new.method
This code should find the type Bar as it's in the namespace where the annotation is used, but the resolve call fails because Bar is looked up in the UsingAnn namespace instead.
Found using this snippet, which fails to compile:
require "json"
class Foo
include JSON::Serializable
@[JSON::Field(converter: Converter)]
getter foo : String
module Converter
def self.from_json(pull)
"foo"
end
end
end
Foo.from_json("")
Can it be related to https://github.com/crystal-lang/crystal/issues/6655?
I knew about this problem but didn't say anything. I don't know how it can be fixed. The problem is not just resolve, is pasting the type in the macro, which ends up in a different context (resolve isn't being used in the macro of JSON::Serialiazable).
The other problem is a converter can also be Time::Format.new("..."), and resolve won't be useful there.
I think annotations were not well thought (my fault, again). Or maybe all expressions in annotations should try to be resolved (but then I have no idea how will they "paste" correctly in macro code).
Maybe annotation could define what keys are available and whether they need to be typed or just passed as AST... just throwing ideas here.
The workaround is of course fully qualifying the type name, but it's not nice.
I guess that's one area where the annotation can be configurable:
Say sth like:
annotation JSON::Field
param converter : Path, resolve_on_call: true
end
So that when I do @[JSON::Field(converter: Converter)] as in the given example in the OP, the macro will get Foo::Converter instead of Converter for the converter param.
@bew Yeah, but that's just adding more on top of the pile of complexity that the language is already. That's not a simple solution.
@asterite why not just resolve the full path of types as soon as you use them in annotations so it's always unambiguous. So @[Ann(Bar)] will always be recorded as @[::Ann(::Foo::Bar)]. That does mean either all types have to be defined before they're used in annotations, or that the compiler needs another pass.
I don't know, I guess someone will have to play with that and try.
Is this related to this particular issue?
require "json"
struct Enum
class StringConverter(T)
def self.from_json(value : JSON::PullParser) : T
T.parse(value.read_string)
end
def self.to_json(value : self, json : JSON::Builder)
json.string(value.to_s)
end
end
end
# Fails to compile
class Foo
include JSON::Serializable
enum Bar
Baz
end
# undefined constant Bar
@[JSON::Field(converter: Enum::StringConverter(Bar))]
property bar : Bar
end
# Compiles nicely
class Foo
enum Bar
Baz
end
JSON.mapping({
bar: {
type: Bar,
converter: Enum::StringConverter(Bar),
},
})
end
Foo.from_json("")
With Serializable: https://carc.in/#/r/5dk7
With JSON.mapping: https://carc.in/#/r/5dk8
Yes, more or less. As a workaround use the fully qualified name.
Here's another broken JSON example, but it doesn't use the JSON::Field annotation. Does this have the same problem, or is it a different problem that I should file a new issue ticket for?
require "json"
module Namespace
struct Foo
include JSON::Serializable
@bar : Bar = \
Bar.new # ERROR: undefined constant Bar
def initialize(@bar = Bar.new)
end
end
end
module Namespace # comment this out, and the example compiles
struct Bar
include JSON::Serializable
@s : String = ""
def initialize(@s = "s")
end
end
end
Namespace::Foo.from_json("{}")
@jemc It's a related issue. The problem is that JSON::Serializable works by defining an initialize method in JSON::Serializable so Bar.new isn't found relative to that location. One way to solve this could be to move the initialize method to the type that includes JSON::Serializable (using an included macro hook) but the problem wouldn't be entirely solved because it could then break for subclasses (unless we keep copying the initialize method somehow).
I don't know yet what the solution to this problem is.
Maybe JSON.mapping wasn't that bad after all :-)
How about use @type.constant? It can lookup the type with relative path.
annotation Ann
end
module UsingAnn
def method
{% for ivar in @type.instance_vars %}
{{ @type.constant(ivar.annotation(::Ann)[0].id) }}
{% end %}
end
end
class Foo
include UsingAnn
@[Ann(Bar)]
getter foo : String?
module Bar
end
end
pp Foo.new.method
While @firejox's solution works in this example, it breaks as soon as someone decides to use the fully qualified name (it will look for Foo::Foo::Bar in this example). This would essentially break existing codebases circumventing this issue using the fully qualified name.
Although i appreciate how horrible the following solution is, i would like to add it in case someone finds it useful (i sincerely hope not)
annotation Ann
end
macro resolve_first(*paths)
{{ (paths.find(&.resolve?) || paths.first).resolve }}
end
macro find_const(const)
{% path_fragments = [""] + @type.name.split("::") %}
{% paths = path_fragments.map_with_index { |_, i| path_fragments[0, path_fragments.size - i].join("::").id } %}
resolve_first({{ paths.map { |path| "#{path}::#{const}".id }.splat }})
end
module UsingAnn
def method
{% for ivar in @type.instance_vars %}
pp find_const({{ ivar.annotation(::Ann)[0].id }})
{% end %}
end
end
class Foo
include UsingAnn
@[Ann(Bar)]
getter foo : String?
@[Ann(Foo::Bar)]
getter foobar : String?
module Bar
end
end
Foo.new.method
Output:
Foo::Bar
Foo::Bar
Most helpful comment
While @firejox's solution works in this example, it breaks as soon as someone decides to use the fully qualified name (it will look for Foo::Foo::Bar in this example). This would essentially break existing codebases circumventing this issue using the fully qualified name.
Although i appreciate how horrible the following solution is, i would like to add it in case someone finds it useful (i sincerely hope not)
Output: