lang: types: Clear map/list types during Into()

Map and list types are now unconditionally initialised during an Into()
call to ensure that the only data within them after the operation is
that added by the Into() function.

Prior to this change, map/list types would likely not be cleared prior
to the data being inserted into them with a few exceptions. Nil
pointers or maps/lists that were not sufficient in capacity would be
reinitialised and used to replace the existing backing data store. In
some cases this wouldn't occur meaning any residual data existing in the
container before the Into() call could persist after the data copy
completes. This behaviour is wildly inconsistent and not ideal in the
vast majority of cases. It should be assumed that the Into() call will
preserve nothing and always produce a consistent and deterministic
output.

Signed-off-by: Joe Groocock <me@frebib.net>

lang/types/value.go

@@ -238,6 +238,12 @@ func ValueOf(v reflect.Value) (Value, error) {
 }
 
 // Into mutates the given reflect.Value with the data represented by the Value.
+//
+// Container types like map/list (and to a certain extent structs) will be
+// cleared before adding the contained data such that the existing data doesn't
+// affect the outcome, and the output reflect.Value directly maps to the input
+// Value.
+//
 // In almost every case, it is likely that the reflect.Value will be modified,
 // instantiating nil pointers and even potentially partially filling data before
 // returning an error. It should be assumed that if this returns an error, the
@@ -323,16 +329,24 @@ func Into(v Value, rv reflect.Value) error {
 		return nil
 
 	case *ListValue:
-		if err := mustInto(reflect.Slice, reflect.Array); err != nil {
-			return err
-		}
-
 		count := len(v.V)
-		if count > rv.Cap() {
+
+		switch kind {
+		case reflect.Slice:
 			pow := nextPowerOfTwo(uint32(count))
 			nval := reflect.MakeSlice(rv.Type(), count, int(pow))
 			rv.Set(nval)
+
+		case reflect.Array:
+			if count > rv.Len() {
+				return fmt.Errorf("%+v is too small for %+v", typ, v)
+			}
+			rv.Set(reflect.New(typ).Elem())
+
+		default:
+			return mustInto() // nothing, always returns err
 		}
+
 		for i, x := range v.V {
 			f := rv.Index(i)
 			el := reflect.New(f.Type()).Elem()
@@ -348,9 +362,7 @@ func Into(v Value, rv reflect.Value) error {
 			return err
 		}
 
-		if rv.IsNil() {
-			rv.Set(reflect.MakeMapWithSize(typ, len(v.V)))
-		}
+		rv.Set(reflect.MakeMapWithSize(typ, len(v.V)))
 
 		// convert both key and value, then set them in the map
 		for mk, mv := range v.V {

lang/types/value_test.go

@@ -781,7 +781,7 @@ func TestValueInto0(t *testing.T) {
 	var l []string
 	var ll [][]string
 	var lptrlptr []*[]*string
-	var arr [10]string
+	var arr [3]string
 
 	var m map[string]int
 
@@ -860,7 +860,7 @@ func TestValueInto0(t *testing.T) {
 		{ // arrays are pretty much the same as slices
 			container: &arr,
 			value:     mustValue([]string{"1", "2", "3"}),
-			compare:   [10]string{"1", "2", "3"},
+			compare:   [3]string{"1", "2", "3"},
 		},
 		{
 			container: &ll,
@@ -894,6 +894,11 @@ func TestValueInto0(t *testing.T) {
 			value:     mustValue(map[int]int{1: 2, 3: 4}),
 			shouldErr: true,
 		},
+		{ // [4]string  into [3]string
+			container: &arr,
+			value:     mustValue([4]string{"1", "2", "3", "4"}),
+			shouldErr: true,
+		},
 
 		// Pointer and pointer-to-pointer tests
 		{
@@ -1008,6 +1013,65 @@ func TestValueInto0(t *testing.T) {
 		})
 	}
 }
+
+func TestValueInto1(t *testing.T) {
+	testCases := []struct {
+		container interface{}
+		compare   interface{}
+		data      Value
+	}{
+		{
+			container: &[]string{"", "", "three"},
+			compare:   []string{"one", "two"},
+			data: &ListValue{
+				T: NewType("[]str"),
+				V: []Value{&StrValue{V: "one"}, &StrValue{V: "two"}},
+			},
+		},
+		{
+			container: &[3]string{"", "", "three"},
+			compare:   [3]string{"one", "two", ""},
+			data: &ListValue{
+				T: NewType("[]str"),
+				V: []Value{&StrValue{V: "one"}, &StrValue{V: "two"}},
+			},
+		},
+		{
+			container: &map[string]string{"3": "three"},
+			compare:   map[string]string{"1": "one", "2": "two"},
+			data: &MapValue{
+				T: NewType("map{str: str}"),
+				V: map[Value]Value{
+					&StrValue{V: "1"}: &StrValue{V: "one"},
+					&StrValue{V: "2"}: &StrValue{V: "two"},
+				},
+			},
+		},
+	}
+
+	for index, tc := range testCases {
+		name := fmt.Sprintf("test Into() %s #%d", reflect.TypeOf(tc.container).Elem(), index)
+
+		tc := tc
+		t.Run(name, func(t *testing.T) {
+			ctrVal := reflect.ValueOf(tc.container)
+
+			// ensure Into() clears existing elements out of the container
+			if err := Into(tc.data, ctrVal); err != nil {
+				t.Errorf("func Into(%+v, %+v) failed: %s", tc.data, tc.container, err)
+				return
+			}
+
+			// follow the container pointer: (*tc.container).(interface{})
+			ctrPtr := ctrVal.Elem().Interface()
+			if !reflect.DeepEqual(ctrPtr, tc.compare) {
+				t.Errorf("func Into(%+v, %+v) did not clear existing values from the list", tc.data, ctrPtr)
+				return
+			}
+		})
+	}
+}
+
 func TestValueIntoStructNameMapping(t *testing.T) {
 	st := NewStruct(NewType("struct{word str; magic int}"))
 	if err := st.Set("word", &StrValue{V: "zing"}); err != nil {