From 068279f13befe90729ff3b5eab3c46edf0a05cc1 Mon Sep 17 00:00:00 2001 From: Thomas Pelletier Date: Thu, 7 Apr 2022 19:51:09 -0400 Subject: [PATCH] encode: respect stdlib rules for embedded structs (#747) --- README.md | 34 +++---------------------- marshaler.go | 38 ++++++++++++++++++++++------ marshaler_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 4ace049a..c391d96d 100644 --- a/README.md +++ b/README.md @@ -527,42 +527,14 @@ The new name is `Encoder.SetArraysMultiline`. The behavior should be the same. The new name is `Encoder.SetIndentSymbol`. The behavior should be the same. -#### Embedded structs are tables +#### Embedded structs behave like stdlib V1 defaults to merging embedded struct fields into the embedding struct. This behavior was unexpected because it does not follow the standard library. To avoid breaking backward compatibility, the `Encoder.PromoteAnonymous` method was added to make the encoder behave correctly. Given backward compatibility is not -a problem anymore, v2 does the right thing by default. There is no way to revert -to the old behavior, and `Encoder.PromoteAnonymous` has been removed. - -```go -type Embedded struct { - Value string `toml:"value"` -} - -type Doc struct { - Embedded -} - -d := Doc{} - -fmt.Println("v1:") -b, err := v1.Marshal(d) -fmt.Println(string(b)) - -fmt.Println("v2:") -b, err = v2.Marshal(d) -fmt.Println(string(b)) - -// Output: -// v1: -// value = "" -// -// v2: -// [Embedded] -// value = '' -``` +a problem anymore, v2 does the right thing by default: it follows the behavior +of `encoding/json`. `Encoder.PromoteAnonymous` has been removed. [nodoc]: https://github.com/pelletier/go-toml/discussions/506#discussioncomment-1526038 diff --git a/marshaler.go b/marshaler.go index 4aaff21c..5d120d47 100644 --- a/marshaler.go +++ b/marshaler.go @@ -555,16 +555,25 @@ type table struct { } func (t *table) pushKV(k string, v reflect.Value, options valueOptions) { + for _, e := range t.kvs { + if e.Key == k { + return + } + } + t.kvs = append(t.kvs, entry{Key: k, Value: v, Options: options}) } func (t *table) pushTable(k string, v reflect.Value, options valueOptions) { + for _, e := range t.tables { + if e.Key == k { + return + } + } t.tables = append(t.tables, entry{Key: k, Value: v, Options: options}) } -func (enc *Encoder) encodeStruct(b []byte, ctx encoderCtx, v reflect.Value) ([]byte, error) { - var t table - +func walkStruct(ctx encoderCtx, t *table, v reflect.Value) { // TODO: cache this typ := v.Type() for i := 0; i < typ.NumField(); i++ { @@ -575,8 +584,6 @@ func (enc *Encoder) encodeStruct(b []byte, ctx encoderCtx, v reflect.Value) ([]b continue } - k := fieldType.Name - tag := fieldType.Tag.Get("toml") // special field name to skip field @@ -584,13 +591,22 @@ func (enc *Encoder) encodeStruct(b []byte, ctx encoderCtx, v reflect.Value) ([]b continue } - name, opts := parseTag(tag) - if isValidName(name) { - k = name + k, opts := parseTag(tag) + if !isValidName(k) { + k = "" } f := v.Field(i) + if k == "" { + if fieldType.Anonymous { + walkStruct(ctx, t, f) + continue + } else { + k = fieldType.Name + } + } + if isNil(f) { continue } @@ -607,6 +623,12 @@ func (enc *Encoder) encodeStruct(b []byte, ctx encoderCtx, v reflect.Value) ([]b t.pushTable(k, f, options) } } +} + +func (enc *Encoder) encodeStruct(b []byte, ctx encoderCtx, v reflect.Value) ([]byte, error) { + var t table + + walkStruct(ctx, &t, v) return enc.encodeTable(b, ctx, t) } diff --git a/marshaler_test.go b/marshaler_test.go index ae9d1e75..3b720f6c 100644 --- a/marshaler_test.go +++ b/marshaler_test.go @@ -947,6 +947,70 @@ func TestIssue678(t *testing.T) { require.Equal(t, cfg, cfg2) } +func TestMarshalNestedAnonymousStructs(t *testing.T) { + type Embedded struct { + Value string `toml:"value" json:"value"` + Top struct { + Value string `toml:"value" json:"value"` + } `toml:"top" json:"top"` + } + + type Named struct { + Value string `toml:"value" json:"value"` + } + + var doc struct { + Embedded + Named `toml:"named" json:"named"` + Anonymous struct { + Value string `toml:"value" json:"value"` + } `toml:"anonymous" json:"anonymous"` + } + + expected := `value = '' +[top] +value = '' + +[named] +value = '' + +[anonymous] +value = '' + +` + + result, err := toml.Marshal(doc) + require.NoError(t, err) + require.Equal(t, expected, string(result)) +} + +func TestMarshalNestedAnonymousStructs_DuplicateField(t *testing.T) { + type Embedded struct { + Value string `toml:"value" json:"value"` + Top struct { + Value string `toml:"value" json:"value"` + } `toml:"top" json:"top"` + } + + var doc struct { + Value string `toml:"value" json:"value"` + Embedded + } + doc.Embedded.Value = "shadowed" + doc.Value = "shadows" + + expected := `value = 'shadows' +[top] +value = '' + +` + + result, err := toml.Marshal(doc) + require.NoError(t, err) + require.NoError(t, err) + require.Equal(t, expected, string(result)) +} + func ExampleMarshal() { type MyConfig struct { Version int