encoding/json: Map key marshals with MarshalText but unmarshals with UnmarshalJSON

[bmoylan]

What version of Go are you using (go version)?

$ go version
go version go1.11.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

playgound: https://play.golang.org/p/4g3sHaRjh7z

1. Declare a struct type that implements all of MarshalJSON, UnmarshalJSON, MarshalText, UnmarshalText, where the JSON form is an object, not string.
2. Use that type as a map key in a map.
3. json.Marshal the map. Notice the MarshalText behavior is used to convert the map keys to strings in the resulting JSON object.
4. json.Unmarshal the JSON bytes from (3) into a new map.

What did you expect to see?

json.Unmarshal should use the map key's UnmarshalText method, because it is forced to be a JSON string.

What did you see instead?

json.Unmarshal used the type's UnmarshalJSON method on the map key data.

Debugging

TextMarshaler map keys were introduced in ffbd31e. The core issue is that literalStore always prefers UnmarshalJSON (decode.go#L855) before attempting UnmarshalText because indirect only returns one of the interfaces and prefers JSON to Text. This is correct in most cases, but for map keys we're calling literalStore explicitly because the value implements UnmarshalText (decode.go#L776).

[mvdan]

Perhaps a duplicate of #23455?

[bmoylan]

Thanks for the link, @mvdan. That issue seems to be describing a type which implements UnmarshalJSON but not UnmarshalText. In my case, my type implements both interfaces. Decode correctly checks if the map key type implements UnmarshalText, but if yes it eventually attempts UnmarshalJSON instead (if both are implemented).

[bcmills]

CC @rsc @dsnet @bradfitz for encoding/json

[bcmills]

From the current documentation for encoding/json:

Map values encode as JSON objects. The map's key type must either be a string, an integer type, or implement encoding.TextMarshaler.

Unmarshal […] stores key-value pairs from the JSON object into the map. The map's key type must either be a string, an integer, or implement encoding.TextUnmarshaler.

Note that in your example, the map's key type itself does not implement encoding.TextUnmarshaler: the pointer to that key type does. However, I think that's just a documentation issue: I don't see any plausible way that a value type that is sensible as a map key could also implement TextUnmarshaler.

[bcmills]

At any rate, removing the UnmarshalJSON method fixes the linked example, so there is clearly an asymmetry here that should be fixed.