فهرست منبع

Do strict enum name checking only for proto3

There seem to already be .proto files out there that have conflicting
enum names, which will not be able to build successfully for some
languages (like C#). To prevent this problem from spreading, let's make
it an error for proto3 but just issue a warning for proto2. This fixes
issue #2179.
Adam Cozzette 9 سال پیش
والد
کامیت
70477613b9
2فایلهای تغییر یافته به همراه27 افزوده شده و 9 حذف شده
  1. 14 5
      src/google/protobuf/descriptor.cc
  2. 13 4
      src/google/protobuf/descriptor_unittest.cc

+ 14 - 5
src/google/protobuf/descriptor.cc

@@ -4707,11 +4707,20 @@ void DescriptorBuilder::CheckEnumValueUniqueness(
     // stripping should de-dup the labels in this case).
     // stripping should de-dup the labels in this case).
     if (!inserted && insert_result.first->second->name() != value->name() &&
     if (!inserted && insert_result.first->second->name() != value->name() &&
         insert_result.first->second->number() != value->number()) {
         insert_result.first->second->number() != value->number()) {
-      AddError(value->full_name(), proto.value(i),
-               DescriptorPool::ErrorCollector::NAME,
-               "When enum name is stripped and label is PascalCased (" +
-                   stripped + "), this value label conflicts with " +
-                   values[stripped]->name());
+      string error_message =
+          "When enum name is stripped and label is PascalCased (" + stripped +
+          "), this value label conflicts with " + values[stripped]->name() +
+          ". This will make the proto fail to compile for some languages, such "
+          "as C#.";
+      // There are proto2 enums out there with conflicting names, so to preserve
+      // compatibility we issue only a warning for proto2.
+      if (result->file()->syntax() == FileDescriptor::SYNTAX_PROTO2) {
+        AddWarning(value->full_name(), proto.value(i),
+                   DescriptorPool::ErrorCollector::NAME, error_message);
+      } else {
+        AddError(value->full_name(), proto.value(i),
+                 DescriptorPool::ErrorCollector::NAME, error_message);
+      }
     }
     }
   }
   }
 }
 }

+ 13 - 4
src/google/protobuf/descriptor_unittest.cc

@@ -5565,6 +5565,7 @@ TEST_F(ValidationErrorTest, MapEntryConflictsWithEnum) {
 
 
 TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) {
 TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) {
   BuildFileWithErrors(
   BuildFileWithErrors(
+      "syntax: 'proto3'"
       "name: 'foo.proto' "
       "name: 'foo.proto' "
       "enum_type {"
       "enum_type {"
       "  name: 'FooEnum' "
       "  name: 'FooEnum' "
@@ -5572,9 +5573,11 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) {
       "  value { name: 'BAZ' number: 1 }"
       "  value { name: 'BAZ' number: 1 }"
       "}",
       "}",
       "foo.proto: BAZ: NAME: When enum name is stripped and label is "
       "foo.proto: BAZ: NAME: When enum name is stripped and label is "
-      "PascalCased (Baz), this value label conflicts with FOO_ENUM_BAZ\n");
+      "PascalCased (Baz), this value label conflicts with FOO_ENUM_BAZ. This "
+      "will make the proto fail to compile for some languages, such as C#.\n");
 
 
   BuildFileWithErrors(
   BuildFileWithErrors(
+      "syntax: 'proto3'"
       "name: 'foo.proto' "
       "name: 'foo.proto' "
       "enum_type {"
       "enum_type {"
       "  name: 'FooEnum' "
       "  name: 'FooEnum' "
@@ -5582,9 +5585,11 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) {
       "  value { name: 'BAZ' number: 1 }"
       "  value { name: 'BAZ' number: 1 }"
       "}",
       "}",
       "foo.proto: BAZ: NAME: When enum name is stripped and label is "
       "foo.proto: BAZ: NAME: When enum name is stripped and label is "
-      "PascalCased (Baz), this value label conflicts with FOOENUM_BAZ\n");
+      "PascalCased (Baz), this value label conflicts with FOOENUM_BAZ. This "
+      "will make the proto fail to compile for some languages, such as C#.\n");
 
 
   BuildFileWithErrors(
   BuildFileWithErrors(
+      "syntax: 'proto3'"
       "name: 'foo.proto' "
       "name: 'foo.proto' "
       "enum_type {"
       "enum_type {"
       "  name: 'FooEnum' "
       "  name: 'FooEnum' "
@@ -5593,9 +5598,11 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) {
       "}",
       "}",
       "foo.proto: BAR__BAZ: NAME: When enum name is stripped and label is "
       "foo.proto: BAR__BAZ: NAME: When enum name is stripped and label is "
       "PascalCased (BarBaz), this value label conflicts with "
       "PascalCased (BarBaz), this value label conflicts with "
-      "FOO_ENUM_BAR_BAZ\n");
+      "FOO_ENUM_BAR_BAZ. This "
+      "will make the proto fail to compile for some languages, such as C#.\n");
 
 
   BuildFileWithErrors(
   BuildFileWithErrors(
+      "syntax: 'proto3'"
       "name: 'foo.proto' "
       "name: 'foo.proto' "
       "enum_type {"
       "enum_type {"
       "  name: 'FooEnum' "
       "  name: 'FooEnum' "
@@ -5604,11 +5611,13 @@ TEST_F(ValidationErrorTest, EnumValuesConflictWhenPrefixesStripped) {
       "}",
       "}",
       "foo.proto: BAR_BAZ: NAME: When enum name is stripped and label is "
       "foo.proto: BAR_BAZ: NAME: When enum name is stripped and label is "
       "PascalCased (BarBaz), this value label conflicts with "
       "PascalCased (BarBaz), this value label conflicts with "
-      "FOO_ENUM__BAR_BAZ\n");
+      "FOO_ENUM__BAR_BAZ. This "
+      "will make the proto fail to compile for some languages, such as C#.\n");
 
 
   // This isn't an error because the underscore will cause the PascalCase to
   // This isn't an error because the underscore will cause the PascalCase to
   // differ by case (BarBaz vs. Barbaz).
   // differ by case (BarBaz vs. Barbaz).
   BuildFile(
   BuildFile(
+      "syntax: 'proto3'"
       "name: 'foo.proto' "
       "name: 'foo.proto' "
       "enum_type {"
       "enum_type {"
       "  name: 'FooEnum' "
       "  name: 'FooEnum' "