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

Add linting for to_json #6378

Closed
allcentury opened this issue Oct 11, 2018 · 3 comments
Closed

Add linting for to_json #6378

allcentury opened this issue Oct 11, 2018 · 3 comments

Comments

@allcentury
Copy link
Contributor

Is your feature request related to a problem? Please describe.

classes that override to_json need to ensure an optional parameter is
available.

class MyClass
  def to_json
  end
end

While calling MyClass.new.to_json works, calling
JSON.generate(MyClass.new) results in an ArgumentError. That's because the ruby JSON api wants to send additional options (via a hash) to to_json, ie to_json(opts = {})

Describe the solution you'd like

I'd like a linter to catch this as it's not obvious from the ruby JSON docs.

Describe alternatives you've considered

@allcentury allcentury mentioned this issue Oct 11, 2018
8 tasks
@bquorning
Copy link
Contributor

That's because the ruby JSON api wants to send additional options (via a hash) to to_json, ie to_json(opts = {})

Are you sure the argument is a Hash?

require 'json'
class MyClass
  def to_json(*args)
    p args
    ""
  end
end
JSON.generate(MyClass.new)
#=> [#<JSON::Ext::Generator::State:0x00007faacb155a40>]

@allcentury
Copy link
Contributor Author

@bquorning thanks for calling that out - it currently returns a hash-like object:

https://ruby-doc.org/stdlib-2.0.0/libdoc/json/rdoc/JSON.html#method-i-generate

from the docs:

Generate a JSON document from the Ruby data structure obj and return it. state is

a JSON::State object,

or a Hash like object (responding to to_hash),

or an object convertible into a hash by a to_h method, that is used as or to configure a State object.

Using a default of {} felt sane, because the api of State acts like a hash:

[6] pry(#<MyClass>)> args
=> #<JSON::Ext::Generator::State:0x00007fc4959e2af8>
[7] pry(#<MyClass>)> args[:indent]
=> " "
[8] pry(#<MyClass>)> args.to_hash
=> {:indent=>" ", :space=>"", :space_before=>"", :object_nl=>"", :array_nl=>"", :allow_nan=>false, :ascii_only=>false, :max_nesting=>100, :depth=>0, :buffer_initial_length=>1024}

that said perhaps just an argument with no default is enough, ie:

def to_json(_opts)
end

What do you think?

@bquorning
Copy link
Contributor

Weird then, that the signature for e.g. Date#to_json says it accepts *args.

I found this old commit on Ruby where the documentation for to_json signature on TrueClass, FalseClass and NilClass is changed from state=nil to *: ruby/ruby@e662770

Perhaps we should just recommend having * in the signature?

allcentury pushed a commit to allcentury/rubocop that referenced this issue Dec 14, 2018
This fixes issue rubocop#6378

classes that override to_json need to ensure an optional argument is
available.

class MyClass
  def to_json
  end
end

While calling MyClass.new.to_json works, calling
JSON.generate(MyClass.new) results in an ArgumentError.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants