iancoleman / orderedmap

orderedmap is a golang map where the keys keep the order that they're added. It can be de/serialized from/to JSON. It's based closely on the python collections.OrderedDict.
MIT License
360 stars 55 forks source link

add SetJSONLastValueWins allowing JSON unmarshal to error on duplicate instead of last-value-wins #35

Open zamicol opened 1 year ago

iancoleman commented 1 year ago

Thanks for this, looks good. The reason and benefit for this PR is clear (thanks for the excellent comments in the code).

The main hesitation I have with this is surprising behavior compared to the standard encoding/json package.

One of the goals is for OrderedMap to be as close to a drop-in replacement for map as possible, and this would change that expectation.

b := []byte(`{"x":1,"x":2}`)

// normal map behavior
m := map[string]int{}
err := json.Unmarshal(b, &m) // no error

// ordered map behavior
o := New()
err = json.Unmarshal(b, &o) // no error, but would have error if this PR is merged

I tried to see if json v2 in https://github.com/go-json-experiment/json was likely to be incorporated into golang standard libraries but couldn't find any info about it. I'm not sure if deviating from the standard map is desired at this time. Is there some more info about 'the planned revision' in the comment "Until Go releases the planned revision to the JSON package"? Is this planned to be added to encoding/json in the future?

One alternative option might be changing jsonLastValueWins to jsonv2 and only throwing an error if this is true (ie existing behavior is retained, new behavior is opt-in).

zamicol commented 1 year ago

For a drop-in replacement, I believe the easy fix is to flip the default value of jsonLastValueWins to true so the default behavior matches the existing behavior. So Go's default value of false can be used, jsonLastValueWins can be renamed to jsonErrorOnDuplicate.

"Until Go releases the planned revision to the JSON package"?

Yes, the Go team has acknowledge that there are many concerns with the existing implementation of JSON, and that it's deserving of a revision. mvdan has confirmed this, and was more explicit on a personal email. The abstract "JSONv2" will include many changes from the existing implementation but there's no firm plans to get this done any time soon.

Edit:

Looks like Go is planning on DisallowDuplicateFields https://github.com/golang/go/issues/48298

For other JSON problems, this is a good link: https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+encoding%2Fjson+in%3Atitle