Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/json: unexpected json.Unmarshal behavior with maps #12972

Closed
nsoufr opened this issue Oct 17, 2015 · 9 comments
Closed

encoding/json: unexpected json.Unmarshal behavior with maps #12972

nsoufr opened this issue Oct 17, 2015 · 9 comments

Comments

@nsoufr
Copy link

nsoufr commented Oct 17, 2015

http://golang.org/src/encoding/json/decode.go?s=2621:2669#L64

To unmarshal a JSON object into a map, Unmarshal replaces the map with an empty map and then adds key-value pairs from the object to the map.

But, while unmarshalling it behaves differently.

http://play.golang.org/p/HTjkkCEQBV

var r map[string]string
j := []byte(`{"hello": "world"}`)

json.Unmarshal(j, &r)
fmt.Println(r)
// map[string]string{"hello":"world"}

ja := []byte(`{"world": "hello"}`)
json.Unmarshal(j, &r)
fmt.Println(r)
// map[string]string{"hello":"world", "world":"hello"}
// expected to eq map[string]string{"world": "hello"}

I'm totally in favor of this behavior but what docs is saying isn't correct.

That make sense?

@dominikh
Copy link
Member

749b391 added said documentation and a test for the behaviour. Unfortunately, the test itself is broken, too. https://github.com/golang/go/blob/master/src/encoding/json/decode_test.go#L313 is supposed to test the overwriting of the map. However, in https://github.com/golang/go/blob/master/src/encoding/json/decode_test.go#L536 the initial value gets thrown away and replaced with an empty map. That way, the test doesn't actually test the overwriting of the map.

@dominikh
Copy link
Member

Furthermore, this doesn't look like a regression. It does still overwrite slices; and Go 1.4.2 didn't overwrite maps, either.

@patricksuo
Copy link

http://play.golang.org/p/HTjkkCEQBV
Unmarshal resets slice, but doesn't rest map. I think they should have the same behavior.

https://github.com/golang/go/blob/release-branch.go1.5/src/encoding/json/decode.go#L521

@nodirt
Copy link
Contributor

nodirt commented Oct 17, 2015

@gopherbot
Copy link

CL https://golang.org/cl/16020 mentions this issue.

@rakyll rakyll changed the title Unexpected json.Unmarshal behavior with maps encoding/json: unexpected json.Unmarshal behavior with maps Oct 19, 2015
@rakyll
Copy link
Contributor

rakyll commented Oct 19, 2015

/cc @rsc

@rsc
Copy link
Contributor

rsc commented Oct 23, 2015

It's entirely possible we should change the docs instead of changing the implementation.

@rsc rsc added this to the Go1.6 milestone Oct 23, 2015
@rsc rsc added the Thinking label Oct 23, 2015
@james-lawrence
Copy link
Contributor

seems like not changing one of the implementations (slices or maps) leads to inconsistency in the api. if that matters at all in your decision.

@gopherbot
Copy link

CL https://golang.org/cl/17230 mentions this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants