TargetPointer conversion logic can assign zero-value to a pointer variable, causing JSON marshalling to leak go zero values

[childsev]

Describe the bug Goverter can leak zero values when converting from a struct containing non-pointer variables to a struct containing pointer variables. This is undesirable when the target struct is intended to be JSON marshalled to pass to an external application.

The TargetPointer logic does not check whether a source variable is equal to the zero value for a type before it directly assigns the pointer in the target object to the address of the source variable.

https://github.com/jmattheis/goverter/blob/main/builder/pointer.go#L100

To Reproduce

package model

type Source struct {
    Body string
}

type Dest struct {
    Body *string `json:"body,omitempty"`
}

func TestConversion(t *testing.T) {
    src := model.Source{}
    converter := generated.ConverterImpl{}

    converted := converter.ToBody(src)
    assert.Nil(t, converted.Body)

    marshalled, err := json.Marshal(converted)
    require.NoError(t, err)

    assert.Equal(t, "{}", string(marshalled))
}

This test fails because the body in the converted struct is non-nil, and then the marshalled response ends up including the zero value for the body variable.

Error:          Expected nil, but got: (*string)(0xc00002c400)
            Test:           TestConversion

        Error:          Not equal: 
                            expected: "{}"
                            actual  : "{\"body\":\"\"}"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1 +1 @@
                            -{}
                            +{"body":""}
            Test:           TestConversion

Expected behavior I would expect that TargetPointer only assign the pointer to the value of the variable in the Source struct if that variable has a value other than the zero value of the type.

For example

func isValueSet[T comparable](val T) bool {
    var zeroValue T
    return val != zeroValue
}

[jmattheis]

This looks like the inverse of https://goverter.jmattheis.de/reference/useZeroValueOnPointerInconsistency I don't think this should be the default behavior, because I think most user don't expect this behavior when converting a non nilable type to a nilable type.

To make it consistent this would've been done for struct instances too. E.g.

// goverter:converter
type Converter interface {
    Convert(source Input) *Output
}

func test() {
    var c Converter

    c.Convert(Input{}) == nil
}

type Input struct {
    Name string
    Age  int
}

type Output struct {
    Name string
    Age  int
}

I'd say most users won't expect Convert to return nil in this case.

This behavior should be achievable via extend, so I'm not 100% sure I see this inside goverter. So I'd say we wait for comments from other users that want this behavior.

[jmattheis]

Superceded by #160