code review 5656102: encoding/json: disable anonymous fields (issue 5656102)

29 views
Skip to first unread message

r...@golang.org

unread,
Feb 19, 2012, 12:15:05 AM2/19/12
to golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@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/

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


Brad Fitzpatrick

unread,
Feb 19, 2012, 12:17:29 AM2/19/12
to r...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
LGTM

r...@golang.org

unread,
Feb 19, 2012, 12:27:09 AM2/19/12
to r...@golang.org, golan...@googlegroups.com, brad...@golang.org, re...@codereview-hr.appspotmail.com
*** 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


http://codereview.appspot.com/5656102/

Reply all
Reply to author
Forward
0 new messages