If enum aliases overlap in ObjC names skip generating the extras.

Some protos have enum values of "FOO" and "Foo", which the ObjC generation
then makes into the same thing. Just skip generating the enum element for
the duplicate as it would be a compile error because of the name collision.

The descriptors are still generated to support reflection and TextFormat
more completely.
This commit is contained in:
Thomas Van Lenten 2018-12-17 17:20:56 -05:00
parent 4c559316e0
commit d529720e2f
4 changed files with 76 additions and 0 deletions

View File

@ -244,7 +244,44 @@
XCTAssertTrue([descriptor getValue:&value forEnumName:enumName]); XCTAssertTrue([descriptor getValue:&value forEnumName:enumName]);
XCTAssertEqual(value, 2); XCTAssertEqual(value, 2);
XCTAssertEqualObjects([descriptor getEnumTextFormatNameForIndex:4], @"BAR2"); XCTAssertEqualObjects([descriptor getEnumTextFormatNameForIndex:4], @"BAR2");
}
- (void)testEnumAliasNameCollisions {
GPBEnumDescriptor *descriptor = TestEnumObjCNameCollision_EnumDescriptor();
NSString *textFormatName;
int32_t value;
XCTAssertEqual(descriptor.enumNameCount, 5U);
XCTAssertEqualObjects([descriptor getEnumNameForIndex:0], @"TestEnumObjCNameCollision_Foo");
textFormatName = [descriptor getEnumTextFormatNameForIndex:0];
XCTAssertEqualObjects(textFormatName, @"FOO");
XCTAssertTrue([descriptor getValue:&value forEnumTextFormatName:textFormatName]);
XCTAssertEqual(value, 1);
XCTAssertEqualObjects([descriptor getEnumNameForIndex:1], @"TestEnumObjCNameCollision_Foo");
textFormatName = [descriptor getEnumTextFormatNameForIndex:1];
XCTAssertEqualObjects(textFormatName, @"foo");
XCTAssertTrue([descriptor getValue:&value forEnumTextFormatName:textFormatName]);
XCTAssertEqual(value, 1);
XCTAssertEqualObjects([descriptor getEnumNameForIndex:2], @"TestEnumObjCNameCollision_Bar");
textFormatName = [descriptor getEnumTextFormatNameForIndex:2];
XCTAssertEqualObjects(textFormatName, @"BAR");
XCTAssertTrue([descriptor getValue:&value forEnumTextFormatName:textFormatName]);
XCTAssertEqual(value, 2);
XCTAssertEqualObjects([descriptor getEnumNameForIndex:3], @"TestEnumObjCNameCollision_Mumble");
textFormatName = [descriptor getEnumTextFormatNameForIndex:3];
XCTAssertEqualObjects(textFormatName, @"mumble");
XCTAssertTrue([descriptor getValue:&value forEnumTextFormatName:textFormatName]);
XCTAssertEqual(value, 2);
XCTAssertEqualObjects([descriptor getEnumNameForIndex:4], @"TestEnumObjCNameCollision_Mumble");
textFormatName = [descriptor getEnumTextFormatNameForIndex:4];
XCTAssertEqualObjects(textFormatName, @"MUMBLE");
XCTAssertTrue([descriptor getValue:&value forEnumTextFormatName:textFormatName]);
XCTAssertEqual(value, 2);
} }
- (void)testEnumValueValidator { - (void)testEnumValueValidator {

View File

@ -867,3 +867,18 @@ message BoolOnlyMessage {
message WKTRefereceMessage { message WKTRefereceMessage {
optional google.protobuf.Any an_any = 1; optional google.protobuf.Any an_any = 1;
} }
// This is in part a compile test, it ensures that when aliases end up with
// the same ObjC name, we drop them to avoid the duplication names. There
// is a test to ensure the descriptors are still generated to support
// reflection and TextFormat.
enum TestEnumObjCNameCollision {
option allow_alias = true;
FOO = 1;
foo = 1;
BAR = 2;
mumble = 2;
MUMBLE = 2;
}

View File

@ -35,6 +35,7 @@
#include <google/protobuf/compiler/objectivec/objectivec_helpers.h> #include <google/protobuf/compiler/objectivec/objectivec_helpers.h>
#include <google/protobuf/io/printer.h> #include <google/protobuf/io/printer.h>
#include <google/protobuf/stubs/strutil.h> #include <google/protobuf/stubs/strutil.h>
#include <algorithm> // std::find()
namespace google { namespace google {
namespace protobuf { namespace protobuf {
@ -44,6 +45,17 @@ namespace objectivec {
EnumGenerator::EnumGenerator(const EnumDescriptor* descriptor) EnumGenerator::EnumGenerator(const EnumDescriptor* descriptor)
: descriptor_(descriptor), : descriptor_(descriptor),
name_(EnumName(descriptor_)) { name_(EnumName(descriptor_)) {
// Track the names for the enum values, and if an alias overlaps a base
// value, skip making a name for it. Likewise if two alias overlap, the
// first one wins.
// The one gap in this logic is if two base values overlap, but for that
// to happen you have to have "Foo" and "FOO" or "FOO_BAR" and "FooBar",
// and if an enum has that, it is already going to be confusing and a
// compile error is just fine.
// The values are still tracked to support the reflection apis and
// TextFormat handing since they are different there.
std::set<std::string> value_names;
for (int i = 0; i < descriptor_->value_count(); i++) { for (int i = 0; i < descriptor_->value_count(); i++) {
const EnumValueDescriptor* value = descriptor_->value(i); const EnumValueDescriptor* value = descriptor_->value(i);
const EnumValueDescriptor* canonical_value = const EnumValueDescriptor* canonical_value =
@ -51,6 +63,14 @@ EnumGenerator::EnumGenerator(const EnumDescriptor* descriptor)
if (value == canonical_value) { if (value == canonical_value) {
base_values_.push_back(value); base_values_.push_back(value);
value_names.insert(EnumValueName(value));
} else {
string value_name(EnumValueName(value));
if (value_names.find(value_name) != value_names.end()) {
alias_values_to_skip_.insert(value);
} else {
value_names.insert(value_name);
}
} }
all_values_.push_back(value); all_values_.push_back(value);
} }
@ -90,6 +110,9 @@ void EnumGenerator::GenerateHeader(io::Printer* printer) {
"name", name_); "name", name_);
} }
for (int i = 0; i < all_values_.size(); i++) { for (int i = 0; i < all_values_.size(); i++) {
if (alias_values_to_skip_.find(all_values_[i]) != alias_values_to_skip_.end()) {
continue;
}
SourceLocation location; SourceLocation location;
if (all_values_[i]->GetSourceLocation(&location)) { if (all_values_[i]->GetSourceLocation(&location)) {
string comments = BuildCommentsString(location, true).c_str(); string comments = BuildCommentsString(location, true).c_str();

View File

@ -59,6 +59,7 @@ class EnumGenerator {
const EnumDescriptor* descriptor_; const EnumDescriptor* descriptor_;
std::vector<const EnumValueDescriptor*> base_values_; std::vector<const EnumValueDescriptor*> base_values_;
std::vector<const EnumValueDescriptor*> all_values_; std::vector<const EnumValueDescriptor*> all_values_;
std::set<const EnumValueDescriptor*> alias_values_to_skip_;
const string name_; const string name_;
}; };