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: unmarshalling same field into two struct fields #15127

Closed
wlredeye opened this issue Apr 5, 2016 · 6 comments

Comments

Projects
None yet
5 participants
@wlredeye
Copy link

commented Apr 5, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?
    1.6
  2. What operating system and processor architecture are you using (go env)?
    linux, amd64
  3. What did you do?
    I'm trying to unmarshal the same JSON field into two struct fields.
    http://play.golang.org/p/K86eAWaVyr
  4. What did you expect to see?
    Struct fields must have JSON value or json.Unmarshal() method must return an error.
  5. What did you see instead?
    Struct fields are empty and no errors returned.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 7, 2016

Hopefully this is fixable without compatibility worries.

@bradfitz bradfitz added this to the Go1.7 milestone Apr 7, 2016

@bradfitz bradfitz added the Suggested label Apr 7, 2016

@pongad

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2016

I took a look at this. I don't have a complete answer for it, but want to document my findings here.

This behavior is caused by the "annihilation" logic shared between encoding and decoding. For encoding, this makes sense: since the two sources might have different value, we might as well not write anything. One might argue that this should be an error. I'm not sure what the rationale is, but the doc on dominantField function ( https://golang.org/src/encoding/json/encode.go#L1139 ) and a test (TestDuplicatedFieldDisappears) indicate that it's a deliberate decision.

Since encode makes a decision to not error, I think we should refrain from returning an error from the decode side. One way to solve this issue is to give decode its own version of dominantField that does "union" instead of "annihilation", setting all "conflicted" fields to the same value.

Of course, another way out is to document this behavior and keep the implementation as is. What do you think?

(Please let me know if golang-dev is a better place for this discussion.)

@pongad

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2016

ping @bradfitz . Is this too late for 1.7 freeze?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 22, 2016

Things aren't frozen yet, but this would need to have a decision and an implementation soon. I'm not familiar with any past decisions here.

/cc @rsc

@pongad

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2016

I thought a little more about this. Currently dominantField chooses tag over field name. If we make a change along the way I earlier described ("union" instead of "annihilation") we now have to answer more questions like: If we have tagged fields, do we set the fields that are untagged anyway?
Furthermore, we will probably want to document this behavior to prevent confusion in the future. It would be confusing if Marshal and Unmarshal follow different rules. Rule for Marshal is already documented, so we might as well follow it.
I am leaning towards retaining the old behavior. Will this need to wait for 1.8? I'd appreciate your thoughts. @bradfitz @rsc

@rsc

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2016

The annihilation logic is intentional. I don't think we can change this. This use case is already defined to ignore both. I apologize.

@rsc rsc closed this Apr 27, 2016

@golang golang locked and limited conversation to collaborators Apr 27, 2017

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.