From dab6ec7cbc5ac5c7395e60be3c207455669b3766 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Fri, 4 Jun 2021 01:10:24 +0100
Subject: [PATCH] node/tests: cover more error cases for scalar kinds

In particular, calling non-scalar Node and Assembler methods should
always fail.

While at it, remove some unused code, since unions always have
representation strategies.

Brings up code coverage in bindnode from 72.7% to 75%.
---
 node/bindnode/node.go       |  3 ---
 node/bindnode/repr.go       | 13 +++++--------
 node/tests/schemaScalars.go | 22 +++++++++++++++++++++-
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/node/bindnode/node.go b/node/bindnode/node.go
index f824596..abf92bd 100644
--- a/node/bindnode/node.go
+++ b/node/bindnode/node.go
@@ -1142,9 +1142,6 @@ type _mapIterator struct {
 	keysVal    reflect.Value // non-pointer
 	valuesVal  reflect.Value // non-pointer
 	nextIndex  int
-
-	// these are only used in repr.go
-	reprEnd int
 }
 
 func (w *_mapIterator) Next() (key, value ipld.Node, _ error) {
diff --git a/node/bindnode/repr.go b/node/bindnode/repr.go
index e8f373c..dcd3879 100644
--- a/node/bindnode/repr.go
+++ b/node/bindnode/repr.go
@@ -200,12 +200,17 @@ func (w *_nodeRepr) MapIterator() ipld.MapIterator {
 	case schema.UnionRepresentation_Keyed:
 		itr := (*_node)(w).MapIterator().(*_unionIterator)
 		return (*_unionIteratorRepr)(itr)
+	case nil:
+		return (*_node)(w).MapIterator()
 	default:
 		panic(fmt.Sprintf("TODO: %T", stg))
 	}
 }
 
 func (w *_nodeRepr) ListIterator() ipld.ListIterator {
+	if reprStrategy(w.schemaType) == nil {
+		return (*_node)(w).ListIterator()
+	}
 	return nil
 }
 
@@ -730,9 +735,6 @@ func (w *_unionAssemblerRepr) AssembleKey() ipld.NodeAssembler {
 	switch stg := reprStrategy(w.schemaType).(type) {
 	case schema.UnionRepresentation_Keyed:
 		return (*_unionAssembler)(w).AssembleKey()
-	case nil:
-		asm := (*_unionAssembler)(w).AssembleKey()
-		return (*_assemblerRepr)(asm.(*_assembler))
 	default:
 		panic(fmt.Sprintf("TODO: %T", stg))
 	}
@@ -748,9 +750,6 @@ func (w *_unionAssemblerRepr) AssembleValue() ipld.NodeAssembler {
 		valAsm := (*_unionAssembler)(w).AssembleValue()
 		valAsm = valAsm.(TypedAssembler).Representation()
 		return valAsm
-	case nil:
-		asm := (*_unionAssembler)(w).AssembleValue()
-		return (*_assemblerRepr)(asm.(*_assembler))
 	default:
 		panic(fmt.Sprintf("TODO: %T", stg))
 	}
@@ -768,8 +767,6 @@ func (w *_unionAssemblerRepr) Finish() error {
 	switch stg := reprStrategy(w.schemaType).(type) {
 	case schema.UnionRepresentation_Keyed:
 		return (*_unionAssembler)(w).Finish()
-	case nil:
-		return (*_unionAssembler)(w).Finish()
 	default:
 		panic(fmt.Sprintf("TODO: %T", stg))
 	}
diff --git a/node/tests/schemaScalars.go b/node/tests/schemaScalars.go
index c2fa8b0..46d0b60 100644
--- a/node/tests/schemaScalars.go
+++ b/node/tests/schemaScalars.go
@@ -66,8 +66,15 @@ func SchemaTestScalars(t *testing.T, engine Engine) {
 				t.Run(testName, func(t *testing.T) {
 					nb := np.NewBuilder()
 
+					// Assigning null, a list, or a map, should always fail.
+					err := nb.AssignNull()
+					qt.Assert(t, err, qt.Not(qt.IsNil))
+					_, err = nb.BeginMap(-1)
+					qt.Assert(t, err, qt.Not(qt.IsNil))
+					_, err = nb.BeginList(-1)
+					qt.Assert(t, err, qt.Not(qt.IsNil))
+
 					// Assigning the right value for the kind should succeed.
-					var err error
 					if useAssignNode {
 						np2 := engine.PrototypeByName(testAssign.name)
 						nb2 := np2.NewBuilder()
@@ -118,6 +125,19 @@ func SchemaTestScalars(t *testing.T, engine Engine) {
 						} else {
 							qt.Assert(t, err, qt.Not(qt.IsNil))
 						}
+
+						// Using Node methods which should never
+						// work on scalar kinds.
+
+						_, err = n.LookupByString("foo")
+						qt.Assert(t, err, qt.Not(qt.IsNil))
+						_, err = n.LookupByIndex(3)
+						qt.Assert(t, err, qt.Not(qt.IsNil))
+						qt.Assert(t, n.MapIterator(), qt.IsNil)
+						qt.Assert(t, n.ListIterator(), qt.IsNil)
+						qt.Assert(t, n.Length(), qt.Equals, int64(-1))
+						qt.Assert(t, n.IsAbsent(), qt.IsFalse)
+						qt.Assert(t, n.IsNull(), qt.IsFalse)
 					}
 				})
 			}
-- 
GitLab