Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protoc-gen-go: generate MarshalText/UnmarshalText for enums #866

Open
mvdan opened this issue Jun 7, 2019 · 8 comments
Open

protoc-gen-go: generate MarshalText/UnmarshalText for enums #866

mvdan opened this issue Jun 7, 2019 · 8 comments
Labels
generator-proto-option involves generators respecting a proto option to control generated source output proposal
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Jun 7, 2019

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

It's trivial to go from an enum's integer value to its string representation (name). You can just do enum.String(), which under the hood uses proto.EnumName.

However, it's not possible to go from the name to the integer value with a simple method call.

Describe the solution you'd like

An added method for all enums, like UnmarshalText(text []byte) error. A simpler alternative might be something like what flag.Value has, Set(string) error.

Describe alternatives you've considered

If one has access to the containing struct type, the struct field tag will contain protobuf:"enum=foo.Bar"`, which can be fed to proto.EnumValueMap to obtain the relevant map[string]int32. However, this has a few major disadvantages:

  • The field tag is attached to the containing struct type, so it's unreachable if all you have is an enum value or type
  • The use of proto.EnumValueMap forces a module to require the protobuf module, when encoding.TextUnmarshaler would suffice
  • Even if the two issues above were fixed, it's still unnecessarily complex to go from string to int32, given how trivial it is to go the opposite way.

Additional context

I realise the answer for 99% of users is to simply use your JSON decoder. But I'm not decoding JSON here :)

@mvdan
Copy link
Member Author

mvdan commented Jun 7, 2019

Also, I imagine that this could use https://godoc.org/github.com/golang/protobuf/proto#UnmarshalJSONEnum under the hood, meaning that each method would be just two lines.

@mvdan
Copy link
Member Author

mvdan commented Jun 7, 2019

Sorry, ignore my last comment. This wouldn't use UnmarshalJSONEnum, because that's specific to JSON. This function would accept ENUMTYPE_FOO, but not "ENUMTYPE_FOO".

@dsnet
Copy link
Member

dsnet commented Jul 10, 2019

Currently, binary bloat is increasingly a concern for generated types, and any method added contributes to that problem. It's unlikely that we generate such a method by default, but an option is possibly conceivable.

@dsnet dsnet changed the title Proposal: implement encoding.TextUnmarshaler in enums proposal: protoc-gen-go: generate MarshalText/UnmarshalText for enums Jul 10, 2019
@dsnet dsnet added enhancement generator-proto-option involves generators respecting a proto option to control generated source output proposal and removed enhancement labels Jul 10, 2019
@mvdan
Copy link
Member Author

mvdan commented Jul 11, 2019

Thanks for the reply; I see your point about binary size. An option would be fine, if this adds too much code.

@dsnet dsnet changed the title proposal: protoc-gen-go: generate MarshalText/UnmarshalText for enums protoc-gen-go: generate MarshalText/UnmarshalText for enums Mar 4, 2020
@yousseftelda
Copy link

yousseftelda commented Sep 19, 2020

Any updates? I can take a stab at implementing it.

@puellanivis
Copy link
Collaborator

Yes, with all the typical Open Source caveats anyone may go ahead and take a stab at this.

@neild
Copy link
Contributor

neild commented Sep 21, 2020

It is now possible to do this via protobuf reflection:

var e somepb.Enum
val := e.Descriptor().Values().ByName("VALUE_NAME")
e = (somepb.Enum)(val.Number())

While verbose, manipulating enum values by name is a fairly rare case.

I don't believe we would want to add additional methods to enum types at this time.

@mvdan
Copy link
Member Author

mvdan commented Sep 22, 2020

I agree that reflection is likely enough. I'd be fine with having this issue closed.

@dsnet dsnet added this to the unplanned milestone Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator-proto-option involves generators respecting a proto option to control generated source output proposal
Projects
None yet
Development

No branches or pull requests

5 participants