System: Ubuntu WSL, Crystal v0.24.2
JSON#mapping doesn't specify the restriction on getter methods in the macro.
This causes an issue with macros where the Return Type cannot be inferred from a method call.
require "json"
macro method_return(method_name)
puts "{{method_name}} {{My_Class.methods.find { |method| method.name == method_name}.return_type}}"
end
class My_Class
JSON.mapping(mapped: Bool)
end
method_return(:mapped)
Output:
:mapped
I have a working solution here which just replaces #L83 with
def {{key.id}} : {{value[:type]}} {{ (value[:nilable] ? "?" : "").id }
to include the type. Will just wait for approval before Specs & PR
This could also be approached by changing #return_type to be able to infer which would include some more situations as shown below.
Extra code that demonstrates current behaviour:
require "json"
macro method_return(method_name)
puts "{{method_name}} {{My_Class.methods.find { |method| method.name == method_name}.return_type}}"
end
class My_Class
JSON.mapping(mapped: Bool)
def restricted : Int64
end
def inferrable
"string"
end
def not_inferrable
end
end
method_return(:mapped)
method_return(:restricted)
method_return(:inferrable)
method_return(:not_inferrable)
Output:
:mapped
:restricted Int64
:inferrable
:not_inferrable
I'm not sure about your use case, but adding the return type explicitly sounds like a good idea. It is known when the macro is interpreted, so there is no point in letting the global inference figure it out later. The setter method already has it, it's just missing for the getter.
It also improves the API docs when the return type is explicit.
Sorry, this is my use-case
require "json"
struct Storable(T)
JSON.mapping(
data: {type: T, converter: DataConverter}
)
def initialize(@data : T)
end
end
class My_Class
JSON.mapping(
return: My_Return_Class
)
end
class My_Return_Class
end
module DataConverter
macro json_convert(klass, **build)
def self.to_json(instance : {{klass}}, builder : JSON::Builder)
builder.object do
{% if build[:fields] %}
{% for field in build[:fields] %}
object = instance.{{field.id}}
unless object.is_a?(Nil)
builder.field({{field}}, Storable({{klass.resolve.methods.find{ |method| method.name == field}.return_type}}).new(object).to_json)
end
{% end %}
{% end %}
end
end
end
json_convert(My_Class, fields: {:return})
end
Eh, you can just let the compiler figure out the generic type argument: https://carc.in/#/r/3u0z
builder.field({{field}}, Storable.new(object).to_json)
Fixed via #5935
Most helpful comment
I'm not sure about your use case, but adding the return type explicitly sounds like a good idea. It is known when the macro is interpreted, so there is no point in letting the global inference figure it out later. The setter method already has it, it's just missing for the getter.
It also improves the API docs when the return type is explicit.