jmattheis / goverter

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

Allow goverter to consider underlying types when mapping named types #78

Closed ewhauser closed 9 months ago

ewhauser commented 1 year ago

Is your feature request related to a problem? Please describe.

We currently use Goverter to do a lot of conversion from Protocol Buffers to sqlc. The biggest annoyance we have is that we have to write conversion methods for every enum, a la:

func ProgramStatusEnumToProgramStatus(v gensql.ProgramStatusEnum) pb.Program_ProgramStatus {
    if v == "" {
        return pb.Program_ProgramStatus(0)
    }
    return pb.Program_ProgramStatus(Program_ProgramStatus_value[string(v)])
}

We wrote a Protocol Buffer compiler plugin to start automatically generating methods to convert enums to/from strings to make integration with goverter easier:

func ProgramStatusEnumToProgramStatus(v gensql.ProgramStatusEnum) pb.Program_ProgramStatus {
    return pb.ConvertStringToProgram_ProgramStatus(string(v))
}

However, we still have to write these extend methods because Goverter doesn't recognize that the underlying type of gensql.ProgramStatusEnum is a string.

Describe the solution you'd like

I think Goverter's default behavior is totally reasonable here - just use the named type and don't look at the underlying type. However, in this case it would remove a lot of pain for our team if goverter would consider the underlying type when doing the conversion mappings as we already have converter methods available. I imagine this would need to be opt-in, otherwise it could break existing users.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

We could try to code generate functions to convert our Protocol Buffers to sqlc models directly, but unfortunately that would require a ton of additional mapping. Alternatively, we could hack sqlc to not use named types for enums but they are often useful.

jmattheis commented 1 year ago

To summarize your problem, you want goverter to do an goverter:extend lookup for converting the underlying type of the source named type to the named target type. So a minimal example would be:

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

func ConvertIntToOutputID(s int) OutputID {
    // do a real conversion
    return OutputID(strconv.Itoa(s))
}

type InputID  int
type OutputID string

type Input struct  { ID InputID  }
type Output struct { ID OutputID }

Have I understood this correctly? Could you list some example values for both enums, I think a possible solution could be native enum support for goverter (https://github.com/jmattheis/goverter/issues/61).

ewhauser commented 1 year ago

Yes, that is correct. Here's an example:

// Here is the Protocol Buffer enum implementation
type Task_TaskType int32

const (
    Task_TASK_TYPE_UNSPECIFIED Task_TaskType = 0
    Task_ESCALATION            Task_TaskType = 1
    Task_INDIRECT_REVIEW       Task_TaskType = 2
    Task_OTHER                 Task_TaskType = 3
)

var (
    Task_TaskType_name = map[int32]string{
        0: "TASK_TYPE_UNSPECIFIED",
        1: "ESCALATION",
    }
    Task_TaskType_value = map[string]int32{
        "TASK_TYPE_UNSPECIFIED": 0,
        "ESCALATION":            1,
    }
)

// Here is the sqlc implementation
type TaskType string

const (
    TaskTypeTASKTYPEUNSPECIFIED TaskType = "TASK_TYPE_UNSPECIFIED"
    TaskTypeESCALATION          TaskType = "ESCALATION"
)

// Here are the methods we generate for converting the enums to string
func ConvertStringToTask_TaskType(v string) Task_TaskType {
    if v == "" {
        return Task_TaskType(0)
    }
    return Task_TaskType(Task_TaskType_value[string(v)])
}

func ConvertTask_TaskTypeToString(v Task_TaskType) string {
    return Task_TaskType_name[int32(v)]
}

// Here is the goverter implementation (the enums are nested fields)
// goverter:extend github.com/cadencerpm/monorepo/go/pkg/task:Convert.*
type Converter interface {
    ConvertPbTaskToTask(task *taskpb.Task) (*gensql.InsertTaskParams, error)
}

The issue here is that it is difficult for me to generate converters from TaskType_Type -> TaskType but very easy for me to generate TypeType_Type -> string (and vice versa). If goverter could take the underlying type into account, then this would work.

jmattheis commented 1 year ago

Let me brainstorm a little about this, I currently cannot think of a clean solution and I feel like this still could be solved with a generic enum solution where you only define some general rules how to transform source enum keys / values to the target enum key / values.

jmattheis commented 1 year ago

Kinda hacky, but here is a solution with modification of goverter internals from an external package. It's not possible to use the cli with this, so the parameters have to be configured inside the code. It's probably buggy, but it produced ok'ish looking code for my example above.

Hacky workaround (click me)

```go package main import ( "log" "github.com/dave/jennifer/jen" "github.com/jmattheis/goverter" "github.com/jmattheis/goverter/builder" "github.com/jmattheis/goverter/generator" "github.com/jmattheis/goverter/xtype" ) type HackyUnderlyingBuilder struct{} func (*HackyUnderlyingBuilder) Matches(_ *builder.MethodContext, source, target *xtype.Type) bool { return source.Basic && target.Basic && source.Named && target.Named } func (*HackyUnderlyingBuilder) Build(gen builder.Generator, ctx *builder.MethodContext, sourceID *xtype.JenID, source, target *xtype.Type) ([]jen.Code, *xtype.JenID, *builder.Error) { nextSource := xtype.TypeOf(source.NamedType.Underlying()) nextID := xtype.OtherID(nextSource.TypeAsJen().Call(sourceID.Code)) stmt, id, err := gen.Build(ctx, nextID, nextSource, target, builder.NoWrap) if err == nil { return stmt, id, nil } // try another one nextTarget := xtype.TypeOf(target.NamedType.Underlying()) stmt, id, err = gen.Build(ctx, sourceID, source, nextTarget, builder.NoWrap) if err == nil { return stmt, xtype.OtherID(target.TypeAsJen().Call(id.Code)), nil } // default implementation if source.BasicType.Kind() != target.BasicType.Kind() { return nil, nil, builder.NewError("cannot convert this type :/ this probably needs a better error message") } return nil, xtype.OtherID(target.TypeAsJen().Call(sourceID.Code)), nil } func main() { generator.BuildSteps = append([]builder.Builder{&HackyUnderlyingBuilder{}}, generator.BuildSteps...) err := goverter.GenerateConverterFile("generated/generated.go", goverter.GenerateConfig{ PackageName: "generated", ScanDir: ".", }) if err != nil { log.Fatal(err) } } ```

ewhauser commented 1 year ago

Awesome - thanks. Will give it a try.

Super helpful example of how to extend as well.

jmattheis commented 9 months ago

feature added with v1.1.0. https://goverter.jmattheis.de/#/config/useUnderlyingTypeMethods