Implement duplicate and key types check

2021-03-29 10:45:50 -04:00
parent da21b0aecf
commit 2ddbf6be6d
2 changed files with 162 additions and 59 deletions
@@ -16,7 +16,7 @@ Development branch. Probably does not work.
 - [x] Original go-toml unmarshal tests pass.
 - [x] Benchmark!
 - [x] Abstract AST.
- [ ] Original go-toml testgen tests pass.
+- [x] Original go-toml testgen tests pass.
 - [ ] Attach comments to AST (gated by parser flag).
 - [ ] Track file position (line, column) for errors.
 - [ ] Benchmark again!
@@ -28,6 +28,7 @@ Development branch. Probably does not work.
 - [ ] Provide "minimal allocations" option that uses `unsafe` to reuse the input
      byte array as storage for strings.
 - [x] Cache reflection operations per type.
+- [ ] Optimize tracker pass.

 ## Ideas

@@ -9,90 +9,192 @@ import (
 type keyKind uint8

 const (
-	invalid keyKind = iota // also used for the root key
-	value
-	table
-	arrayTable
+	invalidKind keyKind = iota
+	valueKind
+	tableKind
+	arrayTableKind
 )

-type key string
-
-type builder struct {
-	prefix [][]byte
-	local  [][]byte
-}
-
-func (b *builder) Reset(prefix [][]byte) {
-	b.prefix = prefix
-	b.local = b.local[:0]
-}
-
-// Computes the number of bytes required to store the full key.
-func (b *builder) size() int {
-	size := len(b.prefix) + len(b.local) - 1
-	for _, p := range b.prefix {
-		size += len(p)
+func (k keyKind) String() string {
+	switch k {
+	case invalidKind:
+		return "invalid"
+	case valueKind:
+		return "value"
+	case tableKind:
+		return "table"
+	case arrayTableKind:
+		return "array table"
 	}
-	for _, p := range b.local {
-		size += len(p)
-	}
-	return size
-}
-
-func (b *builder) copy(firstJoin bool, from [][]byte, to []byte) int {
-	offset := 0
-	for i, p := range from {
-		if i > 0 || firstJoin {
-			to[offset] = 0x1E
-			offset++
-		}
-		copy(to[offset:], p)
-		offset += len(p)
-	}
-	return offset
-}
-
-func (b *builder) MakeKey() key {
-	k := make([]byte, b.size())
-	b.copy(false, b.prefix, k)
-	b.copy(len(b.prefix) > 0, b.local, k)
-	return key(k)
-}
-
-func (b *builder) Append(k []byte) {
-	b.local = append(b.local, k)
+	panic("missing keyKind string mapping")
 }

 // Tracks which keys have been seen with which TOML type to flag duplicates
 // and mismatches according to the spec.
 type Seen struct {
-	keys map[key]keyKind
+	root    *info
+	current *info
+}

-	// scoping from the previous CheckExpression call.
-	current [][]byte
+type info struct {
+	parent   *info
+	kind     keyKind
+	children map[string]*info
+	explicit bool
+}

-	// key builder
-	builder builder
+func (i *info) Clear() {
+	i.children = nil
+}
+
+func (i *info) Has(k string) (*info, bool) {
+	c, ok := i.children[k]
+	return c, ok
+}
+
+func (i *info) SetKind(kind keyKind) {
+	i.kind = kind
+}
+
+func (i *info) CreateTable(k string, explicit bool) *info {
+	return i.createChild(k, tableKind, explicit)
+}
+
+func (i *info) CreateArrayTable(k string, explicit bool) *info {
+	return i.createChild(k, arrayTableKind, explicit)
+}
+
+func (i *info) createChild(k string, kind keyKind, explicit bool) *info {
+	if i.children == nil {
+		i.children = make(map[string]*info, 1)
+	}
+
+	x := &info{
+		parent:   i,
+		kind:     kind,
+		explicit: explicit,
+	}
+	i.children[k] = x
+	return x
 }

 // CheckExpression takes a top-level node and checks that it does not contain keys
 // that have been seen in previous calls, and validates that types are consistent.
 func (s *Seen) CheckExpression(node ast.Node) error {
-	s.builder.Reset(s.current)
+	if s.root == nil {
+		s.root = &info{
+			kind: tableKind,
+		}
+		s.current = s.root
+	}
 	switch node.Kind {
 	case ast.KeyValue:
-		return s.checkKeyValue(node)
+		return s.checkKeyValue(s.current, node)
 	case ast.Table:
+		return s.checkTable(node)
 	case ast.ArrayTable:
+		return s.checkArrayTable(node)
 	default:
 		panic(fmt.Errorf("this should not be a top level node type: %s", node.Kind))
 	}
 	return nil
 }
+func (s *Seen) checkTable(node ast.Node) error {
+	s.current = s.root

-func (s *Seen) checkKeyValue(node ast.Node) error {
 	it := node.Key()
+	// handle the first parts of the key, excluding the last one
 	for it.Next() {
-		s.builder.Append(it.Node().Data)
+		if !it.Node().Next().Valid() {
+			break
+		}
+
+		k := string(it.Node().Data)
+		child, found := s.current.Has(k)
+		if !found {
+			child = s.current.CreateTable(k, false)
+		}
+		s.current = child
 	}
+
+	// handle the last part of the key
+	k := string(it.Node().Data)
+
+	i, found := s.current.Has(k)
+	if found {
+		if i.kind != tableKind {
+			return fmt.Errorf("key %s should be a table", k)
+		}
+		if i.explicit {
+			return fmt.Errorf("table %s already exists", k)
+		}
+		i.explicit = true
+		s.current = i
+	} else {
+		s.current = s.current.CreateTable(k, true)
+	}
+
+	return nil
+}
+
+func (s *Seen) checkArrayTable(node ast.Node) error {
+	s.current = s.root
+
+	it := node.Key()
+
+	// handle the first parts of the key, excluding the last one
+	for it.Next() {
+		if !it.Node().Next().Valid() {
+			break
+		}
+
+		k := string(it.Node().Data)
+		child, found := s.current.Has(k)
+		if !found {
+			child = s.current.CreateTable(k, false)
+		}
+		s.current = child
+	}
+
+	// handle the last part of the key
+	k := string(it.Node().Data)
+
+	info, found := s.current.Has(k)
+	if found {
+		if info.kind != arrayTableKind {
+			return fmt.Errorf("key %s already exists but is not an array table", k)
+		}
+		info.Clear()
+	} else {
+		info = s.current.CreateArrayTable(k, true)
+	}
+
+	s.current = info
+	return nil
+}
+
+func (s *Seen) checkKeyValue(context *info, node ast.Node) error {
+	it := node.Key()
+
+	// handle the first parts of the key, excluding the last one
+	for it.Next() {
+		k := string(it.Node().Data)
+		child, found := context.Has(k)
+		if found {
+			if child.kind != tableKind {
+				return fmt.Errorf("expected %s to be a table, not a %s", k, child.kind)
+			}
+		} else {
+			child = context.CreateTable(k, false)
+		}
+		context = child
+	}
+
+	if node.Value().Kind == ast.InlineTable {
+		context.SetKind(tableKind)
+	} else {
+		context.SetKind(valueKind)
+	}
+
+	return nil
 }