Skip to content

Commit ab4615d

Browse files
authored
protoparse: enum value name constraints, ostensibly related to JSON format (#524)
1 parent 3057e78 commit ab4615d

18 files changed

+2848
-2433
lines changed

desc/internal/util.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -219,17 +219,17 @@ const (
219219
)
220220

221221
// JsonName returns the default JSON name for a field with the given name.
222+
// This mirrors the algorithm in protoc:
223+
// https://github.com/protocolbuffers/protobuf/blob/v21.3/src/google/protobuf/descriptor.cc#L95
222224
func JsonName(name string) string {
223225
var js []rune
224226
nextUpper := false
225-
for i, r := range name {
227+
for _, r := range name {
226228
if r == '_' {
227229
nextUpper = true
228230
continue
229231
}
230-
if i == 0 {
231-
js = append(js, r)
232-
} else if nextUpper {
232+
if nextUpper {
233233
nextUpper = false
234234
js = append(js, unicode.ToUpper(r))
235235
} else {

desc/protoparse/linker.go

+118-7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"google.golang.org/protobuf/types/descriptorpb"
77
"sort"
88
"strings"
9+
"unicode"
10+
"unicode/utf8"
911

1012
"github.com/golang/protobuf/proto"
1113
dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
@@ -75,7 +77,7 @@ func (l *linker) linkFiles() (map[string]*desc.FileDescriptor, error) {
7577
if err := l.checkExtensionsInFile(fd, r); err != nil {
7678
return nil, err
7779
}
78-
// and final check for json name configuration
80+
// and final check for json name conflicts
7981
if err := l.checkJsonNamesInFile(fd, r); err != nil {
8082
return nil, err
8183
}
@@ -1122,28 +1124,137 @@ func (l *linker) checkJsonNamesInFile(fd *desc.FileDescriptor, res *parseResult)
11221124
return err
11231125
}
11241126
}
1127+
for _, ed := range fd.GetEnumTypes() {
1128+
if err := l.checkJsonNamesInEnum(ed, res); err != nil {
1129+
return err
1130+
}
1131+
}
11251132
return nil
11261133
}
11271134

11281135
func (l *linker) checkJsonNamesInMessage(md *desc.MessageDescriptor, res *parseResult) error {
1129-
if err := checkJsonNames(md, res, false); err != nil {
1136+
if err := checkFieldJsonNames(md, res, false); err != nil {
11301137
return err
11311138
}
1132-
if err := checkJsonNames(md, res, true); err != nil {
1139+
if err := checkFieldJsonNames(md, res, true); err != nil {
11331140
return err
11341141
}
1142+
1143+
for _, nmd := range md.GetNestedMessageTypes() {
1144+
if err := l.checkJsonNamesInMessage(nmd, res); err != nil {
1145+
return err
1146+
}
1147+
}
1148+
for _, ed := range md.GetNestedEnumTypes() {
1149+
if err := l.checkJsonNamesInEnum(ed, res); err != nil {
1150+
return err
1151+
}
1152+
}
1153+
return nil
1154+
}
1155+
1156+
func (l *linker) checkJsonNamesInEnum(ed *desc.EnumDescriptor, res *parseResult) error {
1157+
seen := map[string]*dpb.EnumValueDescriptorProto{}
1158+
for _, evd := range ed.GetValues() {
1159+
scope := "enum value " + ed.GetName() + "." + evd.GetName()
1160+
1161+
name := canonicalEnumValueName(evd.GetName(), ed.GetName())
1162+
if existing, ok := seen[name]; ok && evd.GetNumber() != existing.GetNumber() {
1163+
fldNode := res.getEnumValueNode(evd.AsEnumValueDescriptorProto())
1164+
existingNode := res.getEnumValueNode(existing)
1165+
isProto3 := ed.GetFile().IsProto3()
1166+
conflictErr := errorWithPos(fldNode.Start(), "%s: camel-case name (with optional enum name prefix removed) %q conflicts with camel-case name of enum value %s, defined at %v",
1167+
scope, name, existing.GetName(), existingNode.Start())
1168+
1169+
// Since proto2 did not originally have a JSON format, we report conflicts as just warnings
1170+
if !isProto3 {
1171+
res.errs.warn(conflictErr)
1172+
} else if err := res.errs.handleError(conflictErr); err != nil {
1173+
return err
1174+
}
1175+
} else {
1176+
seen[name] = evd.AsEnumValueDescriptorProto()
1177+
}
1178+
}
11351179
return nil
11361180
}
11371181

1138-
func checkJsonNames(md *desc.MessageDescriptor, res *parseResult, useCustom bool) error {
1139-
type seenName struct {
1182+
func canonicalEnumValueName(enumValueName, enumName string) string {
1183+
return enumValCamelCase(removePrefix(enumValueName, enumName))
1184+
}
1185+
1186+
// removePrefix is used to remove the given prefix from the given str. It does not require
1187+
// an exact match and ignores case and underscores. If the all non-underscore characters
1188+
// would be removed from str, str is returned unchanged. If str does not have the given
1189+
// prefix (even with the very lenient matching, in regard to case and underscores), then
1190+
// str is returned unchanged.
1191+
//
1192+
// The algorithm is adapted from the protoc source:
1193+
// https://github.com/protocolbuffers/protobuf/blob/v21.3/src/google/protobuf/descriptor.cc#L922
1194+
func removePrefix(str, prefix string) string {
1195+
j := 0
1196+
for i, r := range str {
1197+
if r == '_' {
1198+
// skip underscores in the input
1199+
continue
1200+
}
1201+
1202+
p, sz := utf8.DecodeRuneInString(prefix[j:])
1203+
for p == '_' {
1204+
j += sz // consume/skip underscore
1205+
p, sz = utf8.DecodeRuneInString(prefix[j:])
1206+
}
1207+
1208+
if j == len(prefix) {
1209+
// matched entire prefix; return rest of str
1210+
// but skipping any leading underscores
1211+
result := strings.TrimLeft(str[i:], "_")
1212+
if len(result) == 0 {
1213+
// result can't be empty string
1214+
return str
1215+
}
1216+
return result
1217+
}
1218+
if unicode.ToLower(r) != unicode.ToLower(p) {
1219+
// does not match prefix
1220+
return str
1221+
}
1222+
j += sz // consume matched rune of prefix
1223+
}
1224+
return str
1225+
}
1226+
1227+
// enumValCamelCase converts the given string to upper-camel-case.
1228+
//
1229+
// The algorithm is adapted from the protoc source:
1230+
// https://github.com/protocolbuffers/protobuf/blob/v21.3/src/google/protobuf/descriptor.cc#L887
1231+
func enumValCamelCase(name string) string {
1232+
var js []rune
1233+
nextUpper := true
1234+
for _, r := range name {
1235+
if r == '_' {
1236+
nextUpper = true
1237+
continue
1238+
}
1239+
if nextUpper {
1240+
nextUpper = false
1241+
js = append(js, unicode.ToUpper(r))
1242+
} else {
1243+
js = append(js, unicode.ToLower(r))
1244+
}
1245+
}
1246+
return string(js)
1247+
}
1248+
1249+
func checkFieldJsonNames(md *desc.MessageDescriptor, res *parseResult, useCustom bool) error {
1250+
type jsonName struct {
11401251
source *dpb.FieldDescriptorProto
11411252
// field's original JSON nane (which can differ in case from map key)
11421253
orig string
11431254
// true if orig is a custom JSON name (vs. the field's default JSON name)
11441255
custom bool
11451256
}
1146-
seen := map[string]seenName{}
1257+
seen := map[string]jsonName{}
11471258

11481259
for _, fd := range md.GetFields() {
11491260
scope := "field " + md.GetName() + "." + fd.GetName()
@@ -1187,7 +1298,7 @@ func checkJsonNames(md *desc.MessageDescriptor, res *parseResult, useCustom bool
11871298
}
11881299
}
11891300
} else {
1190-
seen[lcaseName] = seenName{orig: name, source: fd.AsFieldDescriptorProto(), custom: custom}
1301+
seen[lcaseName] = jsonName{source: fd.AsFieldDescriptorProto(), orig: name, custom: custom}
11911302
}
11921303
}
11931304
return nil

0 commit comments

Comments
 (0)