diff --git a/parser/validate.go b/parser/validate.go index 2118a47a..de2b814a 100644 --- a/parser/validate.go +++ b/parser/validate.go @@ -203,6 +203,14 @@ func validateMessage(res *result, isProto3 bool, name protoreflect.FullName, md // or reserved ranges or reserved names rsvdNames := map[string]struct{}{} for _, n := range md.ReservedName { + // validate reserved name while we're here + if !isIdentifier(n) { + node := findMessageReservedNameNode(res.MessageNode(md), n) + nodeInfo := res.file.NodeInfo(node) + if err := handler.HandleErrorf(nodeInfo.Start(), "%s: reserved name %q is not a valid identifier", scope, n); err != nil { + return err + } + } rsvdNames[n] = struct{}{} } fieldTags := map[int32]string{} @@ -242,6 +250,60 @@ func validateMessage(res *result, isProto3 bool, name protoreflect.FullName, md return nil } +func isIdentifier(s string) bool { + if len(s) == 0 { + return false + } + for i, r := range s { + if i == 0 && r >= '0' && r <= '9' { + // can't start with number + return false + } + // alphanumeric and underscore ok; everything else bad + switch { + case r >= '0' && r <= '9': + case r >= 'a' && r <= 'z': + case r >= 'A' && r <= 'Z': + case r == '_': + default: + return false + } + } + return true +} + +func findMessageReservedNameNode(msgNode ast.MessageDeclNode, name string) ast.Node { + var decls []ast.MessageElement + switch msgNode := msgNode.(type) { + case *ast.MessageNode: + decls = msgNode.Decls + case *ast.GroupNode: + decls = msgNode.Decls + default: + // leave decls empty + } + return findReservedNameNode(msgNode, decls, name) +} + +func findReservedNameNode[T ast.Node](parent ast.Node, decls []T, name string) ast.Node { + for _, decl := range decls { + // NB: We have to convert to empty interface first, before we can do a type + // assertion because type assertions on type parameters aren't allowed. (The + // compiler cannot yet know whether T is an interface type or not.) + rsvd, ok := any(decl).(*ast.ReservedNode) + if !ok { + continue + } + for _, rsvdName := range rsvd.Names { + if rsvdName.AsString() == name { + return rsvdName + } + } + } + // couldn't find it? Instead of puking, report position of the parent. + return parent +} + func validateEnum(res *result, isProto3 bool, name protoreflect.FullName, ed *descriptorpb.EnumDescriptorProto, handler *reporter.Handler) error { scope := fmt.Sprintf("enum %s", name) @@ -331,6 +393,14 @@ func validateEnum(res *result, isProto3 bool, name protoreflect.FullName, ed *de // or reserved ranges or reserved names rsvdNames := map[string]struct{}{} for _, n := range ed.ReservedName { + // validate reserved name while we're here + if !isIdentifier(n) { + node := findEnumReservedNameNode(res.EnumNode(ed), n) + nodeInfo := res.file.NodeInfo(node) + if err := handler.HandleErrorf(nodeInfo.Start(), "%s: reserved name %q is not a valid identifier", scope, n); err != nil { + return err + } + } rsvdNames[n] = struct{}{} } for _, ev := range ed.Value { @@ -354,6 +424,15 @@ func validateEnum(res *result, isProto3 bool, name protoreflect.FullName, ed *de return nil } +func findEnumReservedNameNode(enumNode ast.Node, name string) ast.Node { + var decls []ast.EnumElement + if enumNode, ok := enumNode.(*ast.EnumNode); ok { + decls = enumNode.Decls + // if not the right type, we leave decls empty + } + return findReservedNameNode(enumNode, decls, name) +} + func validateField(res *result, isProto3 bool, name protoreflect.FullName, fld *descriptorpb.FieldDescriptorProto, handler *reporter.Handler) error { scope := fmt.Sprintf("field %s", name) diff --git a/parser/validate_test.go b/parser/validate_test.go index 3765414e..ff635b9b 100644 --- a/parser/validate_test.go +++ b/parser/validate_test.go @@ -512,6 +512,79 @@ func TestBasicValidation(t *testing.T) { } } } }`, expectedErr: `test.proto:10:55: message nesting depth must be less than 32`, }, + "failure_message_invalid_reserved_name": { + contents: `syntax = "proto3"; + message Foo { + reserved "foo", "b_a_r9", " blah "; + }`, + expectedErr: `test.proto:3:72: message Foo: reserved name " blah " is not a valid identifier`, + }, + "failure_message_invalid_reserved_name2": { + contents: `syntax = "proto3"; + message Foo { + reserved "foo", "_bar123", "123"; + }`, + expectedErr: `test.proto:3:73: message Foo: reserved name "123" is not a valid identifier`, + }, + "failure_message_invalid_reserved_name3": { + contents: `syntax = "proto3"; + message Foo { + reserved "foo" "_bar123" "@y!!"; + }`, + expectedErr: `test.proto:3:55: message Foo: reserved name "foo_bar123@y!!" is not a valid identifier`, + }, + "failure_message_invalid_reserved_name4": { + contents: `syntax = "proto3"; + message Foo { + reserved ""; + }`, + expectedErr: `test.proto:3:55: message Foo: reserved name "" is not a valid identifier`, + }, + "success_message_reserved_name": { + contents: `syntax = "proto3"; + message Foo { + reserved "foo", "_bar123", "A_B_C_1_2_3"; + }`, + }, + "failure_enum_invalid_reserved_name": { + contents: `syntax = "proto3"; + enum Foo { + BAR = 0; + reserved "foo", "b_a_r9", " blah "; + }`, + expectedErr: `test.proto:4:72: enum Foo: reserved name " blah " is not a valid identifier`, + }, + "failure_enum_invalid_reserved_name2": { + contents: `syntax = "proto3"; + enum Foo { + BAR = 0; + reserved "foo", "_bar123", "123"; + }`, + expectedErr: `test.proto:4:73: enum Foo: reserved name "123" is not a valid identifier`, + }, + "failure_enum_invalid_reserved_name3": { + contents: `syntax = "proto3"; + enum Foo { + BAR = 0; + reserved "foo" "_bar123" "@y!!"; + }`, + expectedErr: `test.proto:4:55: enum Foo: reserved name "foo_bar123@y!!" is not a valid identifier`, + }, + "failure_enum_invalid_reserved_name4": { + contents: `syntax = "proto3"; + enum Foo { + BAR = 0; + reserved ""; + }`, + expectedErr: `test.proto:4:55: enum Foo: reserved name "" is not a valid identifier`, + }, + "success_enum_reserved_name": { + contents: `syntax = "proto3"; + enum Foo { + BAR = 0; + reserved "foo", "_bar123", "A_B_C_1_2_3"; + }`, + }, } for name, tc := range testCases {