encoding/json: allow non-string map keys that implement encoding.TextMarshaler/TextUnmarshaler

[augustoroman]

Original discussion at https://groups.google.com/forum/#!searchin/golang-dev/json$20map/golang-dev/5gSHNrJQpUI/vZGSGRmUrC0J

Currently, json.Marshal will fail to marshal Go maps that have non-string keys, e.g.:

// http://play.golang.org/p/2m9wLZATqw
type Coord struct { X,Y int }
occupied := map[Coord]bool{}
occupied[Coord{1,2}] = true
data, err := json.Marshal(occupied)
fmt.Printf("Data: %s\nErr: %v", data, err)

I propose to enhance the encoding/json package such that:

1. for json.Marshal
   • If the map key is a string kind it is used directly.
   • Otherwise if the map key satisfies the encoding.TextMarshaler interface then that is used to generate the map key.
   • Otherwise it fails as it does today.
2. for json.Unmarshal
   • If the map key is a string kind it is written directly.
   • Otherwise if the map key satisfies the encoding.TextUnmarshaler interface then that is used to decode the map key.
   • Otherwise it fails as it does today.

Example

(http://play.golang.org/p/VxhFluFKTX)

This would, for example, allow a map[Coord]bool to be Marshaled & Unmarshaled if Coord was defined as:

type Coord struct{ X, Y int }
func (c Coord) MarshalText() ([]byte, error) {
    return []byte(fmt.Sprintf("%d,%d", c.X, c.Y)), nil
}
func (c *Coord) UnmarshalText(p []byte) error {
    if n, err := fmt.Sscanf(string(p), "%d,%d", &c.X, &c.Y); err != nil {
        return err
    } else if n != 2 {
        return errors.New("Cannot parse coord: " + string(p))
    }
    return nil
}

And the Go struct

map[Coord]bool{Coord{1, 2}: true, Coord{3, 4}: true}

would correspond to

{"1,2": true,"3,4":true}

Considerations

If the struct marshals to a non-unique value, e.g.

func (c Coord) MarshalText() ([]byte, error) {
    return []byte("always the same"), nil
}

The the json encoder would output JSON that has repeated keys, e.g.:

{"always the same": true,"always the same":true}

This is valid JSON.

Similarly, when decoding a map, it would unmarshal each key and value and then assign that into the map, so last-write-wins, as is currently done.

An interesting side effect is that it would then be possible to explicitly handle JSON with repeated keys (or even explicitly record the order of the json keys) by having the go-side map key TextUnmarshaler have non-deterministic unmarshaling (e.g. record timestamp for each unmarshal).

[stemar94]

I gave it a try. First hacked version (encode should be fine, but decode is ugly).
https://go-review.googlesource.com/#/c/14551/

[extemporalgenome]

The proposed "if string kind, it is used directly" behavior is the opposite of the json package's semantics; I suggest we reverse that so that it is:

• for json.Marshal: if the map key implements json.Marshaler or encoding.TextMarshaler, they are used in that order of preference; otherwise, if the key type is of string kind, the map keys are used directly.
• for json.Unmarshal: if the map key implements json.Unmarshaler or encoding.TextUnmarshaler, they are used in that order of preference; otherwise, if the key type is of string kind, the JSON object keys are stored directly.

[adg]

I think this sounds reasonable to me. Interested to hear some other
opinions.

On 12 October 2015 at 05:57, Kevin Gillette notifications@github.com
wrote:

The proposed "if string kind, it is used directly" behavior is the
opposite of the json package's semantics; I suggest we reverse that so that
it is:

• for json.Marshal: if the map key implements json.Marshaler or
  encoding.TextMarshaler, they are used in that order of preference;
  otherwise, if the key type is of string kind, the map values are used
  directly.
• for json.Unmarshal: if the map key implements json.Unmarshaler or
  encoding.TextUnmarshaler, they are used in that order of preference;
  otherwise, if the key type is of string kind, the JSON object keys are
  stored directly.

—
Reply to this email directly or view it on GitHub
#12146 (comment).

[stemar94]

The proposed "if string kind, it is used directly" behavior is the opposite of the json package's semantics; I suggest we reverse that so that it is...

• @extemporalgenome

I would agree with that, but what about the go1 compatibility promise?

Rest is fine with me.

[extemporalgenome]

@stemar94 I don't see how the compat guarantee applies here. As far as I
can tell, this proposal hasn't been merged into any release yet.
On Oct 13, 2015 4:33 AM, "stemar94" notifications@github.com wrote:

The proposed "if string kind, it is used directly" behavior is the
opposite of the json package's semantics; I suggest we reverse that so that
it is...

• @extemporalgenome https://github.com/extemporalgenome

I would agree with that, but what about the go1 compatibility promise?

Rest is fine with me.

—
Reply to this email directly or view it on GitHub
#12146 (comment).

[augustoroman]

@extemporalgenome Existing keys that have string kind but are also TextMarshalers would change behavior and possibly fail (depending on the marshaling)

E.g. http://play.golang.org/p/d4DBVsfeqJ

[extemporalgenome]

@augustoroman, @stemar94 I see what you mean, and I now see why the proposal as originally specified is the way it is. Either way, it warrants good documentation.

[extemporalgenome]

Are we also accounting for implementations of json.Marshaler and json.Unmarshaler (with the caveat that such implementations would be expected to en/decode JSON strings specifically)?

[augustoroman]

No, the original proposal explicitly leaves json.Marshaler and json.Unmarshaler out. It seems ambiguous whether we would take the resulting json bytes and encode that as a string or check to see whether it corresponds to a json string, plus it seems somewhat inconsistent that we would allow some json.Marshalers but not others -- which may fail at runtime. (e.g. a given json.Marshaler could sometimes produce strings and sometimes not)

[extemporalgenome]

@augustoroman json.Marshaler specifically generates encoded json (you wouldn't wrap its output further, but rather just pass it through after validating).

The JSON package already validates the output of json.Marshaler implementations:
http://play.golang.org/p/gLlXpswvV_

In this case, the validation would just ensure that it's a string, but otherwise pass it through.

I do agree that the "some but not others" point does make it less than desirable, but for the same reason the proposal uses string kinds directly (compatibility), if we're ever going to allow json.Marshaler's and json.Unmarshaler's here, we should do so now, since we'd have to completely invert the normal precedence to maintain compatibility if we add it later (string -> encoding.TextMarshaler -> json.Marshaler).