Expand meta tests; document some problems

Expand the meta tests, and also ensure they all get run so typos will be an error instead of being silently ignored. Replace some map[string]bool with map[string]struct{}, since that won't allocate anything. Super-small micro-optimisation, but I like it as it clarifies that the value is irrelevant and that we're just using the map as a set. Also document some problems with how MetaData and Key works: - The keys "a.b" (one quoted key with a dot) and a.b conflict. - Implicit keys aren't recorded, although IsDefined() does check for it, Type() returns nothing and they're not in the Keys() output. This really should be in there, and becomes important especially later if we use this for more things. - Array entries aren't easily accessible individually; they're recorded in Keys() multiple times with the same key name, and they all have the same type because the types is accessed as a map. This is an issue because not all array entries need to have the same type. The syntax for this should be different; e.g: array ArrayHash array[0].first_name String array[1].first_name String Or something to that effect. The problem is that this isn't really a backwards-compatible change: the Keys() return value will be quite different for arrays.
BurntSushi · Nov 24, 2021 · 8a1e523 · 8a1e523
1 parent 14d3756
commit 8a1e523
Show file tree

Hide file tree

Showing 6 changed files with 318 additions and 50 deletions.
diff --git a/decode.go b/decode.go
@@ -127,11 +127,12 @@ func (dec *Decoder) Decode(v interface{}) (MetaData, error) {
 	if err != nil {
 		return MetaData{}, err
 	}
+
 	md := MetaData{
 		mapping: p.mapping,
 		types:   p.types,
 		keys:    p.ordered,
-		decoded: make(map[string]bool, len(p.ordered)),
+		decoded: make(map[string]struct{}, len(p.ordered)),
 		context: nil,
 	}
 	return md, md.unify(p.mapping, indirect(rv))
@@ -263,7 +264,7 @@ func (md *MetaData) unifyStruct(mapping interface{}, rv reflect.Value) error {
 			}
 
 			if isUnifiable(subv) {
-				md.decoded[md.context.add(key).String()] = true
+				md.decoded[md.context.add(key).String()] = struct{}{}
 				md.context = append(md.context, key)
 				err := md.unify(datum, subv)
 				if err != nil {
@@ -296,7 +297,7 @@ func (md *MetaData) unifyMap(mapping interface{}, rv reflect.Value) error {
 		rv.Set(reflect.MakeMap(rv.Type()))
 	}
 	for k, v := range tmap {
-		md.decoded[md.context.add(k).String()] = true
+		md.decoded[md.context.add(k).String()] = struct{}{}
 		md.context = append(md.context, k)
 
 		rvkey := indirect(reflect.New(rv.Type().Key()))

diff --git a/decode_test.go b/decode_test.go
@@ -704,6 +704,32 @@ func TestDecodeDatetime(t *testing.T) {
 	}
 }
 
+func TestMetaDotConflict(t *testing.T) {
+	// See comment in the metaTest map in toml_test.go
+	t.Skip()
+
+	var m map[string]interface{}
+	meta, err := Decode(`
+		"a.b" = "str"
+		a.b   = 1
+	`, &m)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := "a.b=String; a|b=Integer"
+	have := ""
+	for i, k := range meta.Keys() {
+		if i > 0 {
+			have += "; "
+		}
+		have += strings.Join(k, "|") + "=" + meta.Type(k...)
+	}
+	if have != want {
+		t.Errorf("\nhave: %s\nwant: %s", have, want)
+	}
+}
+
 // errorContains checks if the error message in have contains the text in
 // want.
 //

diff --git a/deprecated.go b/deprecated.go
@@ -13,7 +13,7 @@ type TextUnmarshaler encoding.TextUnmarshaler
 
 // Deprecated: use MetaData.PrimitiveDecode.
 func PrimitiveDecode(primValue Primitive, v interface{}) error {
-	md := MetaData{decoded: make(map[string]bool)}
+	md := MetaData{decoded: make(map[string]struct{})}
 	return md.unify(primValue.undecoded, rvalue(v))
 }
 

diff --git a/meta.go b/meta.go
@@ -1,34 +1,38 @@
 package toml
 
-import "strings"
+import (
+	"strings"
+)
 
-// MetaData allows access to meta information about TOML data that may not be
-// inferable via reflection. In particular, whether a key has been defined and
-// the TOML type of a key.
+// MetaData allows access to meta information about TOML data that's not
+// accessible otherwise.
+//
+// It allows checking if a key is defined in the TOML data, whether any keys
+// were undecoded, and the TOML type of a key.
 type MetaData struct {
 	mapping map[string]interface{}
 	types   map[string]tomlType
 	keys    []Key
-	decoded map[string]bool
+	decoded map[string]struct{}
 	context Key // Used only during decoding.
 }
 
 // IsDefined reports if the key exists in the TOML data.
 //
 // The key should be specified hierarchically, for example to access the TOML
-// key "a.b.c" you would use:
-//
-//	IsDefined("a", "b", "c")
+// key "a.b.c" you would use IsDefined("a", "b", "c"). Keys are case sensitive.
 //
-// IsDefined will return false if an empty key given. Keys are case sensitive.
+// Returns false for an empty key.
 func (md *MetaData) IsDefined(key ...string) bool {
 	if len(key) == 0 {
 		return false
 	}
 
-	var hash map[string]interface{}
-	var ok bool
-	var hashOrVal interface{} = md.mapping
+	var (
+		hash      map[string]interface{}
+		ok        bool
+		hashOrVal interface{} = md.mapping
+	)
 	for _, k := range key {
 		if hash, ok = hashOrVal.(map[string]interface{}); !ok {
 			return false
@@ -77,7 +81,7 @@ func (md *MetaData) Keys() []Key {
 func (md *MetaData) Undecoded() []Key {
 	undecoded := make([]Key, 0, len(md.keys))
 	for _, key := range md.keys {
-		if !md.decoded[key.String()] {
+		if _, ok := md.decoded[key.String()]; !ok {
 			undecoded = append(undecoded, key)
 		}
 	}

diff --git a/parse.go b/parse.go
@@ -15,11 +15,11 @@ type parser struct {
 	types   map[string]tomlType
 	lx      *lexer
 
-	ordered    []Key           // List of keys in the order that they appear in the TOML data.
-	context    Key             // Full key for the current hash in scope.
-	currentKey string          // Base key name for everything except hashes.
-	pos        Position        // Position
-	implicits  map[string]bool // Record implied keys (e.g. 'key.group.names').
+	ordered    []Key               // List of keys in the order that they appear in the TOML data.
+	context    Key                 // Full key for the current hash in scope.
+	currentKey string              // Base key name for everything except hashes.
+	pos        Position            // Position
+	implicits  map[string]struct{} // Record implied keys (e.g. 'key.group.names').
 }
 
 func parse(data string) (p *parser, err error) {
@@ -61,7 +61,7 @@ func parse(data string) (p *parser, err error) {
 		types:     make(map[string]tomlType),
 		lx:        lex(data),
 		ordered:   make([]Key, 0),
-		implicits: make(map[string]bool),
+		implicits: make(map[string]struct{}),
 	}
 	for {
 		item := p.next()
@@ -359,6 +359,12 @@ func (p *parser) valueArray(it item) (interface{}, tomlType) {
 		val, typ := p.value(it, true)
 		array = append(array, val)
 		types = append(types, typ)
+
+		// XXX: types isn't used here, we need it to record the accurate type
+		// information.
+		//
+		// Not entirely sure how to best store this; could use "key[0]",
+		// "key[1]" notation, or maybe store it on the Array type?
 	}
 	return array, tomlArray
 }
@@ -602,9 +608,9 @@ func (p *parser) setType(key string, typ tomlType) {
 
 // Implicit keys need to be created when tables are implied in "a.b.c.d = 1" and
 // "[a.b.c]" (the "a", "b", and "c" hashes are never created explicitly).
-func (p *parser) addImplicit(key Key)     { p.implicits[key.String()] = true }
-func (p *parser) removeImplicit(key Key)  { p.implicits[key.String()] = false }
-func (p *parser) isImplicit(key Key) bool { return p.implicits[key.String()] }
+func (p *parser) addImplicit(key Key)     { p.implicits[key.String()] = struct{}{} }
+func (p *parser) removeImplicit(key Key)  { delete(p.implicits, key.String()) }
+func (p *parser) isImplicit(key Key) bool { _, ok := p.implicits[key.String()]; return ok }
 func (p *parser) isArray(key Key) bool    { return p.types[key.String()] == tomlArray }
 func (p *parser) addImplicitContext(key Key) {
 	p.addImplicit(key)