use isReflectNil instead of isEmptyValue to gate Transformers

[GGabriele]

Hi!

While using this great library, I came across a few issues mostly related to how mergo handles zero-values (#166). Based on the comments from various contributors, I decided to write a custom Transformer to handle my use-case as I wished, but then I realized it wasn't possible, as transformers are executed only if the dst is not an "empty" value (https://github.com/imdario/mergo/blob/master/merge.go#L82-L87). Therefore, we cannot apply any transformer to values like false, 0 etc.

What I'm proposing with this PR is to use isReflectNil instead of isEmptyValue to gate transformers execution.

Following is an example of the usage I'm envisioning:

package mergo_test

import (
	"reflect"
	"testing"

	"github.com/GGabriele/mergo"
)

type asd struct {
	A *bool
}

type boolTransformer struct{}

func Bool(v bool) *bool { return &v }

func (t boolTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
	if typ == reflect.TypeOf(Bool(false)) {
		return func(dst, src reflect.Value) error {
			if dst.CanSet() {
				if src.IsNil() {
					src.Set(dst)
				}
			}
			return nil
		}
	}
	return nil
}

func TestBoolPointers(t *testing.T) {
	// base false, augment true
	base := asd{
		A: Bool(false),
	}
	augment := asd{
		A: Bool(true),
	}
	if err := mergo.Merge(&base, augment, mergo.WithTransformers(boolTransformer{})); err != nil {
		t.Error(err)
	}
	if *base.A != false {
		t.Errorf("1. base.A should be false")
	}

	// base true, augment false
	base = asd{
		A: Bool(true),
	}
	augment = asd{
		A: Bool(false),
	}
	if err := mergo.Merge(&base, augment, mergo.WithTransformers(boolTransformer{})); err != nil {
		t.Error(err)
	}
	if *base.A != true {
		t.Errorf("2. base.A should be true")
	}

	// base empty, augment false
	base = asd{}
	augment = asd{
		A: Bool(false),
	}
	if err := mergo.Merge(&base, augment, mergo.WithTransformers(boolTransformer{})); err != nil {
		t.Error(err)
	}
	if *base.A != false {
		t.Errorf("3. base.A should be false")
	}

	// base empty, augment true
	base = asd{}
	augment = asd{
		A: Bool(true),
	}
	if err := mergo.Merge(&base, augment, mergo.WithTransformers(boolTransformer{})); err != nil {
		t.Error(err)
	}
	if *base.A != true {
		t.Errorf("4. base.A should be true")
	}
}

Let me know what you think!

[sourcelevel-bot]

Hello, @GGabriele! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

[darccio]

@GGabriele LGTM. All tests are green.

[zaquestion]

@imdario Can we merge this, hitting this exact case trying to switch from using wrapper types to proto3 optional types and maintain the merging behavior I had before (can't replicate this transformer #207). We are essentially looking to write an an identical transformer to the author here. Unfortunately we didn't notice this until switching a bunch of stuff over. Don't even need it in a tagged release if that's too much right now I'll happily point at master here rather than a fork.

[zaquestion]

Nevermind my first comment, I found a bug in this change. I've put up an alternate PR (#211) that passes in my environment and the existing mergo tests. Attempting to add a test to catch that case

[darccio]

@zaquestion I'll check it this evening.

[darccio]

@GGabriele Closing this in favour of #211