Forced pointer removal when using 'map . Target' with pointer source

[doreshnikov]

Describe the bug

When instructed to use map . Target with pointer Source, goverter generates a helper converter, stripping away the pointer. This causes go vet to spam warnings when used with Protobuf generated structures, each of which contain a lock inside, resulting in it being copied.

It becomes noticeable with definitions like these:

// goverter:converter
// goverter:output:file ./gen.go
type Converter interface {
	// goverter:map A A2
	Source2TargetBase(s *Source) *TargetBase
	// goverter:map . Base
	Source2Target(s *Source) *Target
}

To Reproduce

Full minimal example to reproduce:

type Source struct {
	A int
	B int
}
type TargetBase struct {
	A int
}
type Target struct {
	Base TargetBase
	B    int
}

// goverter:converter
// goverter:output:file ./gen.go
type Converter interface {
	Source2TargetBase(s *Source) *TargetBase
	// goverter:map . Base
	Source2Target(s *Source) *Target
}

Generates the following code, which, as can be seen, even ignores the already defined Source2Target in favor of a generated helper with stripped pointer on the source:

type ConverterImpl struct{}

func (c *ConverterImpl) Source2Target(source *Source) *Target {
	var pInternalTarget *Target
	if source != nil {
		var internalTarget Target
		internalTarget.Base = c.internalSourceToInternalTargetBase((*source))
		internalTarget.B = (*source).B
		pInternalTarget = &internalTarget
	}
	return pInternalTarget
}
func (c *ConverterImpl) Source2TargetBase(source *Source) *TargetBase {
	var pInternalTargetBase *TargetBase
	if source != nil {
		internalTargetBase := c.internalSourceToInternalTargetBase((*source))
		pInternalTargetBase = &internalTargetBase
	}
	return pInternalTargetBase
}
func (c *ConverterImpl) internalSourceToInternalTargetBase(source Source) TargetBase {
	var internalTargetBase TargetBase
	internalTargetBase.A = source.A
	return internalTargetBase
}

Expected behavior

Preserve the exact same source type when auto-generating helpers while using map .. OR at least prefer an already defined method with pointer receiver instead of generating a new one.

[jmattheis]

Hmm, I'd say this currently works as intended, I think protobuf is an outlier here, by having locks inside inside their structure.

Preserve the exact same source type when auto-generating helpers while using map .

For the protobuf example this may work, but I don't think this should be the default behavior. As this may have some unexpected side effects from a user perspective. E.g. another nil check even if the source is guaranteed to be non-nil.

OR at least prefer an already defined method with pointer receiver instead of generating a new one.

I feel like the generation should be consistent and not use different types in the generation when a certain method signature exists.

---

I'm okay with enabling the behavior "generate Pointer Source to Target Type if map . is used" via a setting. Tho I'm not sure how to call this yet.

[doreshnikov]

Thank you for your reply!

I believe you are right that making this a default behaviour might not be a fitting solution, but I very much appreciate the consideration of a setting that enables it.

Also, an alternative way of looking at it (solving my exact problem but with different approach) might be allowing using other Converter methods when specifying the exact transformation function (map . Target | otherConverterMethod).
This way anyone who desires a pointer preservation would be able to define their pointer-to-pointer method on Converter interface and then just explicitly instruct map to use it.

[jmattheis]

This should already be possible when using https://goverter.jmattheis.de/guide/format#variables-to-assign-variable. Currently, it's not possible to define conversion methods with the same signature (source+target type), therefore this feature would basically only be useful for this usecase. I rather do this as explicit feature/setting where the user doesn't have to manually define the extra pointer method to use it in goverter:map.