Merge pull request #257 from ipld/union-fixes-and-more-tests

schema,tests,gen/go: more tests, gen union fixes.

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 x == y
+	}
 	xk, yk := x.Kind(), y.Kind()
 	if xk != yk {
 		return false

datamodel/equal_test.go

@@ -40,6 +40,8 @@ var deepEqualTests = []struct {
 	// Repeated basicnode.New invocations might return different pointers.
 	{"SameNodeDiffPointer", basic.NewString("same"), basic.NewString("same"), true},
 
+	{"NilVsNil", nil, nil, true},
+	{"NilVsNull", nil, datamodel.Null, false},
 	{"SameKindNull", datamodel.Null, datamodel.Null, true},
 	{"DiffKindNull", datamodel.Null, datamodel.Absent, false},
 	{"SameKindBool", basic.NewBool(true), basic.NewBool(true), true},

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