Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent merge behavior #103

Closed
svyotov opened this issue Feb 28, 2019 · 3 comments · Fixed by #105
Closed

Inconsistent merge behavior #103

svyotov opened this issue Feb 28, 2019 · 3 comments · Fixed by #105

Comments

@svyotov
Copy link
Contributor

svyotov commented Feb 28, 2019

The first test, when merging maps with succeed, while the second, where the same maps are being merged as fields to structs it will fail.
I am not sure if this is intended behavior or not, IMO this is a bug.

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
}

func TestMergoComplexStructMap(t *testing.T) {
	dst := map[string]CustomStruct{
		"Normal": CustomStruct{SomeMap: map[string]string{"key1": "loosethis", "key2": "keepthis"}},
	}
	src := map[string]CustomStruct{
		"Normal": CustomStruct{SomeMap: map[string]string{"key1": "key10"}},
	}
	exp := map[string]CustomStruct{
		"Normal": CustomStruct{SomeMap: map[string]string{"key1": "key10", "key2": "keepthis"}},
	}
	Merge(&dst, src, WithAppendSlice, WithOverride)
	assert.Equal(t, dst, exp)
}
@svyotov
Copy link
Contributor Author

svyotov commented Feb 28, 2019

Found the bug: https://github.com/imdario/mergo/blob/master/merge.go#L142 does not take into consideration Strucs.
It should be:

srcKind := reflect.TypeOf(srcElement.Interface()).Kind()
if dstElement.IsValid() && !isEmptyValue(dstElement) && (srcKind == reflect.Map || srcKind == reflect.Slice || srcKind == reflect.Struct) {

@darccio
Copy link
Owner

darccio commented Jul 17, 2020

Reopening this issue as its PR introduced too many bugs.

@darccio
Copy link
Owner

darccio commented Sep 11, 2023

Closing as the associated PR broke Mergo and some users' projects. I'll make sure it works in v2.

@darccio darccio closed this as completed Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants