Crystal: Object.to_json gives unhelpful error message if no to_json defined

Created on 21 Oct 2016  路  7Comments  路  Source: crystal-lang/crystal

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!

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) 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.

All 7 comments

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()
Was this page helpful?
0 / 5 - 0 ratings

Related issues

lbguilherme picture lbguilherme  路  3Comments

costajob picture costajob  路  3Comments

Papierkorb picture Papierkorb  路  3Comments

jhass picture jhass  路  3Comments

nabeelomer picture nabeelomer  路  3Comments