Commit cb77c4c3 authored by liujisi@google.com's avatar liujisi@google.com

Generate a warning for duplicated enum values, when allow_alias option isn't

set.
parent 4770277e
...@@ -4132,16 +4132,26 @@ void DescriptorBuilder::ValidateFieldOptions(FieldDescriptor* field, ...@@ -4132,16 +4132,26 @@ void DescriptorBuilder::ValidateFieldOptions(FieldDescriptor* field,
void DescriptorBuilder::ValidateEnumOptions(EnumDescriptor* enm, void DescriptorBuilder::ValidateEnumOptions(EnumDescriptor* enm,
const EnumDescriptorProto& proto) { const EnumDescriptorProto& proto) {
VALIDATE_OPTIONS_FROM_ARRAY(enm, value, EnumValue); VALIDATE_OPTIONS_FROM_ARRAY(enm, value, EnumValue);
if (!enm->options().allow_alias()) { if (!enm->options().has_allow_alias() || !enm->options().allow_alias()) {
map<int, string> used_values; map<int, string> used_values;
for (int i = 0; i < enm->value_count(); ++i) { for (int i = 0; i < enm->value_count(); ++i) {
const EnumValueDescriptor* enum_value = enm->value(i); const EnumValueDescriptor* enum_value = enm->value(i);
if (used_values.find(enum_value->number()) != used_values.end()) { if (used_values.find(enum_value->number()) != used_values.end()) {
AddError(enm->full_name(), proto, string error =
DescriptorPool::ErrorCollector::NUMBER,
"\"" + enum_value->full_name() + "\"" + enum_value->full_name() +
"\" uses the same enum value as \"" + "\" uses the same enum value as \"" +
used_values[enum_value->number()] + "\""); used_values[enum_value->number()] + "\". If this is intended, set "
"'option allow_alias = true;' to the enum definition.";
if (!enm->options().allow_alias()) {
// Generate error if duplicated enum values are explicitly disallowed.
AddError(enm->full_name(), proto,
DescriptorPool::ErrorCollector::NUMBER,
error);
} else {
// Generate warning if duplicated values are found but the option
// isn't set.
GOOGLE_LOG(ERROR) << error;
}
} else { } else {
used_values[enum_value->number()] = enum_value->full_name(); used_values[enum_value->number()] = enum_value->full_name();
} }
......
...@@ -4001,7 +4001,9 @@ TEST_F(ValidationErrorTest, DisallowEnumAlias) { ...@@ -4001,7 +4001,9 @@ TEST_F(ValidationErrorTest, DisallowEnumAlias) {
" options { allow_alias: false }" " options { allow_alias: false }"
"}", "}",
"foo.proto: Bar: NUMBER: " "foo.proto: Bar: NUMBER: "
"\"ENUM_B\" uses the same enum value as \"ENUM_A\"\n"); "\"ENUM_B\" uses the same enum value as \"ENUM_A\". "
"If this is intended, set 'option allow_alias = true;' to the enum "
"definition.\n");
} }
// =================================================================== // ===================================================================
......
...@@ -434,6 +434,7 @@ message TestNestedMessageHasBits { ...@@ -434,6 +434,7 @@ message TestNestedMessageHasBits {
// Test an enum that has multiple values with the same number. // Test an enum that has multiple values with the same number.
enum TestEnumWithDupValue { enum TestEnumWithDupValue {
option allow_alias = true;
FOO1 = 1; FOO1 = 1;
BAR1 = 2; BAR1 = 2;
BAZ = 3; BAZ = 3;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment