From fc2a5d1106bb860307d608c9d13b9fd8cdfae788 Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 15 Apr 2019 15:07:26 +0200 Subject: [PATCH 1/2] Fix and rename un-appropriate test, as merging directly 2 slices is not supported. Now this "feature" is explicit demonstrated in this test. Signed-off-by: Silvin Lubecki --- mergo_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mergo_test.go b/mergo_test.go index 1f2ab0b..ad274a9 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -746,11 +746,11 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { } } -func TestMergeSliceDifferentType(t *testing.T) { +func TestMergeSlicesIsNotSupported(t *testing.T) { src := []string{"a", "b"} dst := []int{1, 2} - if err := Merge(&src, &dst, WithOverride, WithAppendSlice); err == nil { - t.Fatal("expected an error, got nothing") + if err := Merge(&src, &dst, WithOverride, WithAppendSlice); err != ErrNotSupported { + t.Fatalf("expected %q, got %q", ErrNotSupported, err) } } From 57ebcc3d88f8e8338c7fe6469d411c2b67a37a9d Mon Sep 17 00:00:00 2001 From: Silvin Lubecki Date: Mon, 15 Apr 2019 15:10:48 +0200 Subject: [PATCH 2/2] Add a new option WithTypeCheck, which ensures that slice types are the same while overriding one by another one. This was the case if used WithAppendSlice and WithOverride together, before commit 54bb0a6f1f6e9aebf8c573b01d9c44890c2c495b. Now to get the same result, one should use WithOverride, WithTypeCheck in conjunction. Signed-off-by: Silvin Lubecki --- merge.go | 14 ++++++++++++-- mergo_test.go | 37 +++++++++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/merge.go b/merge.go index f8de6c5..9c1ba3b 100644 --- a/merge.go +++ b/merge.go @@ -28,6 +28,7 @@ func hasExportedField(dst reflect.Value) (exported bool) { type Config struct { Overwrite bool AppendSlice bool + TypeCheck bool Transformers Transformers overwriteWithEmptyValue bool } @@ -41,6 +42,7 @@ type Transformers interface { // short circuiting on recursive types. func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, config *Config) (err error) { overwrite := config.Overwrite + typeCheck := config.TypeCheck overwriteWithEmptySrc := config.overwriteWithEmptyValue config.overwriteWithEmptyValue = false @@ -129,10 +131,13 @@ func deepMerge(dst, src reflect.Value, visited map[uintptr]*visit, depth int, co } if (!isEmptyValue(src) || overwriteWithEmptySrc) && (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 slice with different type (%s, %s)", 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) } @@ -228,11 +233,16 @@ func WithOverride(config *Config) { config.Overwrite = true } -// WithAppendSlice will make merge append slices instead of overwriting it +// WithAppendSlice will make merge append slices instead of overwriting it. func WithAppendSlice(config *Config) { config.AppendSlice = true } +// WithTypeCheck will make merge check types while overwriting it (must be used with WithOverride). +func WithTypeCheck(config *Config) { + config.TypeCheck = true +} + func merge(dst, src interface{}, opts ...func(*Config)) error { var ( vDst, vSrc reflect.Value diff --git a/mergo_test.go b/mergo_test.go index ad274a9..2b7ba17 100644 --- a/mergo_test.go +++ b/mergo_test.go @@ -8,6 +8,7 @@ package mergo import ( "io/ioutil" "reflect" + "strings" "testing" "time" @@ -734,15 +735,35 @@ func TestBooleanPointer(t *testing.T) { } func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) { - src := map[string]interface{}{ - "foo": []string{"a", "b"}, - } - dst := map[string]interface{}{ - "foo": []int{1, 2}, + testCases := []struct { + name string + options []func(*Config) + err string + }{ + { + "With override and append slice", + []func(*Config){WithOverride, WithAppendSlice}, + "cannot append two slices with different type", + }, + { + "With override and type check", + []func(*Config){WithOverride, WithTypeCheck}, + "cannot override two slices with different type", + }, } - - if err := Merge(&src, &dst, WithOverride, WithAppendSlice); err == nil { - t.Fatal("expected an error, got nothing") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + src := map[string]interface{}{ + "foo": []string{"a", "b"}, + } + dst := map[string]interface{}{ + "foo": []int{1, 2}, + } + + if err := Merge(&src, &dst, tc.options...); err == nil || !strings.Contains(err.Error(), tc.err) { + t.Fatalf("expected %q, got %q", tc.err, err) + } + }) } }