Skip to content

Commit eba53e8

Browse files
Avoid collision name problems between a message named Xyz and a direct sibling enum named XyzView
We unfortuantely have no holistic startegy to avoid broken gencode from cross-type name collisions. We hold a high bar for special case mangling cross-type collisions, but this specific naming pattern combination is recommended by AIP 157. Note that we will still break if the Xyz message and the XyzView enum are defined in separate .proto files but built in the same crate. It is very difficult for us to avoid that specific collision, so we are punting on it for now and only handling the case where the message and enum are defined in the same file. Especially when using blazel, we recommend one proto_library() per proto file, which would avoid that topic from being a concern. PiperOrigin-RevId: 853935768
1 parent 0bc4192 commit eba53e8

File tree

2 files changed

+52
-2
lines changed

2 files changed

+52
-2
lines changed

rust/test/bad_names.proto

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,8 @@ message AccessorsCollide {
5656

5757
optional int32 clear_x = 7;
5858
}
59+
60+
message SomeMsg {}
61+
enum SomeMsgView {
62+
SOME_MSG_VIEW_UNSPECIFIED = 0;
63+
}

src/google/protobuf/compiler/rust/naming.cc

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,52 @@ std::string RsSafeName(absl::string_view name) {
310310
return std::string(name);
311311
}
312312

313+
namespace {
314+
315+
bool AnyChildMessageNamed(const FileDescriptor* scope, absl::string_view name) {
316+
for (int i = 0; i < scope->message_type_count(); ++i) {
317+
if (scope->message_type(i)->name() == name) {
318+
return true;
319+
}
320+
}
321+
return false;
322+
}
323+
324+
bool AnyChildMessageNamed(const Descriptor* scope, absl::string_view name) {
325+
for (int i = 0; i < scope->nested_type_count(); ++i) {
326+
if (scope->nested_type(i)->name() == name) {
327+
return true;
328+
}
329+
}
330+
return false;
331+
}
332+
333+
bool MustMangleEnumName(const EnumDescriptor& desc) {
334+
// If an enum name ends with 'View', we check if there is a message whose name
335+
// matches the enum name without the 'View' suffix. If so,
336+
// append an extra 'X' character on the end of the gencode enum name. The
337+
// reason we special case mangle this is to avoid breakages from the View
338+
// type of the message when the .proto file is following this AIP:
339+
// https://google.aip.dev/157#view-enumeration
340+
if (!absl::EndsWith(desc.name(), "View")) {
341+
return false;
342+
}
343+
absl::string_view name_without_view_suffix =
344+
absl::StripSuffix(desc.name(), "View");
345+
return desc.containing_type() != nullptr
346+
? AnyChildMessageNamed(desc.containing_type(),
347+
name_without_view_suffix)
348+
: AnyChildMessageNamed(desc.file(), name_without_view_suffix);
349+
}
350+
351+
} // namespace
352+
313353
std::string EnumRsName(const EnumDescriptor& desc) {
314-
return RsSafeName(SnakeToUpperCamelCase(desc.name()));
354+
std::string name = RsSafeName(SnakeToUpperCamelCase(desc.name()));
355+
if (MustMangleEnumName(desc)) {
356+
absl::StrAppend(&name, "_");
357+
}
358+
return name;
315359
}
316360

317361
std::string EnumValueRsName(const EnumValueDescriptor& value) {
@@ -323,7 +367,8 @@ std::string EnumValueRsName(const MultiCasePrefixStripper& stripper,
323367
absl::string_view value_name) {
324368
// Enum values may have a prefix of the name of the enum stripped from the
325369
// value names in the gencode. This prefix is flexible:
326-
// - It can be the original enum name, the name as UpperCamel, or snake_case.
370+
// - It can be the original enum name, the name as UpperCamel, or
371+
// snake_case.
327372
// - The stripped prefix may also end in an underscore.
328373
auto stripped = stripper.StripPrefix(value_name);
329374

0 commit comments

Comments
 (0)