From d7906b71d0e8661fa12acb6b283901171c11511d Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Sat, 2 Mar 2019 04:29:04 +0100 Subject: [PATCH 01/21] add support for Struct in Map --- merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merge.go b/merge.go index 3fb6c64..d4a5b11 100644 --- a/merge.go +++ b/merge.go @@ -146,7 +146,7 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co dst.SetMapIndex(key, dstSlice) } } - if dstElement.IsValid() && !isEmptyValue(dstElement) && (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Map || reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Slice) { + if dstElement.IsValid() && !isEmptyValue(dstElement) && (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Map || reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Slice) || (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Struct) { continue } From 71f1eca902063d11c0c45150433cccb2b9ccef30 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 00:55:39 +0100 Subject: [PATCH 02/21] add unit test for issue 90 and 103 --- issue90_test.go | 55 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 issue90_test.go diff --git a/issue90_test.go b/issue90_test.go new file mode 100644 index 0000000..f8a450f --- /dev/null +++ b/issue90_test.go @@ -0,0 +1,55 @@ +package mergo + +import ( + "testing" + + "github.com/magiconair/properties/assert" +) + +func TestMergoSimpleMap(t *testing.T) { + dst := map[string]string{"key1": "loosethis", "key2": "keepthis"} + src := map[string]string{"key1": "key10"} + exp := map[string]string{"key1": "key10", "key2": "keepthis"} + Merge(&dst, src, WithAppendSlice, WithOverride) + assert.Equal(t, dst, exp) +} + +type CustomStruct struct { + SomeMap map[string]string +} + +var testDataStructMap = []struct { + name string + src map[string]CustomStruct + dst map[string]CustomStruct + exp map[string]CustomStruct +}{ + {name: "Normal", + dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "loosethis", "key2": "keepthis"}}}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10", "key2": "keepthis"}}}, + }, + {name: "Init of struct key", dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{}}}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + }, + {name: "Not Init of struct key", dst: map[string]CustomStruct{}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + }, + {name: "Nil struct key", dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: nil}}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}}, +} + +func TestMergoStructMap(t *testing.T) { + + for _, data := range testDataStructMap { + dst := data.dst + src := data.src + exp := data.exp + + Merge(&dst, src, WithAppendSlice, WithOverride) + assert.Equal(t, dst, exp) + } +} From 5210a2af37264235faffc793fd1e3ecfc6de695f Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 00:59:12 +0100 Subject: [PATCH 03/21] err handling in unit tests --- issue90_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/issue90_test.go b/issue90_test.go index f8a450f..bec43eb 100644 --- a/issue90_test.go +++ b/issue90_test.go @@ -10,7 +10,8 @@ func TestMergoSimpleMap(t *testing.T) { dst := map[string]string{"key1": "loosethis", "key2": "keepthis"} src := map[string]string{"key1": "key10"} exp := map[string]string{"key1": "key10", "key2": "keepthis"} - Merge(&dst, src, WithAppendSlice, WithOverride) + err := Merge(&dst, src, WithAppendSlice, WithOverride) + assert.Equal(t, err, nil) assert.Equal(t, dst, exp) } @@ -49,7 +50,8 @@ func TestMergoStructMap(t *testing.T) { src := data.src exp := data.exp - Merge(&dst, src, WithAppendSlice, WithOverride) + err := Merge(&dst, src, WithAppendSlice, WithOverride) + assert.Equal(t, err, nil) assert.Equal(t, dst, exp) } } From 603beb902edbe7ae5d8da0c137f868904407dc40 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 03:20:46 +0100 Subject: [PATCH 04/21] helper functions --- merge.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/merge.go b/merge.go index d4a5b11..aa20536 100644 --- a/merge.go +++ b/merge.go @@ -19,12 +19,34 @@ func hasExportedField(dst reflect.Value) (exported bool) { if field.Anonymous && dst.Field(i).Kind() == reflect.Struct { exported = exported || hasExportedField(dst.Field(i)) } else { - exported = exported || len(field.PkgPath) == 0 + exported = exported || isFieldExported(field) } } return } +func isFieldExported(field reflect.StructField) bool { + if len(field.PkgPath) > 0 { + return false + } + c := field.Name[0] + if 'a' <= c && c <= 'z' || c == '_' { + return false + } + return true +} + +func isTypeExported(v reflect.Type) bool { + if len(v.PkgPath()) > 0 { + return false + } + c := v.Name()[0] + if 'a' <= c && c <= 'z' || c == '_' { + return false + } + return true +} + type Config struct { Overwrite bool AppendSlice bool @@ -281,3 +303,83 @@ func merge(dst, src interface{}, opts ...func(*Config)) error { } return deepMerge(vDst, vSrc, make(map[uintptr]*visit), 0, config) } + +func handleNil(dst reflect.Value) reflect.Value { + fmt.Println("handlesNil(1.0)", dst, dst.CanSet(), dst.Kind()) + fmt.Println("handleNil(1.0.1)", dst, dst.CanSet(), isReflectNil(dst), isEmptyValue(dst)) + if !dst.CanSet() { + t := reflect.Indirect(reflect.ValueOf(dst)).Type() + dsttmp := reflect.New(t).Elem() + fmt.Println(dsttmp) + initiallize(t, dsttmp) + fmt.Println("dsttmp", dsttmp) + fmt.Println("handleNil(1.2.0.0)", dsttmp, dsttmp.CanSet(), dst, dst.CanSet(), dst.Kind()) + if !isTypeExported(t) { + return dsttmp + } + dsttmp.Set(dst) + dst = dsttmp + fmt.Println("handleNil(1.2.0)", dst, dst.CanSet(), dst.Kind()) + } + fmt.Println("handleNil(1.2)", dst, dst.CanSet(), dst.Kind()) + return dst +} + +func initiallize(t reflect.Type, v reflect.Value) { + // fmt.Println("initializeStruct(1.2.1)", t) + switch t.Kind() { + case reflect.Map: + v.Set(reflect.MakeMap(t)) + case reflect.Slice: + v.Set(reflect.MakeSlice(t, 0, 0)) + case reflect.Chan: + v.Set(reflect.MakeChan(t, 0)) + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + f := v.Field(i) + ft := t.Field(i) + initiallize(ft.Type, f) + } + case reflect.Ptr: + ft := t.Elem() + fv := reflect.New(ft) + initiallize(ft, fv.Elem()) + if isTypeExported(ft) { + v.Set(fv) + } + default: + } +} + +func handleEmpty(dst, src reflect.Value, overwrite, appendSlice, overwriteWithEmptySrc bool) reflect.Value { + fmt.Println("handleEmpty(0)", dst, src, dst.CanSet(), dst.Kind(), src.Kind()) + dst = handleNil(dst) + overwrite = (overwrite && !appendSlice || isEmptyValue(dst)) + fmt.Println("handleEmpty(3.0)", dst, src, dst.CanSet(), dst.Kind(), src.Kind(), "can set: ", dst.CanSet(), "!isEmptyValue: ", !isEmptyValue(src), overwriteWithEmptySrc, overwrite) + if (!isEmptyValue(src) || overwriteWithEmptySrc) && overwrite { + fmt.Println("handleEmpty(3.1)", dst, src) + if dst.CanSet() { + dst.Set(src) + } else { + dst = src + } + } + fmt.Println("handleEmpty(4)", dst, src, dst.CanSet()) + if dst.CanInterface() { + dst = reflect.ValueOf(dst.Interface()) + } + return dst +} + +// IsReflectNil is the reflect value provided nil +func isReflectNil(v reflect.Value) bool { + k := v.Kind() + switch k { + case reflect.Interface, reflect.Slice, reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr: + // Both interface and slice are nil if first word is 0. + // Both are always bigger than a word; assume flagIndir. + return v.IsNil() + default: + return false + } +} From fee887300f8de54e984406ddb50c247c180bb545 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 17:04:24 +0100 Subject: [PATCH 05/21] semi working --- map.go | 7 ++-- merge.go | 110 ++++++++++++++++++++++++++++++++++++------------------- 2 files changed, 76 insertions(+), 41 deletions(-) diff --git a/map.go b/map.go index 3f5afa8..d83258b 100644 --- a/map.go +++ b/map.go @@ -99,11 +99,11 @@ func deepMap(dst, src reflect.Value, visited map[uintptr]*visit, depth int, conf continue } if srcKind == dstKind { - if err = deepMerge(dstElement, srcElement, visited, depth+1, config); err != nil { + if _, err = deepMerge(dstElement, srcElement, visited, depth+1, config); err != nil { return } } else if dstKind == reflect.Interface && dstElement.Kind() == reflect.Interface { - if err = deepMerge(dstElement, srcElement, visited, depth+1, config); err != nil { + if _, err = deepMerge(dstElement, srcElement, visited, depth+1, config); err != nil { return } } else if srcKind == reflect.Map { @@ -157,7 +157,8 @@ func _map(dst, src interface{}, opts ...func(*Config)) error { // To be friction-less, we redirect equal-type arguments // to deepMerge. Only because arguments can be anything. if vSrc.Kind() == vDst.Kind() { - return deepMerge(vDst, vSrc, make(map[uintptr]*visit), 0, config) + _, err := deepMerge(vDst, vSrc, make(map[uintptr]*visit), 0, config) + return err } switch vSrc.Kind() { case reflect.Struct: diff --git a/merge.go b/merge.go index aa20536..b03e7eb 100644 --- a/merge.go +++ b/merge.go @@ -26,21 +26,18 @@ func hasExportedField(dst reflect.Value) (exported bool) { } func isFieldExported(field reflect.StructField) bool { - if len(field.PkgPath) > 0 { - return false - } - c := field.Name[0] - if 'a' <= c && c <= 'z' || c == '_' { - return false - } - return true + return isExportedComponent(field.Name, field.PkgPath) } func isTypeExported(v reflect.Type) bool { - if len(v.PkgPath()) > 0 { + return isExportedComponent(v.Name(), v.PkgPath()) +} + +func isExportedComponent(name, pkgPath string) bool { + if len(pkgPath) > 0 { return false } - c := v.Name()[0] + c := name[0] if 'a' <= c && c <= 'z' || c == '_' { return false } @@ -63,7 +60,8 @@ type Transformers interface { // Traverses recursively both values, assigning src's fields values to dst. // The map argument tracks comparisons that have already been seen, which allows // short circuiting on recursive types. -func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, config *Config) (err error) { +func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, config *Config) (dst reflect.Value, err error) { + dst = dstIn overwrite := config.Overwrite typeCheck := config.TypeCheck overwriteWithEmptySrc := config.overwriteWithEmptyValue @@ -80,7 +78,7 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co typ := dst.Type() for p := seen; p != nil; p = p.next { if p.ptr == addr && p.typ == typ { - return nil + return dst, nil } } // Remember, remember... @@ -97,11 +95,28 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co switch dst.Kind() { case reflect.Struct: if hasExportedField(dst) { + dstCp := reflect.New(dst.Type()).Elem() for i, n := 0, dst.NumField(); i < n; i++ { - if err = deepMerge(dst.Field(i), src.Field(i), visited, depth+1, config); err != nil { + dstField := dst.Field(i) + if !isFieldExported(dst.Type().Field(i)) { + continue + } + if dst.Field(i).IsValid() { + k := dstField.Interface() + dstField = reflect.ValueOf(k) + } + dstField, err = deepMerge(dstField, src.Field(i), visited, depth+1, config) + if err != nil { return } + dstCp.Field(i).Set(dstField) } + if dst.CanSet() { + dst.Set(dstCp) + } else { + dst = dstCp + } + } else { if dst.CanSet() && (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) { dst.Set(src) @@ -109,7 +124,12 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co } case reflect.Map: if dst.IsNil() && !src.IsNil() { - dst.Set(reflect.MakeMap(dst.Type())) + if dst.CanSet() { + dst.Set(reflect.MakeMap(dst.Type())) + } else { + dst = src + return + } } for _, key := range src.MapKeys() { srcElement := src.MapIndex(key) @@ -117,6 +137,12 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co continue } dstElement := dst.MapIndex(key) + if dst.MapIndex(key).IsValid() { + k := dstElement.Interface() + dstElement = reflect.ValueOf(k) + } + // dstElement.Set(reflect.ValueOf()) + switch srcElement.Kind() { case reflect.Chan, reflect.Func, reflect.Map, reflect.Interface, reflect.Slice: if srcElement.IsNil() { @@ -141,9 +167,11 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co dstMapElm = reflect.ValueOf(dstMapElm.Interface()) } } - if err = deepMerge(dstMapElm, srcMapElm, visited, depth+1, config); err != nil { + dstMapElm, err = deepMerge(dstMapElm, srcMapElm, visited, depth+1, config) + if err != nil { return } + dst.SetMapIndex(key, dstMapElm) case reflect.Slice: srcSlice := reflect.ValueOf(srcElement.Interface()) @@ -187,22 +215,26 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co dst.Set(src) } else if config.AppendSlice { if src.Type() != dst.Type() { - return fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type()) + err = fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type()) + return } dst.Set(reflect.AppendSlice(dst, src)) } - case reflect.Ptr: - fallthrough - case reflect.Interface: - if src.IsNil() { + case reflect.Ptr, reflect.Interface: + if isReflectNil(src) { break } if dst.Kind() != reflect.Ptr && src.Type().AssignableTo(dst.Type()) { if dst.IsNil() || overwrite { - if dst.CanSet() && (overwrite || isEmptyValue(dst)) { - dst.Set(src) + if overwrite || isEmptyValue(dst) { + if dst.CanSet() { + dst.Set(src) + } else { + dst = src + } } +<<<<<<< HEAD } break } @@ -212,16 +244,19 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co if dst.CanSet() && (overwrite || isEmptyValue(dst)) { dst.Set(src) } +======= + +>>>>>>> semi working } else if src.Kind() == reflect.Ptr { - if err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { + if dst, err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { return } } else if dst.Elem().Type() == src.Type() { - if err = deepMerge(dst.Elem(), src, visited, depth+1, config); err != nil { + if dst, err = deepMerge(dst.Elem(), src, visited, depth+1, config); err != nil { return } } else { - return ErrDifferentArgumentsTypes + return dst, ErrDifferentArgumentsTypes } break } @@ -229,12 +264,17 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co if dst.CanSet() && (overwrite || isEmptyValue(dst)) { dst.Set(src) } - } else if err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { + // TODO HERE + } else if _, err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { return } + // dst.Set() default: - if dst.CanSet() && (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) { + overwrite := (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) + if dst.CanSet() && overwrite { dst.Set(src) + } else { + dst = src } } @@ -298,30 +338,28 @@ func merge(dst, src interface{}, opts ...func(*Config)) error { if vDst, vSrc, err = resolveValues(dst, src); err != nil { return err } + if !vDst.CanSet() { + return fmt.Errorf("cannot set dst, needs reference") + } if vDst.Type() != vSrc.Type() { return ErrDifferentArgumentsTypes } - return deepMerge(vDst, vSrc, make(map[uintptr]*visit), 0, config) + _, err = deepMerge(vDst, vSrc, make(map[uintptr]*visit), 0, config) + return err } func handleNil(dst reflect.Value) reflect.Value { - fmt.Println("handlesNil(1.0)", dst, dst.CanSet(), dst.Kind()) - fmt.Println("handleNil(1.0.1)", dst, dst.CanSet(), isReflectNil(dst), isEmptyValue(dst)) if !dst.CanSet() { t := reflect.Indirect(reflect.ValueOf(dst)).Type() dsttmp := reflect.New(t).Elem() fmt.Println(dsttmp) initiallize(t, dsttmp) - fmt.Println("dsttmp", dsttmp) - fmt.Println("handleNil(1.2.0.0)", dsttmp, dsttmp.CanSet(), dst, dst.CanSet(), dst.Kind()) if !isTypeExported(t) { return dsttmp } dsttmp.Set(dst) dst = dsttmp - fmt.Println("handleNil(1.2.0)", dst, dst.CanSet(), dst.Kind()) } - fmt.Println("handleNil(1.2)", dst, dst.CanSet(), dst.Kind()) return dst } @@ -352,19 +390,15 @@ func initiallize(t reflect.Type, v reflect.Value) { } func handleEmpty(dst, src reflect.Value, overwrite, appendSlice, overwriteWithEmptySrc bool) reflect.Value { - fmt.Println("handleEmpty(0)", dst, src, dst.CanSet(), dst.Kind(), src.Kind()) dst = handleNil(dst) overwrite = (overwrite && !appendSlice || isEmptyValue(dst)) - fmt.Println("handleEmpty(3.0)", dst, src, dst.CanSet(), dst.Kind(), src.Kind(), "can set: ", dst.CanSet(), "!isEmptyValue: ", !isEmptyValue(src), overwriteWithEmptySrc, overwrite) if (!isEmptyValue(src) || overwriteWithEmptySrc) && overwrite { - fmt.Println("handleEmpty(3.1)", dst, src) if dst.CanSet() { dst.Set(src) } else { dst = src } } - fmt.Println("handleEmpty(4)", dst, src, dst.CanSet()) if dst.CanInterface() { dst = reflect.ValueOf(dst.Interface()) } From 8abe216d4be2b0e9e7682451a491b3d4ee3ea85d Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 17:04:59 +0100 Subject: [PATCH 06/21] remove custom functions --- merge.go | 57 -------------------------------------------------------- 1 file changed, 57 deletions(-) diff --git a/merge.go b/merge.go index b03e7eb..7cd1b71 100644 --- a/merge.go +++ b/merge.go @@ -348,63 +348,6 @@ func merge(dst, src interface{}, opts ...func(*Config)) error { return err } -func handleNil(dst reflect.Value) reflect.Value { - if !dst.CanSet() { - t := reflect.Indirect(reflect.ValueOf(dst)).Type() - dsttmp := reflect.New(t).Elem() - fmt.Println(dsttmp) - initiallize(t, dsttmp) - if !isTypeExported(t) { - return dsttmp - } - dsttmp.Set(dst) - dst = dsttmp - } - return dst -} - -func initiallize(t reflect.Type, v reflect.Value) { - // fmt.Println("initializeStruct(1.2.1)", t) - switch t.Kind() { - case reflect.Map: - v.Set(reflect.MakeMap(t)) - case reflect.Slice: - v.Set(reflect.MakeSlice(t, 0, 0)) - case reflect.Chan: - v.Set(reflect.MakeChan(t, 0)) - case reflect.Struct: - for i := 0; i < v.NumField(); i++ { - f := v.Field(i) - ft := t.Field(i) - initiallize(ft.Type, f) - } - case reflect.Ptr: - ft := t.Elem() - fv := reflect.New(ft) - initiallize(ft, fv.Elem()) - if isTypeExported(ft) { - v.Set(fv) - } - default: - } -} - -func handleEmpty(dst, src reflect.Value, overwrite, appendSlice, overwriteWithEmptySrc bool) reflect.Value { - dst = handleNil(dst) - overwrite = (overwrite && !appendSlice || isEmptyValue(dst)) - if (!isEmptyValue(src) || overwriteWithEmptySrc) && overwrite { - if dst.CanSet() { - dst.Set(src) - } else { - dst = src - } - } - if dst.CanInterface() { - dst = reflect.ValueOf(dst.Interface()) - } - return dst -} - // IsReflectNil is the reflect value provided nil func isReflectNil(v reflect.Value) bool { k := v.Kind() From 94505e7256908ed82c02e109c317ce05e1d11117 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 22:49:09 +0100 Subject: [PATCH 07/21] passing unit tests --- merge.go | 47 ++++++++++++++++++++++++++++++----------------- mergo_test.go | 28 +++++++++------------------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/merge.go b/merge.go index 7cd1b71..f71d20b 100644 --- a/merge.go +++ b/merge.go @@ -11,6 +11,7 @@ package mergo import ( "fmt" "reflect" + "unsafe" ) func hasExportedField(dst reflect.Value) (exported bool) { @@ -99,27 +100,34 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, for i, n := 0, dst.NumField(); i < n; i++ { dstField := dst.Field(i) if !isFieldExported(dst.Type().Field(i)) { + rf := dstCp.Field(i) + rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() + + dstRF := dst.Field(i) + if !dst.Field(i).CanAddr() { + continue + } + + dstRF = reflect.NewAt(dstRF.Type(), unsafe.Pointer(dstRF.UnsafeAddr())).Elem() + + rf.Set(dstRF) continue } - if dst.Field(i).IsValid() { - k := dstField.Interface() - dstField = reflect.ValueOf(k) - } - dstField, err = deepMerge(dstField, src.Field(i), visited, depth+1, config) + dstField, err = deepMerge(dst.Field(i), src.Field(i), visited, depth+1, config) if err != nil { return } dstCp.Field(i).Set(dstField) } + if dst.CanSet() { dst.Set(dstCp) } else { dst = dstCp } - } else { - if dst.CanSet() && (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) { - dst.Set(src) + if (isReflectNil(dst) || overwrite) && (!isEmptyValue(src) || overwriteWithEmptySrc) { + dst = src } } case reflect.Map: @@ -251,6 +259,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, if dst, err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { return } + dst = dst.Addr() } else if dst.Elem().Type() == src.Type() { if dst, err = deepMerge(dst.Elem(), src, visited, depth+1, config); err != nil { return @@ -261,20 +270,24 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, break } if dst.IsNil() || overwrite { - if dst.CanSet() && (overwrite || isEmptyValue(dst)) { - dst.Set(src) + if (overwrite || isEmptyValue(dst)) && (overwriteWithEmptySrc || !isEmptyValue(src)) { + if dst.CanSet() { + dst.Set(src) + } else { + dst = src + } } - // TODO HERE } else if _, err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { return } - // dst.Set() default: - overwrite := (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) - if dst.CanSet() && overwrite { - dst.Set(src) - } else { - dst = src + overwriteFull := (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) + if overwriteFull { + if dst.CanSet() { + dst.Set(src) + } else { + dst = src + } } } diff --git a/mergo_test.go b/mergo_test.go index 1786824..02e9705 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -129,10 +129,10 @@ func TestComplexStruct(t *testing.T) { } func TestComplexStructWithOverwrite(t *testing.T) { - a := complexTest{simpleTest{1}, 1, "do-not-overwrite-with-empty-value"} - b := complexTest{simpleTest{42}, 2, ""} + a := complexTest{St: simpleTest{1}, sz: 1, ID: "do-not-overwrite-with-empty-value"} + b := complexTest{St: simpleTest{42}, sz: 2, ID: ""} - expect := complexTest{simpleTest{42}, 1, "do-not-overwrite-with-empty-value"} + expect := complexTest{St: simpleTest{42}, sz: 1, ID: "do-not-overwrite-with-empty-value"} if err := MergeWithOverwrite(&a, b); err != nil { t.FailNow() } @@ -156,7 +156,7 @@ func TestPointerStruct(t *testing.T) { } type embeddingStruct struct { - embeddedStruct + A embeddedStruct } type embeddedStruct struct { @@ -359,7 +359,7 @@ func TestMapsWithOverwrite(t *testing.T) { } expect := map[string]simpleTest{ "a": {16}, - "b": {}, + "b": {42}, "c": {12}, "d": {61}, "e": {14}, @@ -536,23 +536,13 @@ func TestMergeUsingStructAndMap(t *testing.T) { } func TestMaps(t *testing.T) { m := map[string]simpleTest{ - "a": {}, - "b": {42}, - "c": {13}, - "d": {61}, + "a": {0}, "b": {42}, "c": {13}, "d": {61}, } n := map[string]simpleTest{ - "a": {16}, - "b": {}, - "c": {12}, - "e": {14}, + "a": {16}, "b": {}, "c": {12}, "e": {14}, } expect := map[string]simpleTest{ - "a": {0}, - "b": {42}, - "c": {13}, - "d": {61}, - "e": {14}, + "a": {16}, "b": {42}, "c": {13}, "d": {61}, "e": {14}, } if err := Merge(&m, n); err != nil { @@ -562,7 +552,7 @@ func TestMaps(t *testing.T) { if !reflect.DeepEqual(m, expect) { t.Fatalf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", m, expect) } - if m["a"].Value != 0 { + if m["a"].Value != 16 { t.Fatalf(`n merged in m because I solved non-addressable map values TODO: m["a"].Value(%d) != n["a"].Value(%d)`, m["a"].Value, n["a"].Value) } if m["b"].Value != 42 { From 1c80d3f0ed0de1f8ee8ab0b8f3e820c5bec017c0 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 23:03:37 +0100 Subject: [PATCH 08/21] linting --- merge.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/merge.go b/merge.go index f71d20b..9b7e823 100644 --- a/merge.go +++ b/merge.go @@ -30,10 +30,6 @@ func isFieldExported(field reflect.StructField) bool { return isExportedComponent(field.Name, field.PkgPath) } -func isTypeExported(v reflect.Type) bool { - return isExportedComponent(v.Name(), v.PkgPath()) -} - func isExportedComponent(name, pkgPath string) bool { if len(pkgPath) > 0 { return false @@ -113,7 +109,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, rf.Set(dstRF) continue } - dstField, err = deepMerge(dst.Field(i), src.Field(i), visited, depth+1, config) + dstField, err = deepMerge(dstField, src.Field(i), visited, depth+1, config) if err != nil { return } From 4d02cc771360b44eb57a4ed350f0f9f798321971 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Tue, 5 Mar 2019 23:31:36 +0100 Subject: [PATCH 09/21] cleaner test data --- issue90_test.go | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/issue90_test.go b/issue90_test.go index bec43eb..b66f69e 100644 --- a/issue90_test.go +++ b/issue90_test.go @@ -19,33 +19,32 @@ type CustomStruct struct { SomeMap map[string]string } -var testDataStructMap = []struct { - name string - src map[string]CustomStruct - dst map[string]CustomStruct - exp map[string]CustomStruct -}{ - {name: "Normal", - dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "loosethis", "key2": "keepthis"}}}, - src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, - exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10", "key2": "keepthis"}}}, - }, - {name: "Init of struct key", dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{}}}, - src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, - exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, - }, - {name: "Not Init of struct key", dst: map[string]CustomStruct{}, - src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, - exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, - }, - {name: "Nil struct key", dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: nil}}, - src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, - exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}}, -} - func TestMergoStructMap(t *testing.T) { + var testData = []struct { + name string + src map[string]CustomStruct + dst map[string]CustomStruct + exp map[string]CustomStruct + }{ + {name: "Normal", + dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "loosethis", "key2": "keepthis"}}}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10", "key2": "keepthis"}}}, + }, + {name: "Init of struct key", dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{}}}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + }, + {name: "Not Init of struct key", dst: map[string]CustomStruct{}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + }, + {name: "Nil struct key", dst: map[string]CustomStruct{"a": CustomStruct{SomeMap: nil}}, + src: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}, + exp: map[string]CustomStruct{"a": CustomStruct{SomeMap: map[string]string{"key1": "key10"}}}}, + } - for _, data := range testDataStructMap { + for _, data := range testData { dst := data.dst src := data.src exp := data.exp From 697a69379eab6dca3da338f72aa62bdaa598d4b2 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Wed, 6 Mar 2019 00:44:57 +0100 Subject: [PATCH 10/21] fix dstElement.IsNil as it panics when dstElement is string --- merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merge.go b/merge.go index 9b7e823..dca07de 100644 --- a/merge.go +++ b/merge.go @@ -180,7 +180,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, srcSlice := reflect.ValueOf(srcElement.Interface()) var dstSlice reflect.Value - if !dstElement.IsValid() || dstElement.IsNil() { + if !dstElement.IsValid() || isReflectNil(dstElement) { dstSlice = reflect.MakeSlice(srcSlice.Type(), 0, srcSlice.Len()) } else { dstSlice = reflect.ValueOf(dstElement.Interface()) From 7e445ff057cdecc48fb04cb5f821b4240a5e5c69 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov Date: Wed, 13 Mar 2019 20:46:23 +0100 Subject: [PATCH 11/21] PR review clean ups --- issue90_test.go | 20 +++++++------------- merge.go | 1 - 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/issue90_test.go b/issue90_test.go index b66f69e..311bf78 100644 --- a/issue90_test.go +++ b/issue90_test.go @@ -1,20 +1,10 @@ package mergo import ( + "reflect" "testing" - - "github.com/magiconair/properties/assert" ) -func TestMergoSimpleMap(t *testing.T) { - dst := map[string]string{"key1": "loosethis", "key2": "keepthis"} - src := map[string]string{"key1": "key10"} - exp := map[string]string{"key1": "key10", "key2": "keepthis"} - err := Merge(&dst, src, WithAppendSlice, WithOverride) - assert.Equal(t, err, nil) - assert.Equal(t, dst, exp) -} - type CustomStruct struct { SomeMap map[string]string } @@ -50,7 +40,11 @@ func TestMergoStructMap(t *testing.T) { exp := data.exp err := Merge(&dst, src, WithAppendSlice, WithOverride) - assert.Equal(t, err, nil) - assert.Equal(t, dst, exp) + if err != nil { + t.Errorf("mergo error was not nil, %v", err) + } + if !reflect.DeepEqual(dst, exp) { + t.Errorf("Actual: %#v did not match \nExpected: %#v", dst, exp) + } } } diff --git a/merge.go b/merge.go index dca07de..194f9b0 100644 --- a/merge.go +++ b/merge.go @@ -145,7 +145,6 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, k := dstElement.Interface() dstElement = reflect.ValueOf(k) } - // dstElement.Set(reflect.ValueOf()) switch srcElement.Kind() { case reflect.Chan, reflect.Func, reflect.Map, reflect.Interface, reflect.Slice: From fce15cb6fcf217b17e5caa7b0b7932062f4637b7 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Thu, 14 Mar 2019 02:38:29 +0100 Subject: [PATCH 12/21] Reduce code complexity --- issue66_test.go | 2 +- merge.go | 107 ++++++++++++++++-------------------------------- mergo_test.go | 23 ++++++++--- 3 files changed, 54 insertions(+), 78 deletions(-) diff --git a/issue66_test.go b/issue66_test.go index 9e4bcce..ebc4276 100644 --- a/issue66_test.go +++ b/issue66_test.go @@ -21,7 +21,7 @@ func TestPrivateSlice(t *testing.T) { t.Fatalf("Error during the merge: %v", err) } if len(p1.PublicStrings) != 3 { - t.Error("5 elements should be in 'PublicStrings' field") + t.Error("3 elements should be in 'PublicStrings' field, when no append") } if len(p1.privateStrings) != 2 { t.Error("2 elements should be in 'privateStrings' field") diff --git a/merge.go b/merge.go index 194f9b0..b8d7299 100644 --- a/merge.go +++ b/merge.go @@ -17,10 +17,8 @@ import ( func hasExportedField(dst reflect.Value) (exported bool) { for i, n := 0, dst.NumField(); i < n; i++ { field := dst.Type().Field(i) - if field.Anonymous && dst.Field(i).Kind() == reflect.Struct { - exported = exported || hasExportedField(dst.Field(i)) - } else { - exported = exported || isFieldExported(field) + if isFieldExported(field) { + return true } } return @@ -68,6 +66,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, if !src.IsValid() { return } + if dst.CanAddr() { addr := dst.UnsafeAddr() h := 17 * addr @@ -89,6 +88,11 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, } } + if dst.IsValid() && src.IsValid() && src.Type() != dst.Type() { + err = fmt.Errorf("cannot append two different types (%s, %s)", src.Kind(), dst.Kind()) + return + } + switch dst.Kind() { case reflect.Struct: if hasExportedField(dst) { @@ -121,11 +125,13 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, } else { dst = dstCp } + return } else { if (isReflectNil(dst) || overwrite) && (!isEmptyValue(src) || overwriteWithEmptySrc) { dst = src } } + case reflect.Map: if dst.IsNil() && !src.IsNil() { if dst.CanSet() { @@ -137,91 +143,52 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, } for _, key := range src.MapKeys() { srcElement := src.MapIndex(key) + dstElement := dst.MapIndex(key) if !srcElement.IsValid() { continue } - dstElement := dst.MapIndex(key) if dst.MapIndex(key).IsValid() { k := dstElement.Interface() dstElement = reflect.ValueOf(k) } - - switch srcElement.Kind() { - case reflect.Chan, reflect.Func, reflect.Map, reflect.Interface, reflect.Slice: - if srcElement.IsNil() { - continue - } - fallthrough - default: - if !srcElement.CanInterface() { - continue - } - switch reflect.TypeOf(srcElement.Interface()).Kind() { - case reflect.Struct: - fallthrough - case reflect.Ptr: - fallthrough - case reflect.Map: - srcMapElm := srcElement - dstMapElm := dstElement - if srcMapElm.CanInterface() { - srcMapElm = reflect.ValueOf(srcMapElm.Interface()) - if dstMapElm.IsValid() { - dstMapElm = reflect.ValueOf(dstMapElm.Interface()) - } - } - dstMapElm, err = deepMerge(dstMapElm, srcMapElm, visited, depth+1, config) - if err != nil { - return - } - dst.SetMapIndex(key, dstMapElm) - case reflect.Slice: - srcSlice := reflect.ValueOf(srcElement.Interface()) - - var dstSlice reflect.Value - if !dstElement.IsValid() || isReflectNil(dstElement) { - dstSlice = reflect.MakeSlice(srcSlice.Type(), 0, srcSlice.Len()) - } else { - dstSlice = reflect.ValueOf(dstElement.Interface()) - } - - if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { - if typeCheck && srcSlice.Type() != dstSlice.Type() { - return fmt.Errorf("cannot override two slices with different type (%s, %s)", srcSlice.Type(), dstSlice.Type()) - } - dstSlice = srcSlice - } else if config.AppendSlice { - if srcSlice.Type() != dstSlice.Type() { - return fmt.Errorf("cannot append two slices with different type (%s, %s)", srcSlice.Type(), dstSlice.Type()) - } - dstSlice = reflect.AppendSlice(dstSlice, srcSlice) - } - dst.SetMapIndex(key, dstSlice) + if isReflectNil(srcElement) { + if overwrite || isReflectNil(dstElement) { + dst.SetMapIndex(key, srcElement) } + continue } - if dstElement.IsValid() && !isEmptyValue(dstElement) && (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Map || reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Slice) || (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Struct) { + if !srcElement.CanInterface() { continue } - if srcElement.IsValid() && ((srcElement.Kind() != reflect.Ptr && overwrite) || !dstElement.IsValid() || isEmptyValue(dstElement)) { - if dst.IsNil() { - dst.Set(reflect.MakeMap(dst.Type())) + if srcElement.CanInterface() { + srcElement = reflect.ValueOf(srcElement.Interface()) + if dstElement.IsValid() { + dstElement = reflect.ValueOf(dstElement.Interface()) } - dst.SetMapIndex(key, srcElement) } + dstElement, err = deepMerge(dstElement, srcElement, visited, depth+1, config) + if err != nil { + return + } + dst.SetMapIndex(key, dstElement) + } case reflect.Slice: - if !dst.CanSet() { - break - } - if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { - dst.Set(src) + newSlice := dst + if (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { + newSlice = src } else if config.AppendSlice { if src.Type() != dst.Type() { err = fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type()) return } - dst.Set(reflect.AppendSlice(dst, src)) + newSlice = reflect.AppendSlice(dst, src) + } + if dst.CanSet() { + dst.Set(newSlice) + } else { + dst = newSlice } case reflect.Ptr, reflect.Interface: if isReflectNil(src) { @@ -237,7 +204,6 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, dst = src } } -<<<<<<< HEAD } break } @@ -247,9 +213,6 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, if dst.CanSet() && (overwrite || isEmptyValue(dst)) { dst.Set(src) } -======= - ->>>>>>> semi working } else if src.Kind() == reflect.Ptr { if dst, err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil { return diff --git a/mergo_test.go b/mergo_test.go index 02e9705..23eeb31 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -345,13 +345,13 @@ func TestEmptyToNotEmptyMaps(t *testing.T) { } func TestMapsWithOverwrite(t *testing.T) { - m := map[string]simpleTest{ + dst := map[string]simpleTest{ "a": {}, // overwritten by 16 "b": {42}, // overwritten by 0, as map Value is not addressable and it doesn't check for b is set or not set in `n` "c": {13}, // overwritten by 12 "d": {61}, } - n := map[string]simpleTest{ + src := map[string]simpleTest{ "a": {16}, "b": {}, "c": {12}, @@ -365,12 +365,12 @@ func TestMapsWithOverwrite(t *testing.T) { "e": {14}, } - if err := MergeWithOverwrite(&m, n); err != nil { + if err := MergeWithOverwrite(&dst, src); err != nil { t.Fatalf(err.Error()) } - if !reflect.DeepEqual(m, expect) { - t.Fatalf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", m, expect) + if !reflect.DeepEqual(dst, expect) { + t.Fatalf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", dst, expect) } } @@ -885,6 +885,7 @@ func TestBooleanPointer(t *testing.T) { } func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { +<<<<<<< HEAD testCases := []struct { name string options []func(*Config) @@ -900,6 +901,13 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { []func(*Config){WithOverride, WithTypeCheck}, "cannot override two slices with different type", }, +======= + dst := map[string]interface{}{ + "foo": []string{"a", "b"}, + } + src := map[string]interface{}{ + "foo": []int{1, 2}, +>>>>>>> Reduce code complexity } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -910,10 +918,15 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { "foo": []int{1, 2}, } +<<<<<<< HEAD if err := Merge(&src, &dst, tc.options...); err == nil || !strings.Contains(err.Error(), tc.err) { t.Fatalf("expected %q, got %q", tc.err, err) } }) +======= + if err := Merge(&dst, src, WithOverride, WithAppendSlice); err == nil { + t.Fatal("expected an error, got nothing") +>>>>>>> Reduce code complexity } } From de238daf2c03447f7c3725b26466c80179c91b63 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 01:47:19 +0000 Subject: [PATCH 13/21] clean --- mergo_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/mergo_test.go b/mergo_test.go index 23eeb31..590029e 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -885,7 +885,6 @@ func TestBooleanPointer(t *testing.T) { } func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { -<<<<<<< HEAD testCases := []struct { name string options []func(*Config) @@ -901,13 +900,6 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { []func(*Config){WithOverride, WithTypeCheck}, "cannot override two slices with different type", }, -======= - dst := map[string]interface{}{ - "foo": []string{"a", "b"}, - } - src := map[string]interface{}{ - "foo": []int{1, 2}, ->>>>>>> Reduce code complexity } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -918,15 +910,10 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { "foo": []int{1, 2}, } -<<<<<<< HEAD if err := Merge(&src, &dst, tc.options...); err == nil || !strings.Contains(err.Error(), tc.err) { t.Fatalf("expected %q, got %q", tc.err, err) } }) -======= - if err := Merge(&dst, src, WithOverride, WithAppendSlice); err == nil { - t.Fatal("expected an error, got nothing") ->>>>>>> Reduce code complexity } } From ab195abb13d68ccfa196f87951e028b8898a4bcc Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 01:55:09 +0000 Subject: [PATCH 14/21] improvements --- merge.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/merge.go b/merge.go index b8d7299..04fbd6f 100644 --- a/merge.go +++ b/merge.go @@ -17,18 +17,16 @@ import ( func hasExportedField(dst reflect.Value) (exported bool) { for i, n := 0, dst.NumField(); i < n; i++ { field := dst.Type().Field(i) - if isFieldExported(field) { + if isExportedComponent(field) { return true } } return } -func isFieldExported(field reflect.StructField) bool { - return isExportedComponent(field.Name, field.PkgPath) -} - -func isExportedComponent(name, pkgPath string) bool { +func isExportedComponent(field reflect.StructField) bool { + name := field.Name + pkgPath := field.PkgPath if len(pkgPath) > 0 { return false } @@ -99,7 +97,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, dstCp := reflect.New(dst.Type()).Elem() for i, n := 0, dst.NumField(); i < n; i++ { dstField := dst.Field(i) - if !isFieldExported(dst.Type().Field(i)) { + if !isExportedComponent(dst.Type().Field(i)) { rf := dstCp.Field(i) rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() From 45366999a05a75e7008b80bd53dbc40b0d4f2ecc Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 02:02:07 +0000 Subject: [PATCH 15/21] dst --- merge.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/merge.go b/merge.go index 04fbd6f..1a0cf28 100644 --- a/merge.go +++ b/merge.go @@ -174,7 +174,10 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, } case reflect.Slice: newSlice := dst - if (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { + if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { + if typeCheck && src.Type() != dst.Type() { + return fmt.Errorf("cannot override two slices with different type (%s, %s)", src.Type(), dst.Type()) + } newSlice = src } else if config.AppendSlice { if src.Type() != dst.Type() { From 12ad0571f76fc23689c7565e593b7d6f62acec7e Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 02:05:25 +0000 Subject: [PATCH 16/21] bugs --- merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merge.go b/merge.go index 1a0cf28..265a0ff 100644 --- a/merge.go +++ b/merge.go @@ -176,7 +176,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, newSlice := dst if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice { if typeCheck && src.Type() != dst.Type() { - return fmt.Errorf("cannot override two slices with different type (%s, %s)", src.Type(), dst.Type()) + return dst, fmt.Errorf("cannot override two slices with different type (%s, %s)", src.Type(), dst.Type()) } newSlice = src } else if config.AppendSlice { From 14cd5363c7e2fd5406bc1c64aa5798b61af2accd Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 02:12:29 +0000 Subject: [PATCH 17/21] bugs --- merge.go | 2 +- mergo_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/merge.go b/merge.go index 265a0ff..caa746c 100644 --- a/merge.go +++ b/merge.go @@ -180,7 +180,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, } newSlice = src } else if config.AppendSlice { - if src.Type() != dst.Type() { + if typeCheck && src.Type() != dst.Type() { err = fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type()) return } diff --git a/mergo_test.go b/mergo_test.go index 590029e..cb28e4d 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -893,12 +893,12 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { { "With override and append slice", []func(*Config){WithOverride, WithAppendSlice}, - "cannot append two slices with different type", + "cannot append two different types (slice, slice)", }, { "With override and type check", []func(*Config){WithOverride, WithTypeCheck}, - "cannot override two slices with different type", + "cannot append two different types (slice, slice)", }, } for _, tc := range testCases { From 3d8b05c8670dfac5ee11bc4d64e6b5d3f7901f72 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 02:16:32 +0000 Subject: [PATCH 18/21] improvemenst in speed, no copying --- merge.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/merge.go b/merge.go index caa746c..4889030 100644 --- a/merge.go +++ b/merge.go @@ -17,14 +17,14 @@ import ( func hasExportedField(dst reflect.Value) (exported bool) { for i, n := 0, dst.NumField(); i < n; i++ { field := dst.Type().Field(i) - if isExportedComponent(field) { + if isExportedComponent(&field) { return true } } return } -func isExportedComponent(field reflect.StructField) bool { +func isExportedComponent(field *reflect.StructField) bool { name := field.Name pkgPath := field.PkgPath if len(pkgPath) > 0 { @@ -97,7 +97,8 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, dstCp := reflect.New(dst.Type()).Elem() for i, n := 0, dst.NumField(); i < n; i++ { dstField := dst.Field(i) - if !isExportedComponent(dst.Type().Field(i)) { + structField := dst.Type().Field(i) + if !isExportedComponent(&structField) { rf := dstCp.Field(i) rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() From 590d6ed8bc00dc613f5c88eb3b6ce61f5e3315ff Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 02:43:12 +0000 Subject: [PATCH 19/21] disable gosec for unsafe pointer dereference when trying to access struct unexported data fields --- merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/merge.go b/merge.go index 4889030..da79492 100644 --- a/merge.go +++ b/merge.go @@ -100,14 +100,14 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, structField := dst.Type().Field(i) if !isExportedComponent(&structField) { rf := dstCp.Field(i) - rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() + rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() // no:gosec dstRF := dst.Field(i) if !dst.Field(i).CanAddr() { continue } - dstRF = reflect.NewAt(dstRF.Type(), unsafe.Pointer(dstRF.UnsafeAddr())).Elem() + dstRF = reflect.NewAt(dstRF.Type(), unsafe.Pointer(dstRF.UnsafeAddr())).Elem() // no:gosec rf.Set(dstRF) continue From 612279b32f1f3d539a938c29c9c3962029754863 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 03:05:09 +0000 Subject: [PATCH 20/21] nolint --- merge.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/merge.go b/merge.go index da79492..f54daa8 100644 --- a/merge.go +++ b/merge.go @@ -98,17 +98,16 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, for i, n := 0, dst.NumField(); i < n; i++ { dstField := dst.Field(i) structField := dst.Type().Field(i) + // copy un-exported struct fields if !isExportedComponent(&structField) { rf := dstCp.Field(i) - rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() // no:gosec - + rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.Pointer())).Elem() //nolint:gosec dstRF := dst.Field(i) if !dst.Field(i).CanAddr() { continue } - dstRF = reflect.NewAt(dstRF.Type(), unsafe.Pointer(dstRF.UnsafeAddr())).Elem() // no:gosec - + dstRF = reflect.NewAt(dstRF.Type(), unsafe.Pointer(dstRF.UnsafeAddr())).Elem() //nolint:gosec rf.Set(dstRF) continue } From 8eabc023857faf3288647aeb755ea32fb5990986 Mon Sep 17 00:00:00 2001 From: Stanislav Yotov <29090864+svyotov@users.noreply.github.com> Date: Sun, 12 Jan 2020 03:10:12 +0000 Subject: [PATCH 21/21] bugs --- merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/merge.go b/merge.go index f54daa8..eb87886 100644 --- a/merge.go +++ b/merge.go @@ -101,7 +101,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int, // copy un-exported struct fields if !isExportedComponent(&structField) { rf := dstCp.Field(i) - rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.Pointer())).Elem() //nolint:gosec + rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem() //nolint:gosec dstRF := dst.Field(i) if !dst.Field(i).CanAddr() { continue