code review 13180043: encoding/json: add "overflow" struct tag option

encoding/json: add "overflow" struct tag option

Fixes issue 6213.

[adg, 2013-08-28 03:33:58]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[dsymonds, 2013-08-28 03:45:12]

On 28 August 2013 13:33,  <adg@golang.org> wrote:

> encoding/json: add "overflow" function

Might be clearer as `encoding/json: add "overflow" struct tag option`.

[adg, 2013-08-28 03:46:26]

Hello golang-dev@googlegroups.com, dsymonds@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2013-08-28 17:21:31]

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/deco...
File src/pkg/encoding/json/decode_test.go (right):

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/deco...
src/pkg/encoding/json/decode_test.go:1291: if dest.A != 1 {
or just one reflect.DeepEqual

type S struct {
    A int
    E map[string]interface{} `json:",overflow"`
    C int
}
var dest S
var want = S{1, map[string]interface{"B": float64(2)}, 3}
if !reflect.DeepEqual(dest, want) {
    t.Errorf("Got %+v; want %+v", dest, want)
}

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
File src/pkg/encoding/json/encode.go (right):

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode.go:1076: fields = append(fields, field{name,
tagged, index, ft,
too many bools in a row.  subject to bugs if the field struct fields are
rearranged.  name the fields here.

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
File src/pkg/encoding/json/encode_test.go (right):

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode_test.go:418: `{"A":12,"B":42}`,
If there is also a "C", does "B" sort before "C"?  Or are all the extras at the
end?

[adg, 2013-08-29 04:35:12]

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/deco...
File src/pkg/encoding/json/decode_test.go (right):

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/deco...
src/pkg/encoding/json/decode_test.go:1291: if dest.A != 1 {
On 2013/08/28 17:21:32, bradfitz wrote:
> or just one reflect.DeepEqual
> 
> type S struct {
>     A int
>     E map[string]interface{} `json:",overflow"`
>     C int
> }
> var dest S
> var want = S{1, map[string]interface{"B": float64(2)}, 3}
> if !reflect.DeepEqual(dest, want) {
>     t.Errorf("Got %+v; want %+v", dest, want)
> }

Done.

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
File src/pkg/encoding/json/encode.go (right):

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode.go:1076: fields = append(fields, field{name,
tagged, index, ft,
On 2013/08/28 17:21:32, bradfitz wrote:
> too many bools in a row.  subject to bugs if the field struct fields are
> rearranged.  name the fields here.

Done.

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
File src/pkg/encoding/json/encode_test.go (right):

https://codereview.appspot.com/13180043/diff/13001/src/pkg/encoding/json/enco...
src/pkg/encoding/json/encode_test.go:418: `{"A":12,"B":42}`,
On 2013/08/28 17:21:32, bradfitz wrote:
> If there is also a "C", does "B" sort before "C"?  Or are all the extras at
the
> end?

The overflow is marshalled wherever it is in the struct, so if there were a C it
would come after B in the output. I've updated the test to reflect this.

[adg, 2013-08-29 04:40:05]

*** Submitted as https://code.google.com/p/go/source/detail?r=318540e7857f ***

encoding/json: add "overflow" struct tag option

Fixes issue 6213.

R=golang-dev, dsymonds, bradfitz
CC=golang-dev
https://codereview.appspot.com/13180043

[r, 2013-08-29 05:11:44]

LGTM

[r, 2013-08-29 05:12:17]

NOT LGTM (yet)
sorry
clicked on the wrong link in all the undo craziness