From 0d77c824e7015190a9174052fc5290c23f2b85d0 Mon Sep 17 00:00:00 2001 From: Chris Conroy Date: Fri, 25 Oct 2013 16:43:29 -0400 Subject: [PATCH] Sanitize Enum names from collisions with reserved words. --- src/google/protobuf/compiler/cpp/cpp_enum.cc | 11 ++++++----- .../protobuf/compiler/cpp/cpp_helpers.cc | 8 ++++++++ src/google/protobuf/compiler/cpp/cpp_helpers.h | 3 +++ .../cpp/cpp_test_bad_identifiers.proto | 18 ++++++++++++++++++ .../protobuf/compiler/cpp/cpp_unittest.cc | 15 +++++++++++++++ 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/cpp_enum.cc b/src/google/protobuf/compiler/cpp/cpp_enum.cc index 0404b73981..3eb20ab136 100644 --- a/src/google/protobuf/compiler/cpp/cpp_enum.cc +++ b/src/google/protobuf/compiler/cpp/cpp_enum.cc @@ -82,7 +82,7 @@ void EnumGenerator::GenerateDefinition(io::Printer* printer) { const EnumValueDescriptor* max_value = descriptor_->value(0); for (int i = 0; i < descriptor_->value_count(); i++) { - vars["name"] = descriptor_->value(i)->name(); + vars["name"] = EnumValueName(descriptor_->value(i)); // In C++, an value of -2147483648 gets interpreted as the negative of // 2147483648, and since 2147483648 can't fit in an integer, this produces a // compiler warning. This works around that issue. @@ -90,6 +90,7 @@ void EnumGenerator::GenerateDefinition(io::Printer* printer) { vars["prefix"] = (descriptor_->containing_type() == NULL) ? "" : classname_ + "_"; + if (i > 0) printer->Print(",\n"); printer->Print(vars, "$prefix$$name$ = $number$"); @@ -113,8 +114,8 @@ void EnumGenerator::GenerateDefinition(io::Printer* printer) { printer->Outdent(); printer->Print("\n};\n"); - vars["min_name"] = min_value->name(); - vars["max_name"] = max_value->name(); + vars["min_name"] = EnumValueName(min_value); + vars["max_name"] = EnumValueName(max_value); if (options_.dllexport_decl.empty()) { vars["dllexport"] = ""; @@ -174,7 +175,7 @@ void EnumGenerator::GenerateSymbolImports(io::Printer* printer) { printer->Print(vars, "typedef $classname$ $nested_name$;\n"); for (int j = 0; j < descriptor_->value_count(); j++) { - vars["tag"] = descriptor_->value(j)->name(); + vars["tag"] = EnumValueName(descriptor_->value(j)); printer->Print(vars, "static const $nested_name$ $tag$ = $classname$_$tag$;\n"); } @@ -278,7 +279,7 @@ void EnumGenerator::GenerateMethods(io::Printer* printer) { vars["parent"] = ClassName(descriptor_->containing_type(), false); vars["nested_name"] = descriptor_->name(); for (int i = 0; i < descriptor_->value_count(); i++) { - vars["value"] = descriptor_->value(i)->name(); + vars["value"] = EnumValueName(descriptor_->value(i)); printer->Print(vars, "const $classname$ $parent$::$value$;\n"); } diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.cc b/src/google/protobuf/compiler/cpp/cpp_helpers.cc index 237278dba1..4e7155c33c 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.cc +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.cc @@ -176,6 +176,14 @@ string FieldName(const FieldDescriptor* field) { return result; } +string EnumValueName(const EnumValueDescriptor* enum_value) { + string result = enum_value->name(); + if (kKeywords.count(result) > 0) { + result.append("_"); + } + return result; +} + string FieldConstantName(const FieldDescriptor *field) { string field_name = UnderscoresToCamelCase(field->name(), true); string result = "k" + field_name + "FieldNumber"; diff --git a/src/google/protobuf/compiler/cpp/cpp_helpers.h b/src/google/protobuf/compiler/cpp/cpp_helpers.h index c7bb8f98fd..284fa2c1a5 100644 --- a/src/google/protobuf/compiler/cpp/cpp_helpers.h +++ b/src/google/protobuf/compiler/cpp/cpp_helpers.h @@ -74,6 +74,9 @@ string SuperClassName(const Descriptor* descriptor); // anyway, so normally this just returns field->name(). string FieldName(const FieldDescriptor* field); +// Get the sanitized name that should be used for the given enum in C++ code. +string EnumValueName(const EnumValueDescriptor* enum_value); + // Get the unqualified name that should be used for a field's field // number constant. string FieldConstantName(const FieldDescriptor *field); diff --git a/src/google/protobuf/compiler/cpp/cpp_test_bad_identifiers.proto b/src/google/protobuf/compiler/cpp/cpp_test_bad_identifiers.proto index 4fa3c144cd..9f63155b41 100644 --- a/src/google/protobuf/compiler/cpp/cpp_test_bad_identifiers.proto +++ b/src/google/protobuf/compiler/cpp/cpp_test_bad_identifiers.proto @@ -131,6 +131,24 @@ message TestConflictingSymbolNamesExtension { // NO_PROTO3 } // NO_PROTO3 } // NO_PROTO3 +message TestConflictingEnumNames { + enum NestedConflictingEnum { + and = 1; + class = 2; + int = 3; + typedef = 4; + XOR = 5; + } + + optional NestedConflictingEnum conflicting_enum = 1; +} + +enum ConflictingEnum { + NOT_EQ = 1; + volatile = 2; + return = 3; +} + message DummyMessage {} service TestConflictingMethodNames { diff --git a/src/google/protobuf/compiler/cpp/cpp_unittest.cc b/src/google/protobuf/compiler/cpp/cpp_unittest.cc index 2a04b2931b..cc758cf5ee 100644 --- a/src/google/protobuf/compiler/cpp/cpp_unittest.cc +++ b/src/google/protobuf/compiler/cpp/cpp_unittest.cc @@ -794,6 +794,21 @@ TEST(GeneratedMessageTest, TestConflictingSymbolNames) { message.GetExtension(ExtensionMessage::repeated_int32_ext, 0)); } +TEST(GeneratedMessageTest, TestConflictingEnumNames) { + protobuf_unittest::TestConflictingEnumNames message; + message.set_conflicting_enum(protobuf_unittest::TestConflictingEnumNames_NestedConflictingEnum_and_); + EXPECT_EQ(1, message.conflicting_enum()); + message.set_conflicting_enum(protobuf_unittest::TestConflictingEnumNames_NestedConflictingEnum_XOR); + EXPECT_EQ(5, message.conflicting_enum()); + + + protobuf_unittest::ConflictingEnum conflicting_enum; + conflicting_enum = protobuf_unittest::NOT_EQ; + EXPECT_EQ(1, conflicting_enum); + conflicting_enum = protobuf_unittest::return_; + EXPECT_EQ(3, conflicting_enum); +} + #ifndef PROTOBUF_TEST_NO_DESCRIPTORS TEST(GeneratedMessageTest, TestOptimizedForSize) {