require "json"
class A
end
puts A.new.to_pretty_json
results in this rather unfriendly failure message:
in /opt/crystal/src/json/to_json.cr:15: wrong number of arguments for 'A#to_json' (given 1, expected 0)
Overloads are:
- Object#to_json()
to_json JSON::PrettyWriter.new(io, indent: indent)
In the perfect world it would have an error message that said something like "you need to define this method or call JSON.mapping"
Thanks!
What about having json define to_json as an abstract method?
require "json"
class Object
abstract def to_json(io : IO)
end
class Foo
end
puts Foo.new.to_pretty_json
Error in ./issue-3450.cr:4: abstract `def Object#to_json(io : IO)` must be implemented by Reference
abstract def to_json(io : IO)
^~~~~~~
@luislavena That can't work. An abstract method must be implemented by subclasses.
An alternative is to define a to_json(io) on Object that simply gives a better error message:
require "json"
class Object
def to_json(io)
{% raise "#{@type} doesn't define a `to_json(io)` method: either implement it or use JSON.mapping" %}
end
end
class A
end
puts A.new.to_pretty_json
Output:
Error in ./foo.cr:12: instantiating 'A#to_pretty_json()'
puts A.new.to_pretty_json
^~~~~~~~~~~~~~
in ./src/json/to_json.cr:9: instantiating 'String:Class#build()'
String.build do |str|
^~~~~
in ./src/string.cr:234: instantiating 'String::Builder:Class#build(Int32)'
String::Builder.build(capacity) do |builder|
^~~~~
in ./src/string.cr:234: instantiating 'String::Builder:Class#build(Int32)'
String::Builder.build(capacity) do |builder|
^~~~~
in ./src/json/to_json.cr:9: instantiating 'String:Class#build()'
String.build do |str|
^~~~~
in ./src/json/to_json.cr:10: instantiating 'to_pretty_json(String::Builder)'
to_pretty_json str, indent: indent
^~~~~~~~~~~~~~
in ./src/json/to_json.cr:15: instantiating 'to_json(JSON::PrettyWriter)'
to_json JSON::PrettyWriter.new(io, indent: indent)
^~~~~~~
in ./foo.cr:5: expanding macro
{% raise "#{@type} doesn't define a `to_json(io)` method: either implement it or use JSON.mapping" %}
^
in ./foo.cr:5: can't expand macro: A doesn't define a `to_json(io)` method: either implement it or use JSON.mapping
{% raise "#{@type} doesn't define a `to_json(io)` method: either implement it or use JSON.mapping" %}
^~~~~
The output is still a bit long and it has "can't expand macro", but this could be improved. I don't know if this is a good solution, though.
@asterite doh, forgot that _all_ classes would have to define it for it to work, sorry. :disappointed:
I wouldn't mind use the macro raise but perhaps the backtrace can also be collapsed?
@asterite that looks good to me, good idea. It's either that or not add to_json to the Object class but make people include it themselves, ruby style
class A
extend JSONModule
end
or
make a new method required to be called like
JSON.add_methods which adds in all the to_json methods including the abstract one.
Or provide a sensible default 'to_json" (#inspect for instance has a default).
Thanks for your help here.
I think it results in a bad API that JSON spills to_json() etc. on every object, even if there is no implementation of to_json(builder : JSON::Builder).
Maybe it could be a way to use finished macro hook to define to_json() only if to_json(builder) is defined. But i don't know if this would cause other problems and it is probably more like a hack.
Then there is also #4169 suggesting an abstract module JSON::ToJson like rdp one above. This shouldn't be too much extra-effort, because most methods will be probably be generated by JSON.mapping. For other uses it might be helpful to create a macro Object.to_json which defines to_json(builder) and includes the abstract module.
The same applies to to_yaml as well.
With #5697, the code on top results in: Error in line 6: undefined method 'to_pretty_json' for A.
With code on top in Crystal 0.31.1. it gives
$ crystal e.cr
Showing last frame. Use --error-trace for full trace.
In /usr/local/Cellar/crystal/0.31.1/src/json/to_json.cr:22:7
22 | to_json(json)
^------
Error: no overload matches 'A#to_json' with type JSON::Builder
Overloads are:
- Object#to_json(io : IO)
- Object#to_json()
Most helpful comment
@luislavena That can't work. An abstract method must be implemented by subclasses.
An alternative is to define a
to_json(io)onObjectthat simply gives a better error message:Output:
The output is still a bit long and it has "can't expand macro", but this could be improved. I don't know if this is a good solution, though.