code review 5656102: encoding/json: disable anonymous fields

encoding/json: disable anonymous fields

We should, after Go 1, make them work the same as
package xml, that is, make them appear in the outer
struct.  For now turn them off so that people do not
depend on the old behavior.

Fixing them is issue 3069.

[rsc, 2012-02-19 05:15:05]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2012-02-19 05:17:30]

LGTM

On Sat, Feb 18, 2012 at 9:15 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> encoding/json: disable anonymous fields
>
> We should, after Go 1, make them work the same as
> package xml, that is, make them appear in the outer
> struct.  For now turn them off so that people do not
> depend on the old behavior.
>
> Fixing them is issue 3069.
>
> Please review this at
http://codereview.appspot.com/**5656102/<http://codereview.appspot.com/5656102/>
>
> Affected files:
>  M src/pkg/encoding/json/decode.**go
>  M src/pkg/encoding/json/decode_**test.go
>  M src/pkg/encoding/json/encode.**go
>
>
> Index: src/pkg/encoding/json/decode.**go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/json/**decode.go
> +++ b/src/pkg/encoding/json/**decode.go
> @@ -496,6 +496,12 @@
>                                        // Pretend this field doesn't exist.
>                                        continue
>                                }
> +                               if sf.Anonymous {
> +                                       // Pretend this field doesn't
> exist,
> +                                       // so that we can do a good job
> with
> +                                       // these in a later version.
> +                                       continue
> +                               }
>                                // First, tag match
>                                tagName, _ := parseTag(tag)
>                                if tagName == key {
> @@ -963,3 +969,11 @@
>        }
>        return b[0:w], true
>  }
> +
> +// The following is issue 3069.
> +
> +// BUG(rsc): This package ignores anonymous (embedded) struct fields
> +// during encoding and decoding.  A future version may assign meaning
> +// to them.  To force an anonymous field to be ignored in all future
> +// versions of this package, use an explicit `json:"-"` tag in the struct
> +// definition.
> Index: src/pkg/encoding/json/decode_**test.go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/json/**decode_test.go
> +++ b/src/pkg/encoding/json/**decode_test.go
> @@ -619,3 +619,32 @@
>                t.Errorf("got %+v, want %+v", got, want)
>        }
>  }
> +
> +// Test that anonymous fields are ignored.
> +// We may assign meaning to them later.
> +func TestAnonymous(t *testing.T) {
> +       type S struct {
> +               T
> +               N int
> +       }
> +
> +       data, err := Marshal(new(S))
> +       if err != nil {
> +               t.Fatalf("Marshal: %v", err)
> +       }
> +       want := `{"N":0}`
> +       if string(data) != want {
> +               t.Fatalf("Marshal = %#q, want %#q", string(data), want)
> +       }
> +
> +       var s S
> +       if err := Unmarshal([]byte(`{"T": 1, "T": {"Y": 1}, "N": 2}`),
> &s); err != nil {
> +               t.Fatalf("Unmarshal: %v", err)
> +       }
> +       if s.N != 2 {
> +               t.Fatal("Unmarshal: did not set N")
> +       }
> +       if s.T.Y != 0 {
> +               t.Fatal("Unmarshal: did set T.Y")
> +       }
> +}
> Index: src/pkg/encoding/json/encode.**go
> ==============================**==============================**=======
> --- a/src/pkg/encoding/json/**encode.go
> +++ b/src/pkg/encoding/json/**encode.go
> @@ -538,6 +538,11 @@
>                if f.PkgPath != "" {
>                        continue
>                }
> +               if f.Anonymous {
> +                       // We want to do a better job with these later,
> +                       // so for now pretend they don't exist.
> +                       continue
> +               }
>                var ef encodeField
>                ef.i = i
>                ef.tag = f.Name
>
>
>

[rsc, 2012-02-19 05:27:09]

*** Submitted as af5a7f6d962d ***

encoding/json: disable anonymous fields

We should, after Go 1, make them work the same as
package xml, that is, make them appear in the outer
struct.  For now turn them off so that people do not
depend on the old behavior.

Fixing them is issue 3069.

R=golang-dev, bradfitz
CC=golang-dev
http://codereview.appspot.com/5656102