Repository: BurntSushi/toml
Pull request #426 (2024-09-24 14:28:56)

Set mapping keys to "decoded" when custom unmarshaler is used
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: 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



arp242: This seems like a reasonable change.

Could you add a more extensive test though? At the very least something like:

	a   = 1
	arr = [2]

	[tbl]
	b = 3

	inline = {c = 4}

Just so that keys in different locations are tested: array, table, inline table. Maybe a few more? I'm not sure if these are different code-paths off-hand, but doesn't hurt to test it.
mvo5: Thanks a lot for your feedback! I updated the code and it now includes the more complex testcase that you suggested and that uncovered missing handling of nested which I added now too. I am happy to add more tests, but I need to think a bit what else to check for (I'm not super familiar with the toml details). Ideas welcome here of course :) 

Please also let me know if I should squash or rebase my commits, I did them linearly mostly to make it easy to review but happy to do whatever is needed/preferred.
mvo5: Friendly ping, anything I can do or improve to help this along?
arp242: I'd like to carefully look at it and play around with it a bit. Not terrible time-consuming, but I don't have a lot of bandwidth for that right now. It's "on the list" and not forgotten, but it might be a bit before I have a chance to properly look at it.

Patch:
diff --git a/decode.go b/decode.go
index c05a0b7..3fa516c 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,6 +235,16 @@ func (md *MetaData) unify(data any, rv reflect.Value) error {
 		if err != nil {
 			return md.parseErr(err)
 		}
+		// Assume the Unmarshaler decoded everything, so mark all keys under
+		// this table as decoded.
+		if tmap, ok := data.(map[string]any); ok {
+			markDecodedRecursive(md, tmap)
+		}
+		if aot, ok := data.([]map[string]any); ok {
+			for _, tmap := range aot {
+				markDecodedRecursive(md, tmap)
+			}
+		}
 		return nil
 	}
 	if v, ok := rvi.(encoding.TextUnmarshaler); ok {
diff --git a/error.go b/error.go
index 1dd5232..b7077d3 100644
--- a/error.go
+++ b/error.go
@@ -69,7 +69,7 @@ type Position struct {
 	Line  int // Line number, starting at 1.
 	Col   int // Error column, starting at 1.
 	Start int // Start of error, as byte offset starting at 0.
-	Len   int // Lenght of the error in bytes.
+	Len   int // Length of the error in bytes.
 }
 
 func (p Position) withCol(tomlFile string) Position {
diff --git a/lex.go b/lex.go
index cd9de46..1c3b477 100644
--- a/lex.go
+++ b/lex.go
@@ -275,7 +275,9 @@ func (lx *lexer) errorPos(start, length int, err error) stateFn {
 func (lx *lexer) errorf(format string, values ...any) stateFn {
 	if lx.atEOF {
 		pos := lx.getPos()
-		pos.Line--
+		if lx.pos >= 1 && lx.input[lx.pos-1] == '\n' {
+			pos.Line--
+		}
 		pos.Len = 1
 		pos.Start = lx.pos - 1
 		lx.items <- item{typ: itemError, pos: pos, err: fmt.Errorf(format, values...)}
@@ -1117,7 +1119,7 @@ func lexBaseNumberOrDate(lx *lexer) stateFn {
 	case 'x':
 		r = lx.peek()
 		if !isHex(r) {
-			lx.errorf("not a hexidecimal number: '%s%c'", lx.current(), r)
+			lx.errorf("not a hexadecimal number: '%s%c'", lx.current(), r)
 		}
 		return lexHexInteger
 	}
diff --git a/ossfuzz/fuzz.go b/ossfuzz/fuzz.go
index 755f82c..6a29773 100644
--- a/ossfuzz/fuzz.go
+++ b/ossfuzz/fuzz.go
@@ -3,7 +3,6 @@ package ossfuzz
 import (
 	"bytes"
 	"fmt"
-	"strings"
 
 	"github.com/BurntSushi/toml"
 )
@@ -28,11 +27,6 @@ func FuzzToml(data []byte) int {
 	var v2 any
 	_, err = toml.Decode(buf.String(), &v2)
 	if err != nil {
-		// TODO(manunio): remove this when 1.23 lands, see #407.
-		if strings.Contains(err.Error(), "invalid datetime") {
-			return 0
-		}
-
 		panic(fmt.Sprintf("failed round trip: %s", err))
 	}
 
diff --git a/parse.go b/parse.go
index 3f2c090..349a9fa 100644
--- a/parse.go
+++ b/parse.go
@@ -529,7 +529,7 @@ func numUnderscoresOK(s string) bool {
 			}
 		}
 
-		// isHexis a superset of all the permissable characters surrounding an
+		// isHex is a superset of all the permissible characters surrounding an
 		// underscore.
 		accept = isHex(r)
 	}


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 10c363f..4647b48 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -13,11 +13,11 @@
       "env": {"GOPROXY": "direct"},
       "steps": [{
           "name": "Install Go",
-          "uses": "actions/setup-go@v3",
+          "uses": "actions/setup-go@v4",
           "with": {"go-version": "${{ matrix.go-version }}"}
         }, {
           "name": "Checkout code",
-          "uses": "actions/checkout@v3"
+          "uses": "actions/checkout@v4"
         }, {
           "name": "Test",
           "run":  "go test -race ./..."
diff --git a/decode_test.go b/decode_test.go
index b547990..1f82a2a 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,45 @@ func BenchmarkKey(b *testing.B) {
 		k.String()
 	}
 }
+
+type UnmarshalTOMLNoop struct {
+}
+
+func (cs *UnmarshalTOMLNoop) UnmarshalTOML(data any) error {
+	return nil
+}
+
+func TestDecodeCustomStructMarkedDecoded(t *testing.T) {
+	var s struct {
+		Str UnmarshalTOMLNoop `toml:"str"`
+		Int UnmarshalTOMLNoop `toml:"int"`
+		Arr UnmarshalTOMLNoop `toml:"arr"`
+		Tbl UnmarshalTOMLNoop `toml:"tbl"`
+		Aot UnmarshalTOMLNoop `toml:"aot"`
+	}
+	meta, err := Decode(`
+		str = "bar"
+		int   = 1
+		arr = [2]
+
+		[tbl]
+		b = 3
+		inline = {c = 4}
+
+		[[aot]]
+		a = 1
+		[[aot]]
+		a = 1
+	`, &s)
+	if err != nil {
+		t.Fatalf("Decode failed: %s", err)
+	}
+
+	// Note that even though the custom unmarshaller did not decode all fields
+	// as far as the metadata is concerned they are handled. It is the job of
+	// the unmarshaller 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 undecoded fields: %v\n", meta.Undecoded())
+	}
+}
diff --git a/encode_test.go b/encode_test.go
index a5c3a7e..d871aee 100644
--- a/encode_test.go
+++ b/encode_test.go
@@ -149,7 +149,7 @@ func TestEncodeOmitEmptyStruct(t *testing.T) {
 }
 
 func TestEncodeOmitEmpty(t *testing.T) {
-	type compareable struct {
+	type comparable struct {
 		Bool bool `toml:"bool,omitempty"`
 	}
 	type uncomparable struct {
@@ -166,8 +166,8 @@ func TestEncodeOmitEmpty(t *testing.T) {
 		Slice               []int              `toml:"slice,omitempty"`
 		Map                 map[string]string  `toml:"map,omitempty"`
 		Time                time.Time          `toml:"time,omitempty"`
-		Compareable1        compareable        `toml:"compareable1,omitempty"`
-		Compareable2        compareable        `toml:"compareable2,omitempty"`
+		Comparable1         comparable         `toml:"comparable1,omitempty"`
+		Comparable2         comparable         `toml:"comparable2,omitempty"`
 		Uncomparable1       uncomparable       `toml:"uncomparable1,omitempty"`
 		Uncomparable2       uncomparable       `toml:"uncomparable2,omitempty"`
 		NestedUncomparable1 nestedUncomparable `toml:"nesteduncomparable1,omitempty"`
@@ -182,7 +182,7 @@ func TestEncodeOmitEmpty(t *testing.T) {
 		Slice:               []int{2, 3, 4},
 		Map:                 map[string]string{"foo": "bar"},
 		Time:                time.Date(1985, 6, 18, 15, 16, 17, 0, time.UTC),
-		Compareable2:        compareable{true},
+		Comparable2:         comparable{true},
 		Uncomparable2:       uncomparable{[]string{"XXX"}},
 		NestedUncomparable1: nestedUncomparable{uncomparable{[]string{"XXX"}}, false},
 		NestedUncomparable2: nestedUncomparable{uncomparable{}, true},
@@ -195,7 +195,7 @@ time = 1985-06-18T15:16:17Z
 [map]
   foo = "bar"
 
-[compareable2]
+[comparable2]
   bool = true
 
 [uncomparable2]
diff --git a/error_test.go b/error_test.go
index d14283e..fc439e0 100644
--- a/error_test.go
+++ b/error_test.go
@@ -245,6 +245,19 @@ func TestParseError(t *testing.T) {
             |         15:04:05.856018510
 			`,
 		},
+
+		{
+			&struct{ String string }{},
+			`string = "test`,
+			`
+            | toml: error: unexpected EOF; expected '"'
+			|
+			| At line 1, column 14:
+			|
+			|       1 | string = "test
+			|                        ^
+			`,
+		},
 	}
 
 	prep := func(s string) string {
@@ -337,7 +350,7 @@ func (n *Enum2) UnmarshalTOML(text any) error {
 	return nil
 }
 
-func TestMarhsalError(t *testing.T) {
+func TestMarshalError(t *testing.T) {
 	var c struct {
 		K1 string `toml:"k1"`
 		K2 Enum2  `toml:"k2"`
diff --git a/example_test.go b/example_test.go
index da992c1..470959a 100644
--- a/example_test.go
+++ b/example_test.go
@@ -225,7 +225,7 @@ func Example_strictDecoding() {
 type order struct {
 	// NOTE `order.parts` is a private slice of type `part` which is an
 	// interface and may only be loaded from toml using the UnmarshalTOML()
-	// method of the Umarshaler interface.
+	// method of the Unmarshaler interface.
 	parts parts
 }
 
diff --git a/internal/toml-test/runner.go b/internal/toml-test/runner.go
index 666e8ed..69b0e92 100644
--- a/internal/toml-test/runner.go
+++ b/internal/toml-test/runner.go
@@ -101,7 +101,7 @@ type Test struct {
 
 	Skipped          bool          // Skipped this test?
 	Failure          string        // Failure message.
-	Key              string        // TOML key the failure occured on; may be blank.
+	Key              string        // TOML key the failure occurred on; may be blank.
 	Encoder          bool          // Encoder test?
 	Input            string        // The test case that we sent to the external program.
 	Output           string        // Output from the external program.
diff --git a/internal/toml-test/tests/valid/string/escapes.toml b/internal/toml-test/tests/valid/string/escapes.toml
index 5146065..1d949e1 100644
--- a/internal/toml-test/tests/valid/string/escapes.toml
+++ b/internal/toml-test/tests/valid/string/escapes.toml
@@ -8,7 +8,7 @@ backslash     = "|\\."
 delete        = "|\u007F."
 unitseparator = "|\u001F."
 
-# \u is escaped, so should NOT be interperted as a \u escape.
+# \u is escaped, so should NOT be interpreted as a \u escape.
 notunicode1   = "|\\u."
 notunicode2   = "|\u005Cu."
 notunicode3   = "|\\u0075."
diff --git a/toml_test.go b/toml_test.go
index bbf54e4..029fcdc 100644
--- a/toml_test.go
+++ b/toml_test.go
@@ -283,7 +283,7 @@ func runTomlTest(t *testing.T, includeNext bool, wantFail ...string) {
 	}
 
 	// TODO: bit of a hack to make sure not all test run; without this "-run=.."
-	// will still run alll tests, but just report the errors for the -run value.
+	// will still run all tests, but just report the errors for the -run value.
 	// This is annoying in cases where you have some debug printf.
 	//
 	// Need to update toml-test a bit to make this easier, but this good enough