Editions uses identifiers instead of string literals for reserved names #176
Conversation
parser/result.go
Outdated
| ed := &descriptorpb.EnumDescriptorProto{Name: proto.String(en.Name.Val)} | ||
| r.putEnumNode(ed, en) | ||
| rsvdNames := map[string]int{} |
There was a problem hiding this comment.
Small nit - looks like we are only using the count to determine if it is >1. We could tweak to use struct{} value instead.
There was a problem hiding this comment.
This was intentional to de-duplicate the errors. We only report the first duplicate. But perhaps it would be okay (or even better?) to report them all. They are currently de-duplicated via use of int because we only report when count == 1, not when it's >= 1.
There was a problem hiding this comment.
Okay, I changed this so it actually stores the position of the first declaration and can report that in the error message.
| nameNodeInfo := r.file.NodeInfo(node.Identifiers[0]) | ||
| _ = handler.HandleErrorf(nameNodeInfo.Start(), `must use string literals, not identifiers, to reserved names with proto2 and proto3`) | ||
| } | ||
| for _, n := range node.Names { |
There was a problem hiding this comment.
Should we put this in an if statement for non editions syntax? Feels like we should ignore it entirely in error messages if string literals aren't supported at all.
There was a problem hiding this comment.
I re-structured this method to do just this. The branch for editions checks that Names is empty and processes Identifiers. The other branch, does the opposite, checking that Identifiers is empty and processes Names.
This implements the use of identifiers, instead of string literals, in
reservedstatements. This new syntax is for editions; proto2 and proto3 continue to use string literals for backwards compatibility.This effectively ports protocolbuffers/protobuf#13471 to protocompile.