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
nandosousafr opened this issue Oct 17, 2015 · 9 comments

Comments

Projects
None yet
8 participants
@nandosousafr
Copy link

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

This comment has been minimized.

Copy link
Member

commented Oct 17, 2015

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

This comment has been minimized.

Copy link
Member

commented Oct 17, 2015

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

@sillyousu

This comment has been minimized.

Copy link

commented Oct 17, 2015

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

This comment has been minimized.

Copy link
Member

commented Oct 17, 2015

@gopherbot

This comment has been minimized.

Copy link

commented Oct 17, 2015

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

This comment has been minimized.

Copy link
Member

commented Oct 19, 2015

/cc @rsc

@rsc

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2015

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

This comment has been minimized.

Copy link

commented Nov 25, 2015

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

@rsc rsc closed this in 3e6529d Dec 3, 2015

@golang golang locked and limited conversation to collaborators Dec 14, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.