michalmuskala / jason

A blazing fast JSON parser and generator in pure Elixir.
Other
1.6k stars 170 forks source link

nil key in map shouldn't be encoded to "nil" #111

Closed chulkilee closed 4 years ago

chulkilee commented 4 years ago

Found that Jason uses "nil" for nil key in map. It doesn't seem like there is a "correct" way though.. However nil doesn't seem right, since that is Elixir's notation (which is actually :nil atom..).

Just opening a discussion :)

Jason.encode!(%{nil => nil}) |> IO.puts()
# {"nil":null}
require 'json'
puts({nil => nil}.to_json)
# {"":null}
import json
print(json.dumps({None: None}))
# {"null": null}
console.log(JSON.stringify({null: null}))
// {"null":null}
michalmuskala commented 4 years ago

In general I would say this is undefined behaviour in the JSON specs. I'm not sure there's a very strong reason to change - it looks like other languages are not particularly consistent either. Given this would be a breaking change I'm probably going to side with not changing anything around this.

chulkilee commented 4 years ago

Yeah I agree that 1) this is breaking change and 2) this is undefined behavior and 3) I'm not sure there is de-facto way for this.

However, I'm still wondering how it ends up with "nil", since I thought Jason expects String.Chars protocol implemented, and if I call String.Chars.to_string/1 on nil, it returns empty string ("") not "nil". This seems like... inconsistent inside jason.

String.Chars.to_string(nil)
""

String.Chars.to_string(true)
"true"
michalmuskala commented 4 years ago

It's an oversight on my part. I inlined some conversions (including atoms) to avoid protocol dispatch overhead and did not handle this correctly.