Slices should append not overwrite

andy-miracl · vdemeester · commit 0b8c27e3f9b3 · 2017-11-08T11:26:14.000+01:00
diff --git a/merge.go b/merge.go
@@ -87,6 +87,18 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, ov
 					if err = deepMerge(dstElement, srcElement, visited, depth+1, overwrite); err != nil {
 						return
 					}
+				case reflect.Slice:
+					srcSlice := reflect.ValueOf(srcElement.Interface())
+
+					var dstSlice reflect.Value
+					if !dstElement.IsValid() || dstElement.IsNil() {
+						dstSlice = reflect.MakeSlice(srcSlice.Type(), 0, srcSlice.Len())
+					} else {
+						dstSlice = reflect.ValueOf(dstElement.Interface())
+					}
+
+					dstSlice = reflect.AppendSlice(dstSlice, srcSlice)
+					dst.SetMapIndex(key, dstSlice)
 				}
 			}
 			if dstElement.IsValid() && reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Map {
@@ -100,6 +112,8 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, ov
 				dst.SetMapIndex(key, srcElement)
 			}
 		}
+	case reflect.Slice:
+		dst.Set(reflect.AppendSlice(dst, src))
 	case reflect.Ptr:
 		fallthrough
 	case reflect.Interface:
diff --git a/mergo_test.go b/mergo_test.go
@@ -225,30 +225,71 @@ func TestPointerStructNil(t *testing.T) {
 	}
 }
 
-func TestSliceStruct(t *testing.T) {
-	a := sliceTest{}
-	b := sliceTest{[]int{1, 2, 3}}
-	if err := Merge(&a, b); err != nil {
+func testSlice(t *testing.T, a []int, b []int) {
+	bc := b
+	e := append(a, b...)
+
+	sa := sliceTest{a}
+	sb := sliceTest{b}
+	if err := Merge(&sa, sb); err != nil {
 		t.FailNow()
 	}
-	if len(b.S) != 3 {
-		t.FailNow()
+	if !reflect.DeepEqual(sb.S, bc) {
+		t.Fatalf("Source slice was modified %d != %d", sb.S, bc)
 	}
-	if len(a.S) != len(b.S) {
-		t.Fatalf("b not merged in a proper way %d != %d", len(a.S), len(b.S))
+	if !reflect.DeepEqual(sa.S, e) {
+		t.Fatalf("b not merged in a proper way %d != %d", sa.S, e)
 	}
 
-	a = sliceTest{[]int{1}}
-	b = sliceTest{[]int{1, 2, 3}}
-	if err := Merge(&a, b); err != nil {
+	ma := map[string][]int{"S": a}
+	mb := map[string][]int{"S": b}
+	if err := Merge(&ma, mb); err != nil {
 		t.FailNow()
 	}
-	if len(a.S) != 1 {
-		t.FailNow()
+	if !reflect.DeepEqual(mb["S"], bc) {
+		t.Fatalf("Source slice was modified %d != %d", mb["S"], bc)
 	}
-	if len(a.S) == len(b.S) {
-		t.Fatalf("b merged unexpectedly %d != %d", len(a.S), len(b.S))
+	if !reflect.DeepEqual(ma["S"], e) {
+		t.Fatalf("b not merged in a proper way %d != %d", ma["S"], e)
 	}
+
+	if a == nil {
+		// test case with missing dst key
+		ma := map[string][]int{}
+		mb := map[string][]int{"S": b}
+		if err := Merge(&ma, mb); err != nil {
+			t.FailNow()
+		}
+		if !reflect.DeepEqual(mb["S"], bc) {
+			t.Fatalf("Source slice was modified %d != %d", mb["S"], bc)
+		}
+		if !reflect.DeepEqual(ma["S"], e) {
+			t.Fatalf("b not merged in a proper way %d != %d", ma["S"], e)
+		}
+	}
+
+	if b == nil {
+		// test case with missing src key
+		ma := map[string][]int{"S": a}
+		mb := map[string][]int{}
+		if err := Merge(&ma, mb); err != nil {
+			t.FailNow()
+		}
+		if !reflect.DeepEqual(mb["S"], bc) {
+			t.Fatalf("Source slice was modified %d != %d", mb["S"], bc)
+		}
+		if !reflect.DeepEqual(ma["S"], e) {
+			t.Fatalf("b not merged in a proper way %d != %d", ma["S"], e)
+		}
+	}
+}
+
+func TestSlice(t *testing.T) {
+	testSlice(t, nil, []int{1, 2, 3})
+	testSlice(t, []int{}, []int{1, 2, 3})
+	testSlice(t, []int{1}, []int{2, 3})
+	testSlice(t, []int{1}, []int{})
+	testSlice(t, []int{1}, nil)
 }
 
 func TestEmptyMaps(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -87,6 +87,18 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, ov`
`87`	`87`	`if err = deepMerge(dstElement, srcElement, visited, depth+1, overwrite); err != nil {`
`88`	`88`	`return`
`89`	`89`	`}`
	`90`	`+ case reflect.Slice:`
	`91`	`+ srcSlice := reflect.ValueOf(srcElement.Interface())`
	`92`	`+`
	`93`	`+ var dstSlice reflect.Value`
	`94`	`+ if !dstElement.IsValid() \|\| dstElement.IsNil() {`
	`95`	`+ dstSlice = reflect.MakeSlice(srcSlice.Type(), 0, srcSlice.Len())`
	`96`	`+ } else {`
	`97`	`+ dstSlice = reflect.ValueOf(dstElement.Interface())`
	`98`	`+ }`
	`99`	`+`
	`100`	`+ dstSlice = reflect.AppendSlice(dstSlice, srcSlice)`
	`101`	`+ dst.SetMapIndex(key, dstSlice)`
`90`	`102`	`}`
`91`	`103`	`}`
`92`	`104`	`if dstElement.IsValid() && reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Map {`
`@@ -100,6 +112,8 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, ov`
`100`	`112`	`dst.SetMapIndex(key, srcElement)`
`101`	`113`	`}`
`102`	`114`	`}`
	`115`	`+ case reflect.Slice:`
	`116`	`+ dst.Set(reflect.AppendSlice(dst, src))`
`103`	`117`	`case reflect.Ptr:`
`104`	`118`	`fallthrough`
`105`	`119`	`case reflect.Interface:`