jmattheis / goverter

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

Goverter:ignore Not Respected When Converter Handles Pointers #1

Closed Exeson closed 3 years ago

Exeson commented 3 years ago

Describe the bug The //goverter:ignore comment on the interface method is not respected when the input and output types are pointers. This causes an error to be thrown that the field does not exist on the source when generating a converter for pointer types, even thought the field has been marked as ignored.

I've been looking at the generated code to try and trace down where the instruction is getting dropped, it looks like the internal converter function that is being generated with the case of a pointer to pointer conversion that is named {package}{T1}To{package}{T2} (e.g. govertertestInPetToGoverterTestOutPet) is what is skipping over the ignore instructions.

To Reproduce

package govertertest

type InPet struct {
    Name        string
    Description string
}

type OutPet struct {
    ID          string
    Name        string
    Description string
}

// This converter generates correctly, with the ID field being ignored

// goverter:converter
type PetConverter interface {
    // goverter:ignore ID
    Convert(in InPet) OutPet
}

// This converter causes an error to be thrown

// goverter:converter
type PetPrtConverter interface {
    // goverter:ignore ID
    Convert(in *InPet) *OutPet
}

Expected behavior The ignore field(s) annotation should be respected for pointer conversions the same way that it is handled for regular conversions.

jmattheis commented 3 years ago

Yes, ignore currently does only work on non pointer structs. To work around this issue you can declare the both methods.

// goverter:converter
type PetConverter interface {
    // goverter:ignore ID
    Convert2(in InPet) OutPet

    Convert(in *InPet) *OutPet
}
Exeson commented 3 years ago

Ah great!

I've opened a PR with a change that I think will 'fix' this behaviour, however I don't have a good understanding of the codebase as a whole, so I'm not sure what side effects it creates (although all the existing tests still pass): https://github.com/jmattheis/goverter/pull/2

jmattheis commented 3 years ago

Fixed with #2