protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

compiler/protogen: expose the expected Go type for a message field

Open mcesar opened this issue 3 years ago • 2 comments

Plugins that use protogen might need to get the corresponding Go type of a field. The function fieldGoType already does that, however it is in a internal package. It would be nice if this function were moved to protogen and exported. This would avoid copying and pasting the code.

mcesar avatar Aug 12 '20 23:08 mcesar

\cc @neild.

I vaguely recall some discussion in the past about whether to expose this or not. I don't remember the details, but there are some complexities involved in whether fieldGoType actually returns the expected Go type of the field in certain cases (e.g., for extensions and oneofs). Also, fieldGoType is not a pure function since it mutates the state of protogen.GeneratedFile making it a sharp API with subtle behavior. Any public API would need to be safer to use and will probably involve a noticeable increase in the protogen API surface.

dsnet avatar Aug 13 '20 06:08 dsnet

fieldGoType is super fiddly, and not a good choice for exporting as-is. For example, it returns ("int32", true) for a proto2 int32 field (the bool indicating that the field is actually a *int32), but ("*Message", false) for message fields. This inconsistent reporting of pointer-ness makes sense in the context where it is used, but is not something we would want in a supported external API.

I don't think there's anything fundamentally wrong with us adding an API to protogen that exposes field types, but I don't know what that API would look like.

neild avatar Aug 13 '20 07:08 neild