fix: bug where merging slice of interfaces introduced nils in the result

common/deepcopy/merge.go

@@ -62,31 +62,37 @@ func mergeRecursively(xV reflect.Value, yV reflect.Value) reflect.Value {
 	case reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128, reflect.String:
 		return yV
 	case reflect.Array, reflect.Slice:
-		nV := reflect.MakeSlice(xT, xV.Len()+yV.Len(), xV.Cap()+yV.Cap())
-		itemsYetToBeMerged := map[int]bool{}
-		for i := 0; i < yV.Len(); i++ {
-			itemsYetToBeMerged[i] = true
-		}
-		for i := 0; i < xV.Len(); i++ {
-			merged := false
-			for j := 0; j < yV.Len(); j++ {
-				if compare(xV.Index(i), yV.Index(j)) {
-					nV.Index(i).Set(mergeRecursively(xV.Index(i), yV.Index(j)))
+		{
+			// make a copy of xV
+			mergedSlice := copyRecursively(xV)
+			// mergedSlice := reflect.MakeSlice(xT, xV.Len(), xV.Cap())
+			// for xIdx := 0; xIdx < xV.Len(); xIdx++ {
+			// 	mergedSlice.Index(xIdx).Set(copyRecursively(xV.Index(xIdx)))
+			// }
+
+			// for each element in yV
+			for yIdx := 0; yIdx < yV.Len(); yIdx++ {
+				merged := false
+				yVElem := yV.Index(yIdx)
+				// try to merge with an existing element in the merged slice
+				for mergedIdx := 0; mergedIdx < mergedSlice.Len(); mergedIdx++ {
+					existingElem := mergedSlice.Index(mergedIdx)
+					if !compare(existingElem, yVElem) {
+						// can't merge
+						continue
+					}
+					mergedElem := mergeRecursively(existingElem, yVElem)
+					mergedSlice.Index(mergedIdx).Set(mergedElem)
 					merged = true
-					itemsYetToBeMerged[j] = false
 					break
 				}
+				if !merged {
+					// append if we can't merge
+					mergedSlice = reflect.Append(mergedSlice, copyRecursively(yVElem))
+				}
 			}
-			if !merged {
-				nV.Index(i).Set(copyRecursively(xV.Index(i)))
-			}
-		}
-		for i := 0; i < yV.Len(); i++ {
-			if itemsYetToBeMerged[i] {
-				nV.Index(xV.Len() + i).Set(copyRecursively(yV.Index(i)))
-			}
+			return mergedSlice
 		}
-		return nV
 	case reflect.Interface:
 		return mergeRecursively(xV.Elem(), yV.Elem())
 	case reflect.Map:

common/deepcopy/merge_test.go

@@ -0,0 +1,122 @@
+/*
+ *  Copyright IBM Corporation 2023
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package deepcopy_test
+
+import (
+	"testing"
+
+	"github.com/konveyor/move2kube/common/deepcopy"
+)
+
+func TestMerge(t *testing.T) {
+	t.Run("merge interface of slices with different integers", func(t *testing.T) {
+		xs := []interface{}{1}
+		ys := []interface{}{2}
+		wanted := []interface{}{1, 2}
+		zsI := deepcopy.Merge(xs, ys)
+		zs, ok := zsI.([]interface{})
+		if !ok {
+			t.Fatal("expected the merged result to be a slice of interfaces")
+		}
+		if len(zs) != len(wanted) {
+			t.Fatalf("length of the merged result is incorrect. expected: %d actual: %d", len(wanted), len(zs))
+		}
+		for i, z := range zs {
+			zInt, ok := z.(int)
+			if !ok {
+				t.Fatal("expected the elements of the merged result to be integers")
+			}
+			if zInt != wanted[i] {
+				t.Fatalf("the element at index is incorrect. expected: %d actual: %d", wanted[i], zInt)
+			}
+		}
+	})
+	t.Run("merge interface of slices with same integers", func(t *testing.T) {
+		xs := []interface{}{1}
+		ys := []interface{}{1}
+		wanted := []interface{}{1}
+		zsI := deepcopy.Merge(xs, ys)
+		zs, ok := zsI.([]interface{})
+		if !ok {
+			t.Fatal("expected the merged result to be a slice of interfaces")
+		}
+		if len(zs) != len(wanted) {
+			t.Fatalf("length of the merged result is incorrect. expected: %d actual: %d", len(wanted), len(zs))
+		}
+		for i, z := range zs {
+			zInt, ok := z.(int)
+			if !ok {
+				t.Fatal("expected the elements of the merged result to be integers")
+			}
+			if zInt != wanted[i] {
+				t.Fatalf("the element at index is incorrect. expected: %d actual: %d", wanted[i], zInt)
+			}
+		}
+	})
+
+	t.Run("merge interface of slices with duplicate integers in the same slice", func(t *testing.T) {
+		xs := []interface{}{1, 2, 2}
+		ys := []interface{}{0, 2, 3, 2, 1}
+		wanted := []interface{}{1, 2, 2, 0, 3}
+		zsI := deepcopy.Merge(xs, ys)
+		zs, ok := zsI.([]interface{})
+		if !ok {
+			t.Fatal("expected the merged result to be a slice of interfaces")
+		}
+		if len(zs) != len(wanted) {
+			t.Fatalf("length of the merged result is incorrect. expected: %d actual: %d", len(wanted), len(zs))
+		}
+		for i, z := range zs {
+			zInt, ok := z.(int)
+			if !ok {
+				t.Fatal("expected the elements of the merged result to be integers")
+			}
+			if zInt != wanted[i] {
+				t.Fatalf("the element at index is incorrect. expected: %d actual: %d", wanted[i], zInt)
+			}
+		}
+	})
+	t.Run("merge interface of slices with duplicate integers and strings in the same slice", func(t *testing.T) {
+		xs := []interface{}{1, 2, "foo", 2}
+		ys := []interface{}{"foo", 0, "bar", 2, 3, 2, 1}
+		wanted := []interface{}{1, 2, "foo", 2, 0, "bar", 3}
+		zsI := deepcopy.Merge(xs, ys)
+		zs, ok := zsI.([]interface{})
+		if !ok {
+			t.Fatal("expected the merged result to be a slice of interfaces")
+		}
+		if len(zs) != len(wanted) {
+			t.Fatalf("length of the merged result is incorrect. expected: %d actual: %d", len(wanted), len(zs))
+		}
+		for i, z := range zs {
+			zInt, ok := z.(int)
+			if !ok {
+				zStr, ok := z.(string)
+				if !ok {
+					t.Fatal("expected the elements of the merged result to be integers or strings")
+				}
+				if zStr != wanted[i] {
+					t.Fatalf("the element at index is incorrect. expected: %d actual: %d", wanted[i], zInt)
+				}
+				continue
+			}
+			if zInt != wanted[i] {
+				t.Fatalf("the element at index is incorrect. expected: %d actual: %d", wanted[i], zInt)
+			}
+		}
+	})
+}