Update the error message when a duplicate enum number is used to provide the

next available enum number in addition to mentioning the option to allow
aliasing.

For enums that are large but are not sorted numerically it can be difficult to
find the correct enum number to use.

We use "next available" as the next number larger than the number that was
duplicatively used which is not assigned a value. For sparse enums, there may
be other enums past the suggested number.

PiperOrigin-RevId: 523487238
pull/12318/head
Protobuf Team Bot 2 years ago committed by Copybara-Service
parent 5814f6c977
commit 38de65f9c1
  1. 2
      src/google/protobuf/compiler/parser_unittest.cc
  2. 15
      src/google/protobuf/descriptor.cc
  3. 29
      src/google/protobuf/descriptor_unittest.cc

@ -2231,7 +2231,7 @@ TEST_F(ParserValidationErrorTest, EnumValueAliasError) {
"}\n",
"2:8: \"BAZ\" uses the same enum value as \"BAR\". If this is "
"intended, set 'option allow_alias = true;' to the enum "
"definition.\n");
"definition. The next available enum value is 2.\n");
}
TEST_F(ParserValidationErrorTest, ExplicitlyMapEntryError) {

@ -7382,12 +7382,25 @@ void DescriptorBuilder::ValidateEnumOptions(EnumDescriptor* enm,
if (!enm->options().allow_alias()) {
// Generate error if duplicated enum values are explicitly disallowed.
auto make_error = [&] {
return absl::StrCat(
// Find the next free number.
absl::flat_hash_set<int64_t> used;
for (int j = 0; j < enm->value_count(); ++j) {
used.insert(enm->value(j)->number());
}
int64_t next_value = static_cast<int64_t>(enum_value->number()) + 1;
while (used.contains(next_value)) ++next_value;
std::string error = absl::StrCat(
"\"", enum_value->full_name(),
"\" uses the same enum value as \"",
insert_result.first->second,
"\". If this is intended, set "
"'option allow_alias = true;' to the enum definition.");
if (next_value < std::numeric_limits<int32_t>::max()) {
absl::StrAppend(&error, " The next available enum value is ",
next_value, ".");
}
return error;
};
AddError(enm->full_name(), proto.value(i),
DescriptorPool::ErrorCollector::NUMBER, make_error);

@ -6087,6 +6087,35 @@ TEST_F(ValidationErrorTest, DisallowEnumAlias) {
"foo.proto: Bar: NUMBER: "
"\"ENUM_B\" uses the same enum value as \"ENUM_A\". "
"If this is intended, set 'option allow_alias = true;' to the enum "
"definition. The next available enum value is 1.\n");
BuildFileWithErrors(
R"pb(
name: "foo.proto"
enum_type {
name: "Bar"
value { name: "ENUM_A" number: 10 }
value { name: "ENUM_B" number: 10 }
value { name: "ENUM_C" number: 11 }
value { name: "ENUM_D" number: 20 }
})pb",
"foo.proto: Bar: NUMBER: "
"\"ENUM_B\" uses the same enum value as \"ENUM_A\". "
"If this is intended, set 'option allow_alias = true;' to the enum "
"definition. The next available enum value is 12.\n");
BuildFileWithErrors(
absl::Substitute(R"pb(
name: "foo.proto"
enum_type {
name: "Bar"
value { name: "ENUM_A" number: $0 }
value { name: "ENUM_B" number: $0 }
})pb",
std::numeric_limits<int32_t>::max()),
"foo.proto: Bar: NUMBER: "
"\"ENUM_B\" uses the same enum value as \"ENUM_A\". "
"If this is intended, set 'option allow_alias = true;' to the enum "
"definition.\n");
}

Loading…
Cancel
Save