Commit 606cb7ed authored by Jie Luo's avatar Jie Luo Committed by GitHub

There might be duplicated enum values when allow_alias is true. Add…

There might be duplicated enum values when allow_alias is true. Add PreferredAlias into OriginalNameAttribute to remove the duplication (#2727)
parent d41c47ff
...@@ -66,6 +66,18 @@ namespace Google.Protobuf ...@@ -66,6 +66,18 @@ namespace Google.Protobuf
AssertJson("{ 'c': 0 }", formatter.Format(new ForeignMessage())); AssertJson("{ 'c': 0 }", formatter.Format(new ForeignMessage()));
} }
[Test]
public void EnumAllowAlias()
{
var message = new TestEnumAllowAlias
{
Value = TestEnumWithDupValue.Foo2,
};
var actualText = JsonFormatter.Default.Format(message);
var expectedText = "{ 'value': 'FOO1' }";
AssertJson(expectedText, actualText);
}
[Test] [Test]
public void AllSingleFields() public void AllSingleFields()
{ {
......
...@@ -835,6 +835,9 @@ namespace Google.Protobuf ...@@ -835,6 +835,9 @@ namespace Google.Protobuf
// TODO: Consider adding functionality to TypeExtensions to avoid this difference. // TODO: Consider adding functionality to TypeExtensions to avoid this difference.
private static Dictionary<object, string> GetNameMapping(System.Type enumType) => private static Dictionary<object, string> GetNameMapping(System.Type enumType) =>
enumType.GetFields(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static) enumType.GetFields(BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static)
.Where(f => (f.GetCustomAttributes(typeof(OriginalNameAttribute), false)
.FirstOrDefault() as OriginalNameAttribute)
?.PreferredAlias ?? true)
.ToDictionary(f => f.GetValue(null), .ToDictionary(f => f.GetValue(null),
f => (f.GetCustomAttributes(typeof(OriginalNameAttribute), false) f => (f.GetCustomAttributes(typeof(OriginalNameAttribute), false)
.FirstOrDefault() as OriginalNameAttribute) .FirstOrDefault() as OriginalNameAttribute)
...@@ -844,6 +847,8 @@ namespace Google.Protobuf ...@@ -844,6 +847,8 @@ namespace Google.Protobuf
private static Dictionary<object, string> GetNameMapping(System.Type enumType) => private static Dictionary<object, string> GetNameMapping(System.Type enumType) =>
enumType.GetTypeInfo().DeclaredFields enumType.GetTypeInfo().DeclaredFields
.Where(f => f.IsStatic) .Where(f => f.IsStatic)
.Where(f => f.GetCustomAttributes<OriginalNameAttribute>()
.FirstOrDefault()?.PreferredAlias ?? true)
.ToDictionary(f => f.GetValue(null), .ToDictionary(f => f.GetValue(null),
f => f.GetCustomAttributes<OriginalNameAttribute>() f => f.GetCustomAttributes<OriginalNameAttribute>()
.FirstOrDefault() .FirstOrDefault()
......
...@@ -46,6 +46,11 @@ namespace Google.Protobuf.Reflection ...@@ -46,6 +46,11 @@ namespace Google.Protobuf.Reflection
/// </summary> /// </summary>
public string Name { get; set; } public string Name { get; set; }
/// <summary>
/// If the name is preferred in the .proto file.
/// </summary>
public bool PreferredAlias { get; set; }
/// <summary> /// <summary>
/// Constructs a new attribute instance for the given name. /// Constructs a new attribute instance for the given name.
/// </summary> /// </summary>
...@@ -53,6 +58,8 @@ namespace Google.Protobuf.Reflection ...@@ -53,6 +58,8 @@ namespace Google.Protobuf.Reflection
public OriginalNameAttribute(string name) public OriginalNameAttribute(string name)
{ {
Name = ProtoPreconditions.CheckNotNull(name, nameof(name)); Name = ProtoPreconditions.CheckNotNull(name, nameof(name));
PreferredAlias = true;
} }
} }
} }
\ No newline at end of file
...@@ -65,6 +65,7 @@ void EnumGenerator::Generate(io::Printer* printer) { ...@@ -65,6 +65,7 @@ void EnumGenerator::Generate(io::Printer* printer) {
"name", descriptor_->name()); "name", descriptor_->name());
printer->Indent(); printer->Indent();
std::set<string> used_names; std::set<string> used_names;
std::set<int> used_number;
for (int i = 0; i < descriptor_->value_count(); i++) { for (int i = 0; i < descriptor_->value_count(); i++) {
WriteEnumValueDocComment(printer, descriptor_->value(i)); WriteEnumValueDocComment(printer, descriptor_->value(i));
string original_name = descriptor_->value(i)->name(); string original_name = descriptor_->value(i)->name();
...@@ -76,10 +77,18 @@ void EnumGenerator::Generate(io::Printer* printer) { ...@@ -76,10 +77,18 @@ void EnumGenerator::Generate(io::Printer* printer) {
<< ") in " << descriptor_->name() << "; adding underscore to distinguish"; << ") in " << descriptor_->name() << "; adding underscore to distinguish";
name += "_"; name += "_";
} }
printer->Print("[pbr::OriginalName(\"$original_name$\")] $name$ = $number$,\n", int number = descriptor_->value(i)->number();
"original_name", original_name, if (!used_number.insert(number).second) {
"name", name, printer->Print("[pbr::OriginalName(\"$original_name$\", PreferredAlias = false)] $name$ = $number$,\n",
"number", SimpleItoa(descriptor_->value(i)->number())); "original_name", original_name,
"name", name,
"number", SimpleItoa(number));
} else {
printer->Print("[pbr::OriginalName(\"$original_name$\")] $name$ = $number$,\n",
"original_name", original_name,
"name", name,
"number", SimpleItoa(number));
}
} }
printer->Outdent(); printer->Outdent();
printer->Print("}\n"); printer->Print("}\n");
......
...@@ -200,6 +200,9 @@ message TestMutualRecursionB { ...@@ -200,6 +200,9 @@ message TestMutualRecursionB {
int32 optional_int32 = 2; int32 optional_int32 = 2;
} }
message TestEnumAllowAlias {
TestEnumWithDupValue value = 1;
}
// 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 {
......
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