Merge pull request protocolbuffers#228 from cconroy/sanitize-enums

Sanitize Enum names from collisions with reserved words.
lgon528 · Mar 16, 2015 · 813d6d6 · 813d6d6
2 parents 8e2a377 + 0d77c82
commit 813d6d6
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 5 deletions.
diff --git a/src/google/protobuf/compiler/cpp/cpp_enum.cc b/src/google/protobuf/compiler/cpp/cpp_enum.cc
@@ -82,14 +82,15 @@ 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.
     vars["number"] = Int32ToString(descriptor_->value(i)->number());
     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
@@ -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
@@ -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
@@ -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
@@ -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) {