Skip to content

Commit

Permalink
fix: fix String() proto text format inconsistencies
Browse files Browse the repository at this point in the history
Fixes: EmptyMessage { } => EmptyMessage {}

Quotes enum values.

Signed-off-by: Christian Stewart <[email protected]>
  • Loading branch information
paralin committed Sep 17, 2024
1 parent fefe4e2 commit c84205a
Show file tree
Hide file tree
Showing 20 changed files with 2,099 additions and 480 deletions.
78 changes: 50 additions & 28 deletions features/text/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ func (g *textGenerator) genMessage(message *protogen.Message) {
// Generate message text marshaling code
g.P("func (x *", message.GoIdent, ") MarshalProtoText() string {")
g.P("var sb ", g.QualifiedGoIdent(stringsPackage.Ident("Builder")))
g.P("sb.WriteString(\"", message.Desc.Name(), " { \")")
g.P("sb.WriteString(\"", message.Desc.Name(), " {\")")

// initialSbLen is the sb contents length before anything is written
initialSbLen := len(message.Desc.Name()) + 2 // +2 for " {"

handledOneOfs := make(map[string]struct{})
for _, field := range message.Fields {
if oneof := field.Oneof; oneof != nil && !field.Desc.HasOptionalKeyword() {
Expand All @@ -100,27 +104,35 @@ func (g *textGenerator) genMessage(message *protogen.Message) {
g.P("switch body := x.", oneof.GoName, ".(type) {")
for _, oneofField := range oneof.Fields {
g.P("case *", oneofField.GoIdent, ":")
g.genField(oneofField, "body."+oneofField.GoName)
g.genField(initialSbLen, oneofField, "body."+oneofField.GoName)
}
g.P("}")
} else {
accessor := "x." + field.GoName
g.genField(field, accessor)
g.genField(initialSbLen, field, accessor)
}
}
g.P("sb.WriteString(\"}\")")
g.P("return sb.String()")
g.P("}")

g.P()

g.P("func (x *", message.GoIdent, ") String() string {")
g.P("return x.MarshalProtoText()")
g.P("}")
}
func (g *textGenerator) genField(sbInitialLen int, field *protogen.Field, accessor string) {
maybeAddSpace := func() {
g.P("if sb.Len() > ", sbInitialLen, " {")
g.P("sb.WriteString(\" \")")
g.P("}")
}

func (g *textGenerator) genField(field *protogen.Field, accessor string) {
if field.Desc.IsList() {
g.P("if len(", accessor, ") > 0 {")
g.P("sb.WriteString(\" ", field.Desc.Name(), ": [\")")
maybeAddSpace()
g.P("sb.WriteString(\"", field.Desc.Name(), ": [\")")
g.P("for i, v := range ", accessor, " {")
g.P("if i > 0 {")
g.P("sb.WriteString(\", \")")
Expand All @@ -136,53 +148,58 @@ func (g *textGenerator) genField(field *protogen.Field, accessor string) {
case protoreflect.MessageKind:
if field.Desc.IsMap() {
g.P("if len(", accessor, ") > 0 {")
g.P("sb.WriteString(\" ", field.Desc.Name(), ": {\")")
maybeAddSpace()
g.P("sb.WriteString(\"", field.Desc.Name(), ": {\")")
g.P("for k, v := range ", accessor, " {")
g.P("sb.WriteString(\" \")")
g.genFieldValue(field.Message.Fields[0], "k", false)
g.P("sb.WriteString(\": \")")
g.genFieldValue(field.Message.Fields[1], "v", false)
g.P("}")
g.P("sb.WriteString(\" }\")")
g.P("}")
} else {
g.P("if ", accessor, " != nil {")
g.P("sb.WriteString(\" ", field.Desc.Name(), ": \")")
maybeAddSpace()
g.P("sb.WriteString(\"", field.Desc.Name(), ": \")")
g.P("sb.WriteString(", accessor, ".MarshalProtoText())")
g.P("}")
}
case protoreflect.StringKind:
case protoreflect.StringKind, protoreflect.BytesKind:
if field.Desc.HasOptionalKeyword() || g.file.Desc.Syntax() == protoreflect.Proto2 {
g.P("if ", accessor, " != nil {")
} else {
g.P("if ", accessor, " != \"\" {")
emptyCheck := "\"\""
if field.Desc.Kind() == protoreflect.BytesKind {
emptyCheck = "nil"
}
g.P("if ", accessor, " != ", emptyCheck, " {")
}
g.P("sb.WriteString(\" ", field.Desc.Name(), ": \")")
maybeAddSpace()
g.P("sb.WriteString(\"", field.Desc.Name(), ": \")")
g.genFieldValue(field, accessor, false)
case protoreflect.EnumKind, protoreflect.Int32Kind, protoreflect.Int64Kind, protoreflect.Sint32Kind, protoreflect.Sint64Kind,
protoreflect.Sfixed32Kind, protoreflect.Sfixed64Kind, protoreflect.Uint32Kind, protoreflect.Uint64Kind,
protoreflect.Fixed32Kind, protoreflect.Fixed64Kind, protoreflect.FloatKind, protoreflect.DoubleKind:
if field.Desc.HasOptionalKeyword() || g.file.Desc.Syntax() == protoreflect.Proto2 {
g.P("if ", accessor, " != nil {")
} else {
g.P("if ", accessor, " != 0 {")
g.P("}")
case protoreflect.EnumKind, protoreflect.Int32Kind, protoreflect.Int64Kind,
protoreflect.Sint32Kind, protoreflect.Sint64Kind, protoreflect.Sfixed32Kind, protoreflect.Sfixed64Kind,
protoreflect.Uint32Kind, protoreflect.Uint64Kind, protoreflect.Fixed32Kind, protoreflect.Fixed64Kind,
protoreflect.FloatKind, protoreflect.DoubleKind, protoreflect.BoolKind:
zeroValue := "0"
if field.Desc.Kind() == protoreflect.BoolKind {
zeroValue = "false"
}
g.P("sb.WriteString(\" ", field.Desc.Name(), ": \")")
g.genFieldValue(field, accessor, false)
case protoreflect.BytesKind:
g.P("if len(", accessor, ") > 0 {")
g.P("sb.WriteString(\" ", field.Desc.Name(), ": \")")
g.genFieldValue(field, accessor, false)
case protoreflect.BoolKind:
if field.Desc.HasOptionalKeyword() || g.file.Desc.Syntax() == protoreflect.Proto2 {
g.P("if ", accessor, " != nil {")
} else {
g.P("if ", accessor, " {")
g.P("if ", accessor, " != ", zeroValue, " {")
}
g.P("sb.WriteString(\" ", field.Desc.Name(), ": \")")
maybeAddSpace()
g.P("sb.WriteString(\"", field.Desc.Name(), ": \")")
g.genFieldValue(field, accessor, false)
g.P("}")
default:
// For unsupported field types, do nothing
return
}
g.P("}")
}

func (g *textGenerator) genFieldValue(field *protogen.Field, accessor string, isList bool) {
Expand All @@ -202,11 +219,16 @@ func (g *textGenerator) genFieldValue(field *protogen.Field, accessor string, is
g.P("sb.WriteString(\"\\\"\")")
case protoreflect.EnumKind:
if isPointer {
g.P("sb.WriteString(\"\\\"\")")
g.P("sb.WriteString(", accessor, ".String())")
g.P("sb.WriteString(\"\\\"\")")
} else {
g.P("sb.WriteString(\"\\\"\")")
g.P("sb.WriteString(", field.Enum.GoIdent, "(", accessor, ").String())")
g.P("sb.WriteString(\"\\\"\")")
}
case protoreflect.Int32Kind, protoreflect.Int64Kind, protoreflect.Sint32Kind, protoreflect.Sint64Kind, protoreflect.Sfixed32Kind, protoreflect.Sfixed64Kind:
case protoreflect.Int32Kind, protoreflect.Int64Kind, protoreflect.Sint32Kind, protoreflect.Sint64Kind,
protoreflect.Sfixed32Kind, protoreflect.Sfixed64Kind:
if isPointer {
g.P("sb.WriteString(", g.QualifiedGoIdent(strconvPackage.Ident("FormatInt")), "(int64(*", accessor, "), 10))")
} else {
Expand Down
2 changes: 1 addition & 1 deletion hack/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ require (
golang.org/x/sync v0.7.0 // indirect
golang.org/x/sys v0.19.0 // indirect
golang.org/x/text v0.14.0 // indirect
google.golang.org/protobuf v1.34.1 // indirect
google.golang.org/protobuf v1.34.2 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions hack/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -909,8 +909,8 @@ google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGj
google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg=
google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
Loading

0 comments on commit c84205a

Please sign in to comment.