Added JSON marshal/unmarshal

[beatgammit]

TL;DR: adds marshalJSON and unmarshalJSON functions to std.json to mimic Go's json library.

I've written what I feel are in-depth unit tests, but I'm sure I've missed some cases. All unit tests in phobos passed on my machine (Linux x86_64).

I noticed that orange is on the list of new standard library modules to be reviewed. I intend it to be used for simple data passing purposes, not full serialization. A separate JSON archiver should be implemented if orange is added.

There are some restrictions on what can and cannot be marshalled/unmarshalled in this implementation. I wasn't sure how to document those.

EDIT:

I added in whitespace changes too, because this is the only file that uses 8 space indenting, and the D style guide says to use 4 space indenting (which I completely agree with).

[donc]

What happens if you do marshallJson(double.nan)?

BTW: lack of any support for NaN and Inf is a problem with the existing std.json. Many JSON libraries, including Google's, will produce "NaN" or "Inf" even though it technically is not part of the JSON spec. We should have an option to allow it, too. But we definitely shouldn't be producing JSON that we cannot parse.

[jmdavis]

But we definitely shouldn't be producing JSON that we cannot parse.

Not only would not being able to parse what we produce be a bad idea, but it would be a sign of insufficient unit testing if it weren't caught.

[beatgammit]

@donc I've read some posts on the d forums about people complaining about the json library. Is there anything else you can think of that needs to be changed? While I'm in there, I could go ahead and fix a few things.

I didn't think about double.nan (shame on me). Since json doesn't define infinity, -infinity or NaN in the spec, then I agree that it would have to be an option, but not the default.

I'll make some changes this weekend and add a commit to address this.

How do you feel about making all of them null by default. That's what CouchDB does.
http://smartcode.simplewind.com/2011/10/null-nan-infinity-in-json-couchdb.html

The override would create JSON that is incompatible with the JSON spec, but would be able to represent all values in the IEEE floating point spec.

Note, from the spec:

Numeric values that cannot be represented as sequences of digits
(such as Infinity and NaN) are not permitted.

http://www.ietf.org/rfc/rfc4627.txt

[donc]

I think that by default, values which cannot be represented within the JSON spec should produce errors, rather than either producing non-standard JSON or different values to what was provided. AFAIK if the JSON was produced by Javascript itself, it WILL contain NaN or Inf, so the standard is in error. But it is still the standard.
The treatment of NaN and Infinity seems seems to be a "Rebel Standard" which we should also support, but probably not by default. Certainly not for writing.

[beatgammit]

By default, Python includes Infinity, -Infinity and NaN, but no JavaScript engine does (substitutes null):

JSON.stringify({inf: Infinity, neginf: -Infinity, nan: NaN}) -> '{"inf":null,"neginf":null,"nan":null}'

I tested on the following:

• Google Chrome
• Firefox
• Opera
• node.js

I haven't tested IE or Safari because I'm running Linux, but I have the hunch that it would yield the same results.

Go pukes with this (similar for -Inf and NaN): json: unsupported value: +Inf

I propose adding two modes to the parser: strict and permissive.

Strict mode would throw exceptions immediately when Infinity or NaN is encountered, whereas permissive would allow it. By default, the library would throw exceptions after parsing/writing is done, replacing Infinity/NaN on reads and replacing such with nulls on writes.

Does this sound sensible?

[andralex]

Not sure how to go about this as it adds on top of a questionable design which is also implemented in a style removed from the rest of Phobos. Should we merge this in on the assumption that a major overhaul is due, or stop building on a bad foundation?

[braddr]

If the new api is likely to survive a re-implementation of the base it sits on, then maybe it's ok. But if it's likely to also need to change, then it's probably not ok. I'm skeptical that it'd survive, but that's not based on reviewing the code, just experience.

[jpf91]

@braddr It probably won't survive. In a perfect world the marshal/unmarshal functions would be part of a generic serialization framework which should support different backends (json, xml, yaml, binary encodings...).

[beatgammit]

In hindsight, integrating with something like orange would probably be a better option, but I'm not particularly fond of the OO style. I would rather see something like:

marshalJSON!(foo, outputStream)
unmarshalJSON!(inputStream, foo)

Instead of:

auto archive = new JSONArchive!(char); // create a JSON archive
auto serializer = new Serializer(archive); // create the serializer

serializer.serialize(foo); // serialize "foo"

I think I'll change the function signatures for marshal/unmarshal to the above signature, which should make writing a wrapper around orange easy if/when that gets integrated into Phobos. I'll also make the other internal functions private, so only a single marshal/unmarshal function is exposed. This should make the existing code easy to deprecate once a serialization library is implemented.

I think this new signature will make dropping in a new base easier. I'll minimize the exposed API and not use the old JSON_VALUE struct in any public-facing API. This way, the old API can be deprecated, and the new serialization library can be dropped in, all without changing the marshal/unmarshal signature.

[beatgammit]

I've updated the function signatures as specified and hid all others. Tests still pass on my machine (Linux x86_64).

I've hidden the maxDepth parameter to ensure forward compatibility.

[ghost]

I think we should try and pull #1421 first (after some issues with it are resolved).

As for Orange, it seems it's eventually going to get merged in. Perhaps it would be a good idea to think about adding a back-end to Orange that uses std.json behind-the-scenes (I mean after Orange is merged).

Chances are Orange might use UDAs to mark user-defined types/fields with various attributes (we didn't have UDAs back when your pull was made AFAIR), which means it's better to reuse these definitions rather than have marshallJson/etc have to manually inspect this. Orange could theoretically deal with these attributes itself, and then call the back-end json-specific serializer.

Since this pull has been sitting for a long time here I guess it wouldn't be a big problem to wait a little longer until we get a serialization framework in place?

[beatgammit]

This pull request is basically dead now. I'm not going to be rebasing this against master anytime soon, especially since it will likely never get merged.