From d9394ecdb165470a3316480d0dd219dbfa932e7d Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Tue, 19 Jul 2022 13:52:49 +0100 Subject: [PATCH] Add underscore at the end of known common members for C# properties This won't remove all possibilities of naming collisions, but will address the simplest ones. The "test" is just to add all the reserved names in a proto file: if the generated code builds, it works. Note that before this change, using any of these field names would result in a compile-time error, so this is not a breaking change. Generated code is in the next commit. Fixes #8810 --- csharp/protos/unittest_issues.proto | 18 ++++++++++++- .../compiler/csharp/csharp_helpers.cc | 25 +++++++++++++++---- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/csharp/protos/unittest_issues.proto b/csharp/protos/unittest_issues.proto index f46c20e4da..45df7a19a0 100644 --- a/csharp/protos/unittest_issues.proto +++ b/csharp/protos/unittest_issues.proto @@ -169,4 +169,20 @@ message OneofWithNoneName { string x = 1; string y = 2; } -} \ No newline at end of file +} + +// Issue 8810 +message DisambiguateCommonMembers { + int32 disambiguate_common_members = 1; + int32 types = 2; + int32 descriptor = 3; + int32 equals = 4; + int32 to_string = 5; + int32 get_hash_code = 6; + int32 write_to = 7; + int32 clone = 8; + int32 calculate_size = 9; + int32 merge_from = 10; + int32 on_construction = 11; + int32 parser = 12; +} diff --git a/src/google/protobuf/compiler/csharp/csharp_helpers.cc b/src/google/protobuf/compiler/csharp/csharp_helpers.cc index 42d952721e..aa84dc7d82 100644 --- a/src/google/protobuf/compiler/csharp/csharp_helpers.cc +++ b/src/google/protobuf/compiler/csharp/csharp_helpers.cc @@ -379,15 +379,30 @@ std::string GetFieldConstantName(const FieldDescriptor* field) { } std::string GetPropertyName(const FieldDescriptor* descriptor) { + // Names of members declared or overridden in the message. + static const auto& reserved_member_names = *new std::unordered_set({ + "Types", + "Descriptor", + "Equals", + "ToString", + "GetHashCode", + "WriteTo", + "Clone", + "CalculateSize", + "MergeFrom", + "OnConstruction", + "Parser" + }); + // TODO(jtattermusch): consider introducing csharp_property_name field option std::string property_name = UnderscoresToPascalCase(GetFieldName(descriptor)); - // Avoid either our own type name or reserved names. Note that not all names - // are reserved - a field called to_string, write_to etc would still cause a problem. + // Avoid either our own type name or reserved names. // There are various ways of ending up with naming collisions, but we try to avoid obvious - // ones. + // ones. In particular, we avoid the names of all the members we generate. + // Note that we *don't* add an underscore for MemberwiseClone or GetType. Those generate + // warnings, but not errors; changing the name now could be a breaking change. if (property_name == descriptor->containing_type()->name() - || property_name == "Types" - || property_name == "Descriptor") { + || reserved_member_names.find(property_name) != reserved_member_names.end()) { property_name += "_"; } return property_name;