jmattheis / goverter

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

missing & in generated code #18

Closed kliko9 closed 2 years ago

kliko9 commented 2 years ago

Describe the bug Generation doesn't throw any error and successfully generates invalid code.

To Reproduce

// goverter:converter
type Converter interface {
    // goverter:mapIdentity Address
    ConvertPerson(source Person) (APIPerson, error)

    // goverter:map Name StreetName
    ConvertAddress(source Person) (APIAddress, error)
}

type Person struct {
    Name   string
    Street string
    City   string
}

type APIPerson struct {
    Name    string
    Address *APIAddress
}

type APIAddress struct {
    StreetName string
    City       string
}

Expected behavior The generated code is missing ampersand sign.

type ConverterImpl struct{}

func (c *ConverterImpl) ConvertAddress(source proto.Person) (proto.APIAddress, error) {
    var protoAPIAddress proto.APIAddress
    protoAPIAddress.StreetName = source.Name
    protoAPIAddress.City = source.City
    return protoAPIAddress, nil
}
func (c *ConverterImpl) ConvertPerson(source proto.Person) (proto.APIPerson, error) {
    var protoAPIPerson proto.APIPerson
    protoAPIPerson.Name = source.Name
    pProtoAPIAddress, err := c.protoPersonToPProtoAPIAddress(source)
    if err != nil {
        return protoAPIPerson, err
    }
    protoAPIPerson.Address = pProtoAPIAddress
    return protoAPIPerson, nil
}
func (c *ConverterImpl) protoPersonToPProtoAPIAddress(source proto.Person) (*proto.APIAddress, error) {
    protoAPIAddress, err := c.ConvertAddress(source)
    if err != nil {
        return protoAPIAddress, err
    }
    return &protoAPIAddress, nil
}

protoPersonToPProtoAPIAddress in case of err != nil in returned value & is missing. Btw. why there's doubled P in method's name.

jmattheis commented 2 years ago

Good catch, I can reproduce this. The duplicated P is because it is a pointer of proto.APIAddress.

jmattheis commented 2 years ago

Fixed in v0.6.2, sadly I'm not satisfied with the solution, because it creates an allocation if the conversion errors, I've created another issue for this.