Skip to content

Commit

Permalink
Reduce code complexity
Browse files Browse the repository at this point in the history
  • Loading branch information
svyotov authored Mar 14, 2019
1 parent bd85c25 commit 35276fe
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 74 deletions.
2 changes: 1 addition & 1 deletion issue66_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
99 changes: 34 additions & 65 deletions merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -64,6 +62,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
Expand All @@ -85,6 +84,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) {
Expand Down Expand Up @@ -117,11 +121,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() {
Expand All @@ -133,89 +139,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) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
dstSlice = srcSlice
} else if config.AppendSlice {
if srcSlice.Type() != dstSlice.Type() {
err = fmt.Errorf("cannot append two slice with different type (%s, %s)", srcSlice.Type(), dstSlice.Type())
return
}
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() && (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
}
newSlice := dst
if (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
dst.Set(src)
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) {
Expand Down
16 changes: 8 additions & 8 deletions mergo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,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}, // not overwritten by empty value
"c": {13}, // overwritten by 12
"d": {61},
}
n := map[string]simpleTest{
src := map[string]simpleTest{
"a": {16},
"b": {},
"c": {12},
Expand All @@ -364,12 +364,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)
}
}

Expand Down Expand Up @@ -724,14 +724,14 @@ func TestBooleanPointer(t *testing.T) {
}

func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) {
src := map[string]interface{}{
dst := map[string]interface{}{
"foo": []string{"a", "b"},
}
dst := map[string]interface{}{
src := map[string]interface{}{
"foo": []int{1, 2},
}

if err := Merge(&src, &dst, WithOverride, WithAppendSlice); err == nil {
if err := Merge(&dst, src, WithOverride, WithAppendSlice); err == nil {
t.Fatal("expected an error, got nothing")
}
}
Expand Down

0 comments on commit 35276fe

Please sign in to comment.