Message FieldNameEntry not allowed as map value type for field_name

[ergl]

What version of protobuf and what language are you using?
Version: stable 3.19.4
Language: N/A

What operating system (Linux, Windows, ...) and version?

macOS 11.6.4

What runtime / compiler are you using (e.g., python version or gcc version)

N/A, happens before language-specifc code generation

What did you do?

Steps to reproduce the behavior:

1. Attempt to use a message with a name of FieldNameEntry as a map value in field field_name

2. See protoc return the error: map_entry should not be set explicitly. Use map<KeyType, ValueType> instead.

Minimal declaration:

syntax = "proto3"; // also happens on proto2

message AEntry {}

message Container {
    map<uint32, AEntry> a = 1;
}

What did you expect to see

I expected the declaration above to compile cleanly and with no errors.

What did you see instead?

The compiler emits map_entry should not be set explicitly. Use map<KeyType, ValueType> instead. before reaching any language-specific generator.

It took me a bit to realize this has to do with backwards compatibility between maps and repeated values, but the error message did not make sense to me at first, and I did not expect protoc to forbid this usage.

If this is an expected error, perhaps it should be documented (apologies if it is already, and I didn't see it)

[elharo]

Interesting. Do you have a complete reproducible test case, ideally as a failing PR here? I don't immediately see how the code corresponds to the report.

[ergl]

@elharo I don't have a failing PR at hand, sorry, but take the following file (let's say, test.proto)

syntax = "proto3";

message FooEntry {}

message Container {
    map<uint32, FooEntry> foo = 1;
}

If we try to generate code with protoc -I=. --python_out . test.proto, it will error:

# protoc -I=. --python_out . test.proto
test.proto: map_entry should not be set explicitly. Use map<KeyType, ValueType> instead.

I put --python_out above but any other language would work.

I don't immediately see how the code corresponds to the report.

Sorry for any confusion. What I mean is that if a field named foo (or any other name) has a type like map<_, FooEntry> (notice the matching name between the field and the map value type), then this error will be raised.

In my original example, the field is named a and the type is named AEntry. In the above example, the field is named foo and the type is FooEntry.

For snake_case fields, like field_name, the problematic type would need to be named the same, but transformed to CamelCase (FieldNameEntry).

[ergl]

The relevant checks happen here:

protobuf/src/google/protobuf/descriptor.cc

Lines 6905 to 6906 in 2f91da5

	bool DescriptorBuilder::ValidateMapEntry(FieldDescriptor* field,
	const FieldDescriptorProto& proto) {

[elharo]

Thanks for the additional info. Spec says, "The value_type can be any type except another map." so this should be OK. Assuming I can reproduce this, it's probably a bug.

[esorot]

The naming convention reserves *Entry for internal usage:

protobuf/src/google/protobuf/descriptor.cc

Line 6917 in 2f91da5

message->name() != ToCamelCase(field->name(), false) + "Entry" ||

Closing this as this is WAD

[parthea]

It appears that *Entry is a reserved field name. In the interim I'll add workaround in my local code to rename the field to *Entry_ . Please could we re-open this issue? It would be great to have a more elegant solution.

[elharo]

I agree. This isn't documented in the spec and protoc should generate name mangling as needed to make this work.

[ergl]

Moving the *Entry message definition inside the message that contains the map field gives a more clarifying error message.

syntax = "proto3";

message Container {
    message FooEntry {}
    map<uint32, FooEntry> foo = 1;
}

Trying to compile the file above prints the following error:

# protoc -I=. --python_out . test.proto
test.proto: "FooEntry" is already defined in "Container".
test.proto:3:9: Expanded map entry type FooEntry conflicts with an existing nested message type.