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

Add support for fields that match go reserved names #74

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
6 changes: 3 additions & 3 deletions Earthfile
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
VERSION 0.7

ARG ALPINE_VERSION=3.19
ARG GO_VERSION=1.22
ARG LINTER_VERSION=v1.57.2
ARG --global ALPINE_VERSION=3.19
ARG --global GO_VERSION=1.22
ARG --global LINTER_VERSION=v1.57.2
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is necessary, is it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might not be necessary, i vaguely recall running into issues with eathly 0.8+ without it i think

FROM golang:$GO_VERSION-alpine$ALPINE_VERSION
WORKDIR /app

Expand Down
3 changes: 0 additions & 3 deletions protoc/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,6 @@ func generateFile(f *protogen.File, plugin *protogen.Plugin, seen map[string]str

// generateFieldMaskPaths generates a FieldMaskPath struct for each proto message which will contain the fieldmask paths
func generateFieldMaskPaths(g *protogen.GeneratedFile, generatedFileImportPath string, message *protogen.Message, currFieldPath string, seen map[string]struct{}, varsGenerator *varsGenerator, maxDepth uint) []generator {
if len(message.Fields) == 0 {
return nil
}
Comment on lines -54 to -56
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this necessary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, without this messages with zero fields have issues

messageName := string(message.Desc.FullName())
if _, exists := seen[messageName]; exists {
return nil
Expand Down
25 changes: 19 additions & 6 deletions protoc/struct_generator.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package protoc

import (
"go/token"

"github.com/iancoleman/strcase"
"google.golang.org/protobuf/compiler/protogen"
)
Expand Down Expand Up @@ -29,17 +31,28 @@ func (x *structGenerator) AddMessageFields(fields ...*protogen.Field) {
x.msgFields = append(x.msgFields, fields...)
}

// safeFieldName generates a valid go identifier for the field name
//
// Additionally the and underscore is appended if the field name is 'fieldPath' or 'prefix'
func safeFieldName(field *protogen.Field) string {
out := strcase.ToLowerCamel(field.GoName)
if token.IsKeyword(out) || out == "fieldPath" || out == "prefix" {
return out + "_"
}
return out
}

// Generate generates a struct with all fieldmask paths functions for the given type.
func (x *structGenerator) Generate(g *protogen.GeneratedFile) {
// generate struct with all fields
g.P("type ", x.name, " struct {")
g.P("fieldPath string")
g.P("prefix string")
for _, field := range x.strFields {
g.P(strcase.ToLowerCamel(field.GoName), " string")
g.P(safeFieldName(field), " string")
}
for _, field := range x.msgFields {
g.P(strcase.ToLowerCamel(field.GoName), " *", getStructName(field.Message))
g.P(safeFieldName(field), " *", getStructName(field.Message))
}
g.P("}")
g.P()
Expand All @@ -57,11 +70,11 @@ func (x *structGenerator) Generate(g *protogen.GeneratedFile) {
g.P("fieldPath: fieldPath,")
g.P("prefix: prefix,")
for _, field := range x.strFields {
g.P(strcase.ToLowerCamel(field.GoName), ": prefix + \"", field.Desc.Name(), "\",")
g.P(safeFieldName(field), ": prefix + \"", field.Desc.Name(), "\",")
}
for _, field := range x.msgFields {
fieldStructNewFunction := getStructNewFunction(field.Message)
g.P(strcase.ToLowerCamel(field.GoName), ": ", fieldStructNewFunction, "(prefix + \"", field.Desc.Name(), "\", maxDepth - 1),")
g.P(safeFieldName(field), ": ", fieldStructNewFunction, "(prefix + \"", field.Desc.Name(), "\", maxDepth - 1),")
}
g.P("}")
g.P("}")
Expand All @@ -70,10 +83,10 @@ func (x *structGenerator) Generate(g *protogen.GeneratedFile) {
// generate receiver methods
g.P("func (x *", x.name, ") String() string { return x.fieldPath }")
for _, field := range x.strFields {
g.P("func (x *", x.name, ") ", field.GoName, "() string { return x.", strcase.ToLowerCamel(field.GoName), "}")
g.P("func (x *", x.name, ") ", field.GoName, "() string { return x.", safeFieldName(field), "}")
}
for _, field := range x.msgFields {
varName := strcase.ToLowerCamel(field.GoName)
varName := safeFieldName(field)
fieldStructNewFunction := getStructNewFunction(field.Message)
g.P("func (x *", x.name, ") ", field.GoName, "() *", getStructName(field.Message), " {")
g.P("if x.", varName, "!= nil {")
Expand Down
41 changes: 41 additions & 0 deletions protos/cases/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,44 @@ message Foo {
TestNestedInternalMessage my_nested_int_msg = 25;
YetAnotherTestNestedExternalMessage my_yet_another_test_nested_external_msg = 26;
}

message TestReservedGoFieldNames {
// These fields are reserved Go field names and should be escaped.
// copied from https://golang.org/ref/spec#Keywords

string break = 2;
string case = 3;
string chan = 4;
string const = 5;
string continue = 6;

string default = 7;
string defer = 8;
string else = 9;
string fallthrough = 10;
string for = 11;

string func = 12;
string go = 13;
string goto = 14;
string if = 15;
string import = 16;

string interface = 17;
string map = 18;
string package = 19;
string range = 20;
string return = 21;

string select = 22;
string struct = 23;
string switch = 24;
string type = 25;
string var = 26;
}

message EmptyMessage {}

message TestMessageWithEmptyMessage {
EmptyMessage empty = 1;
}
2 changes: 2 additions & 0 deletions test/cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ func (s *TestSuite) TestTypes() {
"an external nested message also gets fieldmasks when it's a parent message": {msg: &cases.TestNestedExternalMessage{}},
"an internal nested message also gets fieldmasks when it's a parent message": {msg: &cases.Foo_TestNestedInternalMessage{}},
"an external nested message from another file also gets fieldmasks when it's a parent message": {msg: &cases.YetAnotherTestNestedExternalMessage{}},
"a message with reserved go field names works": {msg: &cases.TestReservedGoFieldNames{}},
"a message with field that is an empty message works": {msg: &cases.TestMessageWithEmptyMessage{}},
"messages with the same name and a different package get fieldmasks (a.Foo, 1/2)": {msg: &a.Foo{}},
"messages with the same name and a different package get fieldmasks (b.Foo, 2/2)": {msg: &b.Foo{}},
"messages from different proto files, in the same package can get fieldmask for 3rd-parties (1/2)": {msg: &thirdpartyimport.FooA{}},
Expand Down
Loading