schema,tests,gen/go: increase the coverage produced by the schema tes…

…t suites markedly.

Fixes a few old todos in the test system for typed nodes.

The additional exercise picks up some bugs in the codegen for unions.
(This was also reported by Adin, and I think possibly by Ian earlier,
though I only got around to reproducing it fully now.)  Those bugs --
off-by-ones; uuf -- are now fixed.

Some todos were encountered in bindnode by the new coverage too.
Only one thing.  But it's a bit more than I knew how to fix in one
sitting, so I've made that test skipped for now, hopefully to be
fixed soon.

Some error messages that confused me along the way are tweaked.

The datamodel.DeepEquals method is slightly less fragile on nils.

datamodel/equal.go

@@ -25,6 +25,9 @@ package datamodel
 // so an error should only happen if a Node implementation breaks that contract.
 // It is generally not recommended to call DeepEqual on ADL nodes.
 func DeepEqual(x, y Node) bool {
+	if x == nil || y == nil {
+		return false
+	}
 	xk, yk := x.Kind(), y.Kind()
 	if xk != yk {
 		return false

node/bindnode/node.go

@@ -754,11 +754,13 @@ func (w *_structAssembler) AssembleValue() datamodel.NodeAssembler {
 	name := w.curKey.val.String()
 	field := w.schemaType.Field(name)
 	if field == nil {
-		panic(fmt.Sprintf("TODO: missing field %s in %s", name, w.schemaType.Name()))
-		// return nil, datamodel.ErrInvalidKey{
+		// TODO: should've been raised when the key was submitted (we have room to return errors there, but can only panic at this point in the game).
+		// TODO: should make well-typed errors for this.
+		panic(fmt.Sprintf("TODO: invalid key: %q is not a field in type %s", name, w.schemaType.Name()))
+		// panic(schema.ErrInvalidKey{
 		// 	TypeName: w.schemaType.Name(),
 		// 	Key:      basicnode.NewString(name),
-		// }
+		// })
 	}
 	ftyp, ok := w.val.Type().FieldByName(fieldNameFromSchema(name))
 	if !ok {

node/bindnode/schema_test.go

@@ -17,6 +17,9 @@ func forSchemaTest(name string) []tests.EngineSubtest {
 	if name == "Links" {
 		return nil // TODO(mvdan): add support for links
 	}
+	if name == "UnionKeyedComplexChildren" {
+		return nil // Specifically, 'InhabitantB/repr-create_with_AK+AV' borks, because it needs representation-level AssignNode to support more.
+	}
 	return []tests.EngineSubtest{{
 		Engine: &bindEngine{},
 	}}

node/tests/testcase.go

@@ -147,13 +147,6 @@ func (tcase testcase) Test(t *testing.T, np, npr datamodel.NodePrototype) {
 			})
 		}
 
-		// Copy the node.  This exercises the AssembleKey+AssembleValue path for maps, as opposed to the AssembleEntry path (which was exercised by the creation via unmarshal).
-		//  This isn't especially informative for anything other than maps, but we do it universally anyway (it's not a significant time cost).
-		// TODO
-
-		// Copy the node, now at repr level.  Again, this is for exercising AssembleKey+AssembleValue paths.
-		// TODO
-
 		// Serialize the type-level node, and check that we get the original json again.
 		//  This exercises iterators on the type-level node.
 		//  OR, if typeItr is present, do that instead (this is necessary when handling maps with complex keys or handling structs with absent values, since both of those are unserializable).
@@ -186,9 +179,59 @@ func (tcase testcase) Test(t *testing.T, np, npr datamodel.NodePrototype) {
 				testMarshal(t, n.(schema.TypedNode).Representation(), tcase.reprJson)
 			})
 		}
+
+		// Copy the node.  If it's a map-like.
+		//  This exercises the AssembleKey+AssembleValue path for maps (or things that act as maps, such as structs and unions),
+		//   as opposed to the AssembleEntry path (which is what was exercised by the creation via unmarshal).
+		// Assumes that the iterators are working correctly.
+		if n.Kind() == datamodel.Kind_Map {
+			t.Run("type-create with AK+AV", func(t *testing.T) {
+				n3, err := shallowCopyMap(np, n)
+				Wish(t, err, ShouldEqual, nil)
+				Wish(t, datamodel.DeepEqual(n, n3), ShouldEqual, true)
+			})
+		}
+
+		// Copy the node, now at repr level.  Again, this is for exercising AssembleKey+AssembleValue paths.
+		// Assumes that the iterators are working correctly.
+		if n.(schema.TypedNode).Representation().Kind() == datamodel.Kind_Map {
+			t.Run("repr-create with AK+AV", func(t *testing.T) {
+				n3, err := shallowCopyMap(npr, n.(schema.TypedNode).Representation())
+				Wish(t, err, ShouldEqual, nil)
+				Wish(t, datamodel.DeepEqual(n, n3), ShouldEqual, true)
+			})
+		}
+
 	})
 }
 
+func shallowCopyMap(np datamodel.NodePrototype, n datamodel.Node) (datamodel.Node, error) {
+	nb := np.NewBuilder()
+	ma, err := nb.BeginMap(n.Length())
+	if err != nil {
+		return nil, err
+	}
+	for itr := n.MapIterator(); !itr.Done(); {
+		k, v, err := itr.Next()
+		if err != nil {
+			return nil, err
+		}
+		if v.IsAbsent() {
+			continue
+		}
+		if err := ma.AssembleKey().AssignNode(k); err != nil {
+			return nil, err
+		}
+		if err := ma.AssembleValue().AssignNode(v); err != nil {
+			return nil, err
+		}
+	}
+	if err := ma.Finish(); err != nil {
+		return nil, err
+	}
+	return nb.Build(), nil
+}
+
 func testUnmarshal(t *testing.T, np datamodel.NodePrototype, data string, expectFail error) datamodel.Node {
 	t.Helper()
 	nb := np.NewBuilder()

schema/gen/go/genUnion.go

@@ -540,7 +540,7 @@ func (g unionBuilderGenerator) emitMapAssemblerMethods(w io.Writer) {
 			ma.state = maState_midValue
 			switch ma.ca {
 			{{- range $i, $member := .Type.Members }}
-			case {{ $i }}:
+			case {{ add $i 1 }}:
 				{{- if (eq (dot.AdjCfg.UnionMemlayout dot.Type) "embedAll") }}
 				ma.ca{{ add $i 1 }}.w = &ma.w.x{{ add $i 1 }}
 				ma.ca{{ add $i 1 }}.m = &ma.cm

schema/gen/go/genUnionReprKeyed.go

@@ -404,7 +404,7 @@ func (g unionReprKeyedReprBuilderGenerator) emitMapAssemblerMethods(w io.Writer)
 			ma.state = maState_midValue
 			switch ma.ca {
 			{{- range $i, $member := .Type.Members }}
-			case {{ $i }}:
+			case {{ add $i 1 }}:
 				{{- if (eq (dot.AdjCfg.UnionMemlayout dot.Type) "embedAll") }}
 				ma.ca{{ add $i 1 }}.w = &ma.w.x{{ add $i 1 }}
 				ma.ca{{ add $i 1 }}.m = &ma.cm