Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve more java field accessor name conflicts #8198

Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,31 @@
final class DescriptorMessageInfoFactory implements MessageInfoFactory {
private static final String GET_DEFAULT_INSTANCE_METHOD_NAME = "getDefaultInstance";
private static final DescriptorMessageInfoFactory instance = new DescriptorMessageInfoFactory();

/**
* Names that should be avoided (in UpperCamelCase format).
* Using them will cause the compiler to generate accessors whose names are
philsttr marked this conversation as resolved.
Show resolved Hide resolved
* colliding with methods defined in base classes.
*
* Keep this list in sync with kForbiddenWordList in
* src/google/protobuf/compiler/java/java_helpers.cc
*/
private static final Set<String> specialFieldNames =
new HashSet<>(Arrays.asList("cached_size", "serialized_size", "class"));
new HashSet<>(Arrays.asList(
// java.lang.Object:
"Class",
// com.google.protobuf.MessageLiteOrBuilder:
"DefaultInstanceForType",
// com.google.protobuf.MessageLite:
"ParserForType",
"SerializedSize",
// com.google.protobuf.MessageOrBuilder:
"AllFields",
"DescriptorForType",
"InitializationErrorString",
"UnknownFields",
// obsolete. kept for backwards compatibility of generated code
"CachedSize"));

// Disallow construction - it's a singleton.
private DescriptorMessageInfoFactory() {}
Expand Down Expand Up @@ -593,21 +616,25 @@ static String getFieldName(FieldDescriptor fd) {
String name = (fd.getType() == FieldDescriptor.Type.GROUP)
? fd.getMessageType().getName()
: fd.getName();
String suffix = specialFieldNames.contains(name) ? "__" : "_";
return snakeCaseToCamelCase(name) + suffix;
String suffix = specialFieldNames.contains(snakeCaseToCamelCase(name, true)) ? "__" : "_";
philsttr marked this conversation as resolved.
Show resolved Hide resolved
return snakeCaseToCamelCase(name, false) + suffix;
}

private static String getCachedSizeFieldName(FieldDescriptor fd) {
return snakeCaseToCamelCase(fd.getName()) + "MemoizedSerializedSize";
}

private static String snakeCaseToCamelCase(String snakeCase) {
return snakeCaseToCamelCase(snakeCase, false);
philsttr marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* This method must match exactly with the corresponding function in protocol compiler. See:
* https://github.com/google/protobuf/blob/v3.0.0/src/google/protobuf/compiler/java/java_helpers.cc#L153
* https://github.com/protocolbuffers/protobuf/blob/v3.15.0/src/google/protobuf/compiler/java/java_helpers.cc#L158
philsttr marked this conversation as resolved.
Show resolved Hide resolved
*/
private static String snakeCaseToCamelCase(String snakeCase) {
private static String snakeCaseToCamelCase(String snakeCase, boolean capFirst) {
philsttr marked this conversation as resolved.
Show resolved Hide resolved
StringBuilder sb = new StringBuilder(snakeCase.length() + 1);
boolean capNext = false;
boolean capNext = capFirst;
for (int ctr = 0; ctr < snakeCase.length(); ctr++) {
char next = snakeCase.charAt(ctr);
if (next == '_') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,61 @@ option java_generic_services = true; // auto-added
option java_package = "com.google.protobuf";
option java_outer_classname = "TestBadIdentifiersProto";

message TestMessage {
optional string cached_size = 1;
optional string serialized_size = 2;
optional string class = 3;
// Message with field names using underscores that conflict with accessors in the base message class in java.
// See kForbiddenWordList in src/google/protobuf/compiler/java/java_helpers.cc
message ForbiddenWordsUnderscoreMessage {
// java.lang.Object
optional bool class = 1;
// com.google.protobuf.MessageLiteOrBuilder
optional bool default_instance_for_type = 2;
// com.google.protobuf.MessageLite
optional bool parser_for_type = 3;
optional bool serialized_size = 4;
// com.google.protobuf.MessageOrBuilder
optional bool all_fields = 5;
optional bool descriptor_for_type = 6;
optional bool initialization_error_string = 7;
optional bool unknown_fields = 8;
// obsolete. kept for backwards compatibility of generated code
optional bool cached_size = 9;
}

// Message with field names using leading underscores that conflict with accessors in the base message class in java.
// See kForbiddenWordList in src/google/protobuf/compiler/java/java_helpers.cc
message ForbiddenWordsLeadingUnderscoreMessage {
// java.lang.Object
optional bool _class = 1;
// com.google.protobuf.MessageLiteOrBuilder
optional bool _default_instance_for_type = 2;
// com.google.protobuf.MessageLite
optional bool _parser_for_type = 3;
optional bool _serialized_size = 4;
// com.google.protobuf.MessageOrBuilder
optional bool _all_fields = 5;
optional bool _descriptor_for_type = 6;
optional bool _initialization_error_string = 7;
optional bool _unknown_fields = 8;
// obsolete. kept for backwards compatibility of generated code
optional bool _cached_size = 9;
}

// Message with field names in camel case that conflict with accessors in the base message class in java.
// See kForbiddenWordList in src/google/protobuf/compiler/java/java_helpers.cc
message ForbiddenWordsCamelMessage {
// java.lang.Object
optional bool class = 1;
// com.google.protobuf.MessageLiteOrBuilder
optional bool defaultInstanceForType = 2;
// com.google.protobuf.MessageLite
optional bool serializedSize = 3;
optional bool parserForType = 4;
// com.google.protobuf.MessageOrBuilder:
optional bool initializationErrorString = 5;
optional bool descriptorForType = 6;
optional bool allFields = 7;
optional bool unknownFields = 8;
// obsolete. kept for backwards compatibility of generated code
optional bool cachedSize = 9;
}

message Descriptor {
Expand Down Expand Up @@ -84,7 +135,7 @@ message Deprecated {

optional int32 field1 = 1 [deprecated = true];
optional TestEnum field2 = 2 [deprecated = true];
optional TestMessage field3 = 3 [deprecated = true];
optional ForbiddenWordsUnderscoreMessage field3 = 3 [deprecated = true];
}

message Override {
Expand Down Expand Up @@ -117,32 +168,32 @@ message Double {
}

service TestConflictingMethodNames {
rpc Override(TestMessage) returns (TestMessage);
rpc Override(ForbiddenWordsUnderscoreMessage) returns (ForbiddenWordsUnderscoreMessage);
}

message TestConflictingFieldNames {
enum TestEnum {
UNKNOWN = 0;
FOO = 1;
}
message TestMessage {}
message ForbiddenWordsUnderscoreMessage {}
repeated int32 int32_field = 1;
repeated TestEnum enum_field = 2;
repeated string string_field = 3;
repeated bytes bytes_field = 4;
repeated TestMessage message_field = 5;
repeated ForbiddenWordsUnderscoreMessage message_field = 5;

optional int32 int32_field_count = 11;
optional TestEnum enum_field_count = 12;
optional string string_field_count = 13;
optional bytes bytes_field_count = 14;
optional TestMessage message_field_count = 15;
optional ForbiddenWordsUnderscoreMessage message_field_count = 15;

repeated int32 Int32Field = 21; // NO_PROTO3
repeated TestEnum EnumField = 22; // NO_PROTO3
repeated string StringField = 23; // NO_PROTO3
repeated bytes BytesField = 24; // NO_PROTO3
repeated TestMessage MessageField = 25; // NO_PROTO3
repeated ForbiddenWordsUnderscoreMessage MessageField = 25; // NO_PROTO3

// This field conflicts with "int32_field" as they both generate
// the method getInt32FieldList().
Expand Down
21 changes: 15 additions & 6 deletions src/google/protobuf/compiler/java/java_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,24 @@ namespace {

const char* kDefaultPackage = "";

// Names that should be avoided as field names.
// Names that should be avoided (in UpperCamelCase format).
// Using them will cause the compiler to generate accessors whose names are
// colliding with methods defined in base classes.
const char* kForbiddenWordList[] = {
// message base class:
"cached_size",
"serialized_size",
// java.lang.Object:
"class",
"Class",
// com.google.protobuf.MessageLiteOrBuilder:
"DefaultInstanceForType",
// com.google.protobuf.MessageLite:
"ParserForType",
"SerializedSize",
// com.google.protobuf.MessageOrBuilder:
"AllFields",
"DescriptorForType",
"InitializationErrorString",
"UnknownFields",
// obsolete. kept for backwards compatibility of generated code
"CachedSize",
};

const std::unordered_set<std::string>* kReservedNames =
Expand All @@ -93,7 +102,7 @@ const std::unordered_set<std::string>* kReservedNames =

bool IsForbidden(const std::string& field_name) {
for (int i = 0; i < GOOGLE_ARRAYSIZE(kForbiddenWordList); ++i) {
if (field_name == kForbiddenWordList[i]) {
if (UnderscoresToCamelCase(field_name, true) == kForbiddenWordList[i]) {
return true;
}
}
Expand Down