jmattheis / goverter

Generate type-safe Go converters by simply defining an interface
https://goverter.jmattheis.de/
MIT License
502 stars 47 forks source link

goverter:extend with interface does not seem to work as expected #109

Closed dropwhile closed 10 months ago

dropwhile commented 10 months ago

Describe the bug goverter:extend with Interface does not seem to work as expected.

To Reproduce

Note: extracted from a larger codebase, so this example isn't quite exact

type RefID struct {
    SomeOpaqueThing // embedded type with a .String() method
}

type Notification struct {
    ID           int
    RefID        RefID
    Message      string
}

// protobuf generated
type PbNotification struct {
    RefId   string           
    Message string                
}

// goverter:converter
// goverter:matchIgnoreCase
// goverter:extend StringerToString
type Converter interface {
    ConvertNotifications(source []*Notification) []*PbNotification

    // goverter:ignoreUnexported
    ConvertNotification(source *Notification) *PbNotification
}

// I also tried (x fmt.Stringer)
func StringerToString(x interface{ String() string }) string {
    return x.String()
}

output

| *Notification
|
|     | Notification
|     |
|     | | RefID
|     | |
source*.RefID
target*.RefId
|     | |
|     | | string
|     |
|     | /PbNotification
|
| */PbNotification

TypeMismatch: Cannot convert RefID to string

Expected behavior I would have expected it to work.

Current work around I am using:

type Converter interface {
    ConvertNotifications(source []*Notification) []*PbNotification

    // goverter:ignoreUnexported
    // goverter:map . RefId | GetRefId
    ConvertNotification(source *Notification) *PbNotification
}

func GetRefId(x *Notification) string {
    return x.RefID.String()
}

It works, but then I have to annotate all methods (there will be many more instances/uses of RefID).

jmattheis commented 10 months ago

Goverter currently doesn't check if the source type is assignable to an interface of an extend method.

In the error message the conversion error occurs for the types RefID -> string, you could define the extend method like this.

// goverter:converter
// goverter:matchIgnoreCase
// goverter:extend StringerToString
type Converter interface {
    ConvertNotifications(source []*Notification) []*PbNotification

    // goverter:ignoreUnexported
    ConvertNotification(source *Notification) *PbNotification
}

func StringerToString(x RefID) string {
    return x.String()
}

This should be generic enough for this use-case. I'm not 100% against supporting interface assignment checks for extend methods, but it needs a better use-case for me to consider integrating it into goverter.

dropwhile commented 10 months ago

Unfortunately RefID in my example is one of many RefID "types" (used so static typing makes it harder to call methods with bad identifier types). I have a workaround for now, and I think my use case is probably a bit unusual or uncommon.

Thanks for the consideration though!