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.

Sign up for GitHub

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

Jump to bottom

Fix JSON.dump overload combination #558

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Fix JSON.dump overload combination #558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions lib/json/common.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,16 +612,13 @@ class << self
# Output:
# {"foo":[0,1],"bar":{"baz":2,"bat":3},"bam":"bad"}
def dump(obj, anIO = nil, limit = nil, kwargs = nil)
if anIO and limit.nil?
anIO = anIO.to_io if anIO.respond_to?(:to_io)
unless anIO.respond_to?(:write)
if kwargs.nil? and anIO.is_a?(Hash)
kwargs = anIO
else
limit = anIO
end
anIO = nil
end
io_limit_opt = [anIO, limit, kwargs].compact
kwargs = io_limit_opt.pop if io_limit_opt.last.is_a?(Hash)
anIO, limit = io_limit_opt
if anIO.respond_to?(:to_io)
anIO = anIO.to_io
elsif limit.nil? && !anIO.respond_to?(:write)
anIO, limit = nil, anIO
Comment on lines +618 to +621
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the inconsistency

dump(obj, io, nil) #=> io.to_io was called
dump(obj, io, 10) #=> io.to_io was not called

And if to_io is called, the result of to_io should be an IO, no need to check for respond_to?(:write)

end
opts = JSON.dump_default_options
opts = opts.merge(:max_nesting => limit) if limit
Expand All @@ -642,12 +639,14 @@ def self.iconv(to, from, string)
string.encode(to, from)
end

private
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge_dump_options should be private instance method and also private module function

module_function

def a_private_instance_method_and_public_module_function
end

private # module_function rule is cleared

def a_private_instance_method # not a module function
end


def merge_dump_options(opts, strict: NOT_SET)
opts = opts.merge(strict: strict) if NOT_SET != strict
opts
end

class << self
private :merge_dump_options
end
end

module ::Kernel
Expand Down
32 changes: 20 additions & 12 deletions tests/json_common_interface_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,26 @@ def test_load_null

def test_dump
too_deep = '[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]'
assert_equal too_deep, dump(eval(too_deep))
assert_kind_of String, Marshal.dump(eval(too_deep))
assert_raise(ArgumentError) { dump(eval(too_deep), 100) }
assert_raise(ArgumentError) { Marshal.dump(eval(too_deep), 100) }
assert_equal too_deep, dump(eval(too_deep), 101)
assert_kind_of String, Marshal.dump(eval(too_deep), 101)
output = StringIO.new
dump(eval(too_deep), output)
assert_equal too_deep, output.string
output = StringIO.new
dump(eval(too_deep), output, 101)
assert_equal too_deep, output.string
obj = eval(too_deep)
assert_equal too_deep, dump(obj)
assert_kind_of String, Marshal.dump(obj)
assert_raise(ArgumentError) { dump(obj, 100) }
assert_raise(ArgumentError) { Marshal.dump(obj, 100) }
assert_equal too_deep, dump(obj, 101)
assert_kind_of String, Marshal.dump(obj, 101)

assert_equal too_deep, JSON.dump(obj, StringIO.new, 101, strict: false).string
assert_equal too_deep, dump(obj, StringIO.new, 101, strict: false).string
assert_raise(JSON::GeneratorError) { JSON.dump(Object.new, StringIO.new, 101, strict: true).string }
assert_raise(JSON::GeneratorError) { dump(Object.new, StringIO.new, 101, strict: true).string }

assert_equal too_deep, dump(obj, nil, nil, strict: false)
assert_equal too_deep, dump(obj, nil, 101, strict: false)
assert_equal too_deep, dump(obj, StringIO.new, nil, strict: false).string
assert_equal too_deep, dump(obj, nil, strict: false)
assert_equal too_deep, dump(obj, 101, strict: false)
assert_equal too_deep, dump(obj, StringIO.new, strict: false).string
assert_equal too_deep, dump(obj, strict: false)
end

def test_dump_should_modify_defaults
Expand Down