go icon indicating copy to clipboard operation
go copied to clipboard

Support reporting errors/warnings when detecting duplicate fields

Open luxas opened this issue 2 years ago • 1 comments

Related to #570

As can be read in YAML 1.2 spec :

JSON's RFC4627 requires that mappings keys merely “SHOULD” be unique, while YAML insists they “MUST” be. Technically, YAML therefore complies with the JSON spec, choosing to treat duplicates as an error. In practice, since JSON is silent on the semantics of such duplicates, the only portable JSON files are those with unique keys, which are therefore valid YAML files.

As the recommendation of JSON specifies that fields SHOULD be unique; it would be nice to have an option to enforce this; especially for consistency between JSON and YAML parsers (e.g. both would return errors on duplicate fields consistently).

yaml.v3 errors out if there are duplicate fields. It can be very non-obvious how parsing is done if duplicate fields don't yield an error, e.g. encoding/json where duplicate fields with object values are merged, whereas all other types are replaced with the latest value (ref: https://github.com/golang/go/issues/24415#issuecomment-373513285):

The documentation doesn't specify what happens in this case; it says it matches object keys to fields but doesn't specify in what order it does so (it only matters with duplicate keys). Changing the current behaviour might break backwards compatibility too much, but I think at least it should be documented.

Also, the behaviour is inconsistent between a map and a slice field; the maps get merged, the slice field takes the last value seen, like the other types.

Here is that comment behavior in action: https://play.golang.org/p/CkCMdoGXc07 This was unexpected to me, and I believe most users, too.

I have an implementation of this fix, providing an opt-in method to enable errors on duplicate fields: https://github.com/luxas/json-iterator/tree/error_for_duplicates

But, I think it should be possible to write this as a json-iter extension as well; I don't know which one is better for the community in the long run (i.e. feature creep vs extensibility).

If json-iter supported "warnings", it'd also be possible to begin the transition to duplicate fields erroring by showing the user warnings (in the spirit of https://kubernetes.io/blog/2020/09/03/warnings/)

luxas avatar Sep 02 '21 09:09 luxas

Go also has an open issue for this: https://github.com/golang/go/issues/48298. dsnet wrote a great summary of the problem and I'd suggest a read.

As far as implementation, this looks like it might require another bool on generalStructDecoder:

https://github.com/json-iterator/go/blob/e6b9536d3649bda3e8842bb7e4fab489d79a97ea/reflect_struct_decoder.go#L493

zamicol avatar Jul 25 '22 18:07 zamicol