From e4d6f996a8928270c335ece2028dbc8361d04582 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 24 Sep 2024 16:10:17 +0200 Subject: [PATCH 1/3] decode: add (failing) test for undecoded fields in a struct This commit adds a failing test for the scenario decribed in issue https://github.com/BurntSushi/toml/issues/425 --- decode_test.go | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/decode_test.go b/decode_test.go index b547990c..e34fcf5e 100644 --- a/decode_test.go +++ b/decode_test.go @@ -1018,7 +1018,7 @@ func TestCustomEncode(t *testing.T) { // Test for #341 func TestCustomDecode(t *testing.T) { var outer Outer - _, err := Decode(` + meta, err := Decode(` Int = 10 Enum = "OTHER_VALUE" Slice = ["text1", "text2"] @@ -1036,6 +1036,9 @@ func TestCustomDecode(t *testing.T) { if fmt.Sprint(outer.Slice.value) != fmt.Sprint([]string{"text1", "text2"}) { t.Errorf("\nhave:\n%v\nwant:\n%v\n", outer.Slice.value, []string{"text1", "text2"}) } + if len(meta.Undecoded()) > 0 { + t.Errorf("\ncustom decode leaves unencoded fields: %v\n", meta.Undecoded()) + } } // TODO: this should be improved for v2: @@ -1144,3 +1147,30 @@ func BenchmarkKey(b *testing.B) { k.String() } } + +type CustomStruct struct { + Foo string `json:"foo"` +} + +func (cs *CustomStruct) UnmarshalTOML(data interface{}) error { + d, _ := data.(map[string]interface{}) + cs.Foo = d["foo"].(string) + return nil +} + +func TestDecodeCustomStruct(t *testing.T) { + var cs CustomStruct + meta, err := Decode(` + foo = "bar" + `, &cs) + if err != nil { + t.Fatalf("Decode failed: %s", err) + } + + if cs.Foo != "bar" { + t.Errorf("\nhave:\n%v\nwant:\n%v\n", cs.Foo, "bar") + } + if len(meta.Undecoded()) > 0 { + t.Errorf("\ncustom decode leaves unencoded fields: %v\n", meta.Undecoded()) + } +} From 70d427dc9782b446329735431be96369a22c4eaa Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 24 Sep 2024 16:22:53 +0200 Subject: [PATCH 2/3] decode: set all keys under a mapping with custom marshaler decoded This is a naive fix for the issue of custom unmarshal and marking keys as "decoded". It simply assumes that if there was a custom unmarshal for a mapping type all keys got handlded by the custom unmarshal code. This might be too naive but it seems reasonable and it also seems we would need a richer interface for a custom unmarshal that gives access to `MetaData` if we want to be more fine grained. --- decode.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/decode.go b/decode.go index c05a0b7e..afdd0183 100644 --- a/decode.go +++ b/decode.go @@ -222,6 +222,13 @@ func (md *MetaData) unify(data any, rv reflect.Value) error { if err != nil { return md.parseErr(err) } + // assume the Unmarshaler did it's job and decoded all fields + tmap, ok := data.(map[string]any) + if ok { + for key := range tmap { + md.decoded[md.context.add(key).String()] = struct{}{} + } + } return nil } if v, ok := rvi.(encoding.TextUnmarshaler); ok { From 8ff45713ecd2d0076ddcc5263bc21ced4384365d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 26 Sep 2024 09:37:09 +0200 Subject: [PATCH 3/3] decode: expand test and mark fields decoded recursively This commit expands the test `TestDecodeCustomStructMarkedDecoded` so that nested data is also tested (thanks to Martin Tournoij). This lead to an update of the code to also mark all nested keys as decoded when a custom `UnmarshalTOML(data any)` interface is used. It is the job of the implemenator of the UnmarshalTOML() to ensure everything is decoded. An alternative would be to provide a new `UnmarshalTOMLWithMetadata(data any, md *MetaData)` inteface that would allow the implementaor of the custom unmarshaller to mark fields as decoded. But it's unclear if that is needed because a UnmarshalTOML() can already error if it "sees" unexpected or missing data. --- decode.go | 23 +++++++++++++++++------ decode_test.go | 28 +++++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/decode.go b/decode.go index afdd0183..8103dfb8 100644 --- a/decode.go +++ b/decode.go @@ -196,6 +196,19 @@ func (md *MetaData) PrimitiveDecode(primValue Primitive, v any) error { return md.unify(primValue.undecoded, rvalue(v)) } +// markDecodedRecursive is a helper to mark any key under the given tmap +// as decoded, recursing as needed +func markDecodedRecursive(md *MetaData, tmap map[string]any) { + for key := range tmap { + md.decoded[md.context.add(key).String()] = struct{}{} + if tmap, ok := tmap[key].(map[string]any); ok { + md.context = append(md.context, key) + markDecodedRecursive(md, tmap) + md.context = md.context[0 : len(md.context)-1] + } + } +} + // unify performs a sort of type unification based on the structure of `rv`, // which is the client representation. // @@ -222,12 +235,10 @@ func (md *MetaData) unify(data any, rv reflect.Value) error { if err != nil { return md.parseErr(err) } - // assume the Unmarshaler did it's job and decoded all fields - tmap, ok := data.(map[string]any) - if ok { - for key := range tmap { - md.decoded[md.context.add(key).String()] = struct{}{} - } + // assume the Unmarshaler did it's job and mark all + // keys under this map decoded + if tmap, ok := data.(map[string]any); ok { + markDecodedRecursive(md, tmap) } return nil } diff --git a/decode_test.go b/decode_test.go index e34fcf5e..262ca076 100644 --- a/decode_test.go +++ b/decode_test.go @@ -1149,19 +1149,31 @@ func BenchmarkKey(b *testing.B) { } type CustomStruct struct { - Foo string `json:"foo"` + Foo string + TblB int64 + TblInlineC int64 } func (cs *CustomStruct) UnmarshalTOML(data interface{}) error { d, _ := data.(map[string]interface{}) cs.Foo = d["foo"].(string) + cs.TblB = d["tbl"].(map[string]interface{})["b"].(int64) + cs.TblInlineC = d["tbl"].(map[string]interface{})["inline"].(map[string]interface{})["c"].(int64) + return nil } -func TestDecodeCustomStruct(t *testing.T) { +func TestDecodeCustomStructMarkedDecoded(t *testing.T) { var cs CustomStruct meta, err := Decode(` - foo = "bar" +foo = "bar" +a = 1 +arr = [2] + +[tbl] +b = 3 + +inline = {c = 4} `, &cs) if err != nil { t.Fatalf("Decode failed: %s", err) @@ -1170,6 +1182,16 @@ func TestDecodeCustomStruct(t *testing.T) { if cs.Foo != "bar" { t.Errorf("\nhave:\n%v\nwant:\n%v\n", cs.Foo, "bar") } + if cs.TblB != 3 { + t.Errorf("\nhave:\n%v\nwant:\n%v\n", cs.TblB, 3) + } + if cs.TblInlineC != 4 { + t.Errorf("\nhave:\n%v\nwant:\n%v\n", cs.TblB, 4) + } + // Note that even though the custom unmarshaler did not decode + // all fields as far as the metadata is concerned they are handlded. + // It is the job of the unmarshaler to ensure this or we would need + // a more powerful interface like UnmarshalTOML(data any, md *MetaData) if len(meta.Undecoded()) > 0 { t.Errorf("\ncustom decode leaves unencoded fields: %v\n", meta.Undecoded()) }