From d138d7a2b42ee330000a159838d589875ae49121 Mon Sep 17 00:00:00 2001 From: kkHAIKE Date: Thu, 16 Jun 2022 11:26:23 +0800 Subject: [PATCH 1/5] change eindirect behave match with indirect from decode --- encode.go | 101 +++++++++++++++++++++++++----------------------------- 1 file changed, 47 insertions(+), 54 deletions(-) diff --git a/encode.go b/encode.go index ee459765..bfb76744 100644 --- a/encode.go +++ b/encode.go @@ -318,7 +318,7 @@ func (enc *Encoder) eArrayOrSliceElement(rv reflect.Value) { length := rv.Len() enc.wf("[") for i := 0; i < length; i++ { - elem := rv.Index(i) + elem := eindirect(rv.Index(i)) enc.eElement(elem) if i != length-1 { enc.wf(", ") @@ -332,7 +332,7 @@ func (enc *Encoder) eArrayOfTables(key Key, rv reflect.Value) { encPanic(errNoKey) } for i := 0; i < rv.Len(); i++ { - trv := rv.Index(i) + trv := eindirect(rv.Index(i)) if isNil(trv) { continue } @@ -357,7 +357,7 @@ func (enc *Encoder) eTable(key Key, rv reflect.Value) { } func (enc *Encoder) eMapOrStruct(key Key, rv reflect.Value, inline bool) { - switch rv := eindirect(rv); rv.Kind() { + switch rv.Kind() { case reflect.Map: enc.eMap(key, rv, inline) case reflect.Struct: @@ -379,7 +379,7 @@ func (enc *Encoder) eMap(key Key, rv reflect.Value, inline bool) { var mapKeysDirect, mapKeysSub []string for _, mapKey := range rv.MapKeys() { k := mapKey.String() - if typeIsTable(tomlTypeOfGo(rv.MapIndex(mapKey))) { + if typeIsTable(tomlTypeOfGo(eindirect(rv.MapIndex(mapKey)))) { mapKeysSub = append(mapKeysSub, k) } else { mapKeysDirect = append(mapKeysDirect, k) @@ -389,7 +389,7 @@ func (enc *Encoder) eMap(key Key, rv reflect.Value, inline bool) { var writeMapKeys = func(mapKeys []string, trailC bool) { sort.Strings(mapKeys) for i, mapKey := range mapKeys { - val := rv.MapIndex(reflect.ValueOf(mapKey)) + val := eindirect(rv.MapIndex(reflect.ValueOf(mapKey))) if isNil(val) { continue } @@ -441,27 +441,16 @@ func (enc *Encoder) eStruct(key Key, rv reflect.Value, inline bool) { continue } - frv := rv.Field(i) + frv := eindirect(rv.Field(i)) // Treat anonymous struct fields with tag names as though they are // not anonymous, like encoding/json does. // // Non-struct anonymous fields use the normal encoding logic. if f.Anonymous { - t := f.Type - switch t.Kind() { - case reflect.Struct: - if getOptions(f.Tag).name == "" { - addFields(t, frv, append(start, f.Index...)) - continue - } - case reflect.Ptr: - if t.Elem().Kind() == reflect.Struct && getOptions(f.Tag).name == "" { - if !frv.IsNil() { - addFields(t.Elem(), frv.Elem(), append(start, f.Index...)) - } - continue - } + if getOptions(f.Tag).name == "" && frv.Kind() == reflect.Struct { + addFields(frv.Type(), frv, append(start, f.Index...)) + continue } } @@ -487,7 +476,7 @@ func (enc *Encoder) eStruct(key Key, rv reflect.Value, inline bool) { writeFields := func(fields [][]int) { for _, fieldIndex := range fields { fieldType := rt.FieldByIndex(fieldIndex) - fieldVal := rv.FieldByIndex(fieldIndex) + fieldVal := eindirect(rv.FieldByIndex(fieldIndex)) if isNil(fieldVal) { /// Don't write anything for nil fields. continue @@ -540,6 +529,21 @@ func tomlTypeOfGo(rv reflect.Value) tomlType { if isNil(rv) || !rv.IsValid() { return nil } + + if rv.Kind() == reflect.Struct { + if _, ok := rv.Interface().(time.Time); ok { + return tomlDatetime + } + if isMarshaler(rv) { + return tomlString + } + return tomlHash + } + + if isMarshaler(rv) { + return tomlString + } + switch rv.Kind() { case reflect.Bool: return tomlBool @@ -561,19 +565,7 @@ func tomlTypeOfGo(rv reflect.Value) tomlType { return tomlString case reflect.Map: return tomlHash - case reflect.Struct: - if _, ok := rv.Interface().(time.Time); ok { - return tomlDatetime - } - if isMarshaler(rv) { - return tomlString - } - return tomlHash default: - if isMarshaler(rv) { - return tomlString - } - encPanic(errors.New("unsupported type: " + rv.Kind().String())) panic("unreachable") } @@ -586,16 +578,6 @@ func isMarshaler(rv reflect.Value) bool { case Marshaler: return true } - - // Someone used a pointer receiver: we can make it work for pointer values. - if rv.CanAddr() { - if _, ok := rv.Addr().Interface().(encoding.TextMarshaler); ok { - return true - } - if _, ok := rv.Addr().Interface().(Marshaler); ok { - return true - } - } return false } @@ -605,19 +587,19 @@ func isTableArray(arr reflect.Value) bool { return false } - /// Don't allow nil. + ret := true for i := 0; i < arr.Len(); i++ { - if tomlTypeOfGo(arr.Index(i)) == nil { + tt := tomlTypeOfGo(eindirect(arr.Index(i))) + // Don't allow nil. + if tt == nil { encPanic(errArrayNilElement) } - } - for i := 0; i < arr.Len(); i++ { - if !typeEqual(tomlHash, tomlTypeOfGo(arr.Index(i))) { - return false + if ret && !typeEqual(tomlHash, tt) { + ret = false } } - return true + return ret } type tagOptions struct { @@ -716,12 +698,23 @@ func encPanic(err error) { } func eindirect(v reflect.Value) reflect.Value { - switch v.Kind() { - case reflect.Ptr, reflect.Interface: - return eindirect(v.Elem()) - default: + if v.Kind() != reflect.Ptr && v.Kind() != reflect.Interface { + if isMarshaler(v) { + return v + } + if v.CanAddr() { + if pv := v.Addr(); isMarshaler(pv) { + return pv + } + } return v } + + if v.IsNil() { + return v + } + + return eindirect(v.Elem()) } func isNil(rv reflect.Value) bool { From eaf0d98e8959a5586a50ae2d86946fe6b52e82f1 Mon Sep 17 00:00:00 2001 From: kkHAIKE Date: Fri, 17 Jun 2022 11:17:02 +0800 Subject: [PATCH 2/5] add test --- encode_test.go | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/encode_test.go b/encode_test.go index 446979b5..ea527c8f 100644 --- a/encode_test.go +++ b/encode_test.go @@ -7,6 +7,7 @@ import ( "math" "net" "os" + "strconv" "strings" "testing" "time" @@ -399,11 +400,13 @@ type ( food struct{ F []string } fun func() cplx complex128 + ints []int sound2 struct{ S string } food2 struct{ F []string } fun2 func() cplx2 complex128 + ints2 []int ) // This is intentionally wrong (pointer receiver) @@ -415,6 +418,26 @@ func (c cplx) MarshalText() ([]byte, error) { return []byte(fmt.Sprintf("(%f+%fi)", real(cplx), imag(cplx))), nil } +func intsValue(is []int) []byte { + var buf bytes.Buffer + buf.WriteByte('<') + for i, v := range is { + if i > 0 { + buf.WriteByte(',') + } + buf.WriteString(strconv.Itoa(v)) + } + buf.WriteByte('>') + return buf.Bytes() +} + +func (is *ints) MarshalText() ([]byte, error) { + if is == nil { + return []byte("[]"), nil + } + return intsValue(*is), nil +} + func (s *sound2) MarshalTOML() ([]byte, error) { return []byte("\"" + s.S + "\""), nil } func (f food2) MarshalTOML() ([]byte, error) { return []byte("[\"" + strings.Join(f.F, "\", \"") + "\"]"), nil @@ -424,6 +447,13 @@ func (c cplx2) MarshalTOML() ([]byte, error) { cplx := complex128(c) return []byte(fmt.Sprintf("\"(%f+%fi)\"", real(cplx), imag(cplx))), nil } +func (is *ints2) MarshalTOML() ([]byte, error) { + // MarshalTOML must quote by self + if is == nil { + return []byte(`"[]"`), nil + } + return []byte(fmt.Sprintf(`"%s"`, intsValue(*is))), nil +} func TestEncodeTextMarshaler(t *testing.T) { x := struct { @@ -435,6 +465,8 @@ func TestEncodeTextMarshaler(t *testing.T) { Food2 *food Complex cplx Fun fun + Ints ints + Ints2 *ints2 }{ Name: "Goblok", Sound: sound{"miauw"}, @@ -447,26 +479,28 @@ func TestEncodeTextMarshaler(t *testing.T) { Food2: &food{[]string{"chicken", "fish"}}, Complex: complex(42, 666), Fun: func() { panic("x") }, + Ints: ints{1, 2, 3, 4}, + Ints2: &ints2{1, 2, 3, 4}, } var buf bytes.Buffer - if err := NewEncoder(&buf).Encode(x); err != nil { + if err := NewEncoder(&buf).Encode(&x); err != nil { t.Fatal(err) } want := `Name = "Goblok" +Sound = "miauw" Sound2 = "miauw" Food = "chicken, fish" Food2 = "chicken, fish" Complex = "(42.000000+666.000000i)" Fun = "why would you do this?" +Ints = "<1,2,3,4>" +Ints2 = "<1,2,3,4>" [Labels] color = "black" type = "cat" - -[Sound] - S = "miauw" ` if buf.String() != want { From 0ae83fe04ce3ccb185491c3cbe827df0c1531e64 Mon Sep 17 00:00:00 2001 From: kkHAIKE Date: Fri, 17 Jun 2022 14:44:07 +0800 Subject: [PATCH 3/5] replace some Interface() check when encode private embed struct --- encode.go | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/encode.go b/encode.go index bfb76744..758a7e6f 100644 --- a/encode.go +++ b/encode.go @@ -64,6 +64,12 @@ var dblQuotedReplacer = strings.NewReplacer( "\x7f", `\u007f`, ) +var ( + marshalToml = reflect.TypeOf((*Marshaler)(nil)).Elem() + marshalText = reflect.TypeOf((*encoding.TextMarshaler)(nil)).Elem() + timeType = reflect.TypeOf((*time.Time)(nil)).Elem() +) + // Marshaler is the interface implemented by types that can marshal themselves // into valid TOML. type Marshaler interface { @@ -154,12 +160,12 @@ func (enc *Encoder) encode(key Key, rv reflect.Value) { // If we can marshal the type to text, then we use that. This prevents the // encoder for handling these types as generic structs (or whatever the // underlying type of a TextMarshaler is). - switch t := rv.Interface().(type) { - case encoding.TextMarshaler, Marshaler: + switch { + case isMarshaler(rv): enc.writeKeyValue(key, rv, false) return - case Primitive: // TODO: #76 would make this superfluous after implemented. - enc.encode(key, reflect.ValueOf(t.undecoded)) + case rv.Type() == primitiveType: // TODO: #76 would make this superfluous after implemented. + enc.encode(key, reflect.ValueOf(rv.Interface().(Primitive).undecoded)) return } @@ -429,11 +435,19 @@ func (enc *Encoder) eStruct(key Key, rv reflect.Value, inline bool) { rt = rv.Type() fieldsDirect, fieldsSub [][]int addFields func(rt reflect.Type, rv reflect.Value, start []int) + ptrto func(t reflect.Type) reflect.Type ) + ptrto = func(t reflect.Type) reflect.Type { + if t.Kind() == reflect.Ptr { + return ptrto(t.Elem()) + } + return t + } addFields = func(rt reflect.Type, rv reflect.Value, start []int) { for i := 0; i < rt.NumField(); i++ { f := rt.Field(i) - if f.PkgPath != "" && !f.Anonymous { /// Skip unexported fields. + isEmbed := f.Anonymous && ptrto(f.Type).Kind() == reflect.Struct + if f.PkgPath != "" && !isEmbed { /// Skip unexported fields. continue } opts := getOptions(f.Tag) @@ -447,7 +461,7 @@ func (enc *Encoder) eStruct(key Key, rv reflect.Value, inline bool) { // not anonymous, like encoding/json does. // // Non-struct anonymous fields use the normal encoding logic. - if f.Anonymous { + if isEmbed { if getOptions(f.Tag).name == "" && frv.Kind() == reflect.Struct { addFields(frv.Type(), frv, append(start, f.Index...)) continue @@ -531,7 +545,7 @@ func tomlTypeOfGo(rv reflect.Value) tomlType { } if rv.Kind() == reflect.Struct { - if _, ok := rv.Interface().(time.Time); ok { + if rv.Type() == timeType { return tomlDatetime } if isMarshaler(rv) { @@ -572,13 +586,8 @@ func tomlTypeOfGo(rv reflect.Value) tomlType { } func isMarshaler(rv reflect.Value) bool { - switch rv.Interface().(type) { - case encoding.TextMarshaler: - return true - case Marshaler: - return true - } - return false + return rv.Type().Implements(marshalText) || + rv.Type().Implements(marshalToml) } // isTableArray reports if all entries in the array or slice are a table. From 6805a3d586fba7aac0e9a7f15e6c2ee0fde53359 Mon Sep 17 00:00:00 2001 From: kkHAIKE Date: Fri, 17 Jun 2022 14:44:35 +0800 Subject: [PATCH 4/5] add test for embed field --- encode_test.go | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/encode_test.go b/encode_test.go index ea527c8f..6a1e57d3 100644 --- a/encode_test.go +++ b/encode_test.go @@ -246,17 +246,22 @@ func TestEncodeOmitemptyWithEmptyName(t *testing.T) { func TestEncodeAnonymousStruct(t *testing.T) { type Inner struct{ N int } - type Outer0 struct{ Inner } + type inner struct{ B int } + type Outer0 struct { + Inner + inner + } type Outer1 struct { Inner `toml:"inner"` + inner `toml:"innerb"` } - v0 := Outer0{Inner{3}} - expected := "N = 3\n" + v0 := Outer0{Inner{3}, inner{4}} + expected := "N = 3\nB = 4\n" encodeExpected(t, "embedded anonymous untagged struct", v0, expected, nil) - v1 := Outer1{Inner{3}} - expected = "[inner]\n N = 3\n" + v1 := Outer1{Inner{3}, inner{4}} + expected = "[inner]\n N = 3\n\n[innerb]\n B = 4\n" encodeExpected(t, "embedded anonymous tagged struct", v1, expected, nil) } @@ -315,6 +320,33 @@ func TestEncodeNestedAnonymousStructs(t *testing.T) { encodeExpected(t, "nested anonymous untagged structs", v, expected, nil) } +type InnerForNextTest struct{ N int } + +func (InnerForNextTest) F() {} +func (InnerForNextTest) G() {} + +func TestEncodeAnonymousNoStructField(t *testing.T) { + type Inner interface{ F() } + type inner interface{ G() } + type IntS []int + type intS []int + type Outer0 struct { + Inner + inner + IntS + intS + } + + v0 := Outer0{ + Inner: InnerForNextTest{3}, + inner: InnerForNextTest{4}, + IntS: []int{5, 6}, + intS: []int{7, 8}, + } + expected := "IntS = [5, 6]\n\n[Inner]\n N = 3\n" + encodeExpected(t, "non struct anonymous field", v0, expected, nil) +} + func TestEncodeIgnoredFields(t *testing.T) { type simple struct { Number int `toml:"-"` From c03a31caf873c5910944dac337e54ea838876464 Mon Sep 17 00:00:00 2001 From: Martin Tournoij Date: Sat, 25 Jun 2022 23:56:46 +0200 Subject: [PATCH 5/5] Small stylistic updates --- encode.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/encode.go b/encode.go index 758a7e6f..dc8568d1 100644 --- a/encode.go +++ b/encode.go @@ -423,6 +423,13 @@ func (enc *Encoder) eMap(key Key, rv reflect.Value, inline bool) { const is32Bit = (32 << (^uint(0) >> 63)) == 32 +func pointerTo(t reflect.Type) reflect.Type { + if t.Kind() == reflect.Ptr { + return pointerTo(t.Elem()) + } + return t +} + func (enc *Encoder) eStruct(key Key, rv reflect.Value, inline bool) { // Write keys for fields directly under this key first, because if we write // a field that creates a new table then all keys under it will be in that @@ -435,18 +442,11 @@ func (enc *Encoder) eStruct(key Key, rv reflect.Value, inline bool) { rt = rv.Type() fieldsDirect, fieldsSub [][]int addFields func(rt reflect.Type, rv reflect.Value, start []int) - ptrto func(t reflect.Type) reflect.Type ) - ptrto = func(t reflect.Type) reflect.Type { - if t.Kind() == reflect.Ptr { - return ptrto(t.Elem()) - } - return t - } addFields = func(rt reflect.Type, rv reflect.Value, start []int) { for i := 0; i < rt.NumField(); i++ { f := rt.Field(i) - isEmbed := f.Anonymous && ptrto(f.Type).Kind() == reflect.Struct + isEmbed := f.Anonymous && pointerTo(f.Type).Kind() == reflect.Struct if f.PkgPath != "" && !isEmbed { /// Skip unexported fields. continue } @@ -586,8 +586,7 @@ func tomlTypeOfGo(rv reflect.Value) tomlType { } func isMarshaler(rv reflect.Value) bool { - return rv.Type().Implements(marshalText) || - rv.Type().Implements(marshalToml) + return rv.Type().Implements(marshalText) || rv.Type().Implements(marshalToml) } // isTableArray reports if all entries in the array or slice are a table. @@ -706,12 +705,13 @@ func encPanic(err error) { panic(tomlEncodeError{err}) } +// Resolve any level of pointers to the actual value (e.g. **string → string). func eindirect(v reflect.Value) reflect.Value { if v.Kind() != reflect.Ptr && v.Kind() != reflect.Interface { if isMarshaler(v) { return v } - if v.CanAddr() { + if v.CanAddr() { /// Special case for marshalers; see #358. if pv := v.Addr(); isMarshaler(pv) { return pv }