Don't append slice if not asked to

Fix the case when we don't ask for slice to be appended but are because of `src` and `dst` values. Signed-off-by: Vincent Demeester <[email protected]>
darccio · Jun 8, 2018 · d69ef30 · d69ef30
1 parent 7045960
commit d69ef30
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 18 deletions.
diff --git a/issue66_test.go b/issue66_test.go
@@ -20,6 +20,25 @@ func TestPrivateSlice(t *testing.T) {
 	if err := Merge(&p1, p2); err != nil {
 		t.Fatalf("Error during the merge: %v", err)
 	}
+	if len(p1.PublicStrings) != 3 {
+		t.Error("5 elements should be in 'PublicStrings' field")
+	}
+	if len(p1.privateStrings) != 2 {
+		t.Error("2 elements should be in 'privateStrings' field")
+	}
+}
+
+func TestPrivateSliceWithAppendSlice(t *testing.T) {
+	p1 := PrivateSliceTest66{
+		PublicStrings:  []string{"one", "two", "three"},
+		privateStrings: []string{"four", "five"},
+	}
+	p2 := PrivateSliceTest66{
+		PublicStrings: []string{"six", "seven"},
+	}
+	if err := Merge(&p1, p2, WithAppendSlice); err != nil {
+		t.Fatalf("Error during the merge: %v", err)
+	}
 	if len(p1.PublicStrings) != 5 {
 		t.Error("5 elements should be in 'PublicStrings' field")
 	}

diff --git a/merge.go b/merge.go
@@ -124,7 +124,11 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co
 						dstSlice = reflect.ValueOf(dstElement.Interface())
 					}
 
-					dstSlice = reflect.AppendSlice(dstSlice, srcSlice)
+					if !isEmptyValue(src) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
+						dstSlice = srcSlice
+					} else if config.AppendSlice {
+						dstSlice = reflect.AppendSlice(dstSlice, srcSlice)
+					}
 					dst.SetMapIndex(key, dstSlice)
 				}
 			}
@@ -145,7 +149,7 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co
 		}
 		if !isEmptyValue(src) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
 			dst.Set(src)
-		} else {
+		} else if config.AppendSlice {
 			dst.Set(reflect.AppendSlice(dst, src))
 		}
 	case reflect.Ptr:

diff --git a/mergo_test.go b/mergo_test.go
@@ -6,11 +6,12 @@
 package mergo
 
 import (
-	"gopkg.in/yaml.v2"
 	"io/ioutil"
 	"reflect"
 	"testing"
 	"time"
+
+	"gopkg.in/yaml.v2"
 )
 
 type simpleTest struct {
@@ -225,13 +226,13 @@ func TestPointerStructNil(t *testing.T) {
 	}
 }
 
-func testSlice(t *testing.T, a []int, b []int) {
+func testSlice(t *testing.T, a []int, b []int, e []int, opts ...func(*Config)) {
+	t.Helper()
 	bc := b
-	e := append(a, b...)
 
 	sa := sliceTest{a}
 	sb := sliceTest{b}
-	if err := Merge(&sa, sb); err != nil {
+	if err := Merge(&sa, sb, opts...); err != nil {
 		t.FailNow()
 	}
 	if !reflect.DeepEqual(sb.S, bc) {
@@ -243,14 +244,14 @@ func testSlice(t *testing.T, a []int, b []int) {
 
 	ma := map[string][]int{"S": a}
 	mb := map[string][]int{"S": b}
-	if err := Merge(&ma, mb); err != nil {
+	if err := Merge(&ma, mb, opts...); err != nil {
 		t.FailNow()
 	}
 	if !reflect.DeepEqual(mb["S"], bc) {
-		t.Fatalf("Source slice was modified %d != %d", mb["S"], bc)
+		t.Fatalf("map value: 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)
+		t.Fatalf("map value: b not merged in a proper way %d != %d", ma["S"], e)
 	}
 
 	if a == nil {
@@ -261,10 +262,10 @@ func testSlice(t *testing.T, a []int, b []int) {
 			t.FailNow()
 		}
 		if !reflect.DeepEqual(mb["S"], bc) {
-			t.Fatalf("Source slice was modified %d != %d", mb["S"], bc)
+			t.Fatalf("missing dst key: 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)
+			t.Fatalf("missing dst key: b not merged in a proper way %d != %d", ma["S"], e)
 		}
 	}
 
@@ -276,20 +277,25 @@ func testSlice(t *testing.T, a []int, b []int) {
 			t.FailNow()
 		}
 		if !reflect.DeepEqual(mb["S"], bc) {
-			t.Fatalf("Source slice was modified %d != %d", mb["S"], bc)
+			t.Fatalf("missing src key: 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)
+			t.Fatalf("missing src key: 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)
+	testSlice(t, nil, []int{1, 2, 3}, []int{1, 2, 3})
+	testSlice(t, []int{}, []int{1, 2, 3}, []int{1, 2, 3})
+	testSlice(t, []int{1}, []int{2, 3}, []int{1})
+	testSlice(t, []int{1}, []int{}, []int{1})
+	testSlice(t, []int{1}, nil, []int{1})
+	testSlice(t, nil, []int{1, 2, 3}, []int{1, 2, 3}, WithAppendSlice)
+	testSlice(t, []int{}, []int{1, 2, 3}, []int{1, 2, 3}, WithAppendSlice)
+	testSlice(t, []int{1}, []int{2, 3}, []int{1, 2, 3}, WithAppendSlice)
+	testSlice(t, []int{1}, []int{}, []int{1}, WithAppendSlice)
+	testSlice(t, []int{1}, nil, []int{1}, WithAppendSlice)
 }
 
 func TestEmptyMaps(t *testing.T) {