Skip to content

Commit

Permalink
Allow open enums from editions files in proto3.
Browse files Browse the repository at this point in the history
The old check assumed all non-proto3 enums were closed, which is no longer true.

PiperOrigin-RevId: 582411371
  • Loading branch information
mkruskal-google authored and copybara-github committed Nov 14, 2023
1 parent 4773327 commit c3bae39
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 8 deletions.
10 changes: 3 additions & 7 deletions src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7711,17 +7711,13 @@ void DescriptorBuilder::ValidateProto3Field(const FieldDescriptor* field,
"Explicit default values are not allowed in proto3.");
}
if (field->cpp_type() == FieldDescriptor::CPPTYPE_ENUM &&
field->enum_type() &&
FileDescriptorLegacy(field->enum_type()->file()).syntax() !=
FileDescriptorLegacy::Syntax::SYNTAX_PROTO3 &&
FileDescriptorLegacy(field->enum_type()->file()).syntax() !=
FileDescriptorLegacy::Syntax::SYNTAX_UNKNOWN) {
// Proto3 messages can only use Proto3 enum types; otherwise we can't
field->enum_type() && field->enum_type()->is_closed()) {
// Proto3 messages can only use open enum types; otherwise we can't
// guarantee that the default value is zero.
AddError(
field->full_name(), proto, DescriptorPool::ErrorCollector::TYPE, [&] {
return absl::StrCat("Enum type \"", field->enum_type()->full_name(),
"\" is not a proto3 enum, but is used in \"",
"\" is not an open enum, but is used in \"",
field->containing_type()->full_name(),
"\" which is a proto3 message type.");
});
Expand Down
65 changes: 64 additions & 1 deletion src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7034,10 +7034,73 @@ TEST_F(ValidationErrorTest, ValidateProto3EnumFromProto2) {
" field { name:'bar' number:1 label:LABEL_OPTIONAL type:TYPE_ENUM "
" type_name: 'foo.FooEnum' }"
"}",
"bar.proto: Foo.bar: TYPE: Enum type \"foo.FooEnum\" is not a proto3 "
"bar.proto: Foo.bar: TYPE: Enum type \"foo.FooEnum\" is not an open "
"enum, but is used in \"Foo\" which is a proto3 message type.\n");
}

TEST_F(ValidationErrorTest, ValidateProto3ClosedEnum) {
// Define a closed enum in an editions file.
BuildFile(R"pb(name: 'foo.proto'
package: 'foo'
syntax: 'editions'
edition: EDITION_2023
enum_type {
name: 'FooEnum'
value { name: 'DEFAULT_OPTION' number: 0 }
options { features { enum_type: CLOSED } }
})pb");

BuildFileWithErrors(
R"pb(name: 'bar.proto'
dependency: 'foo.proto'
syntax: 'proto3'
message_type {
name: 'Foo'
field {
name: 'bar'
number: 1
label: LABEL_OPTIONAL
type: TYPE_ENUM
type_name: 'foo.FooEnum'
}
})pb",
"bar.proto: Foo.bar: TYPE: Enum type \"foo.FooEnum\" is not an open "
"enum, but is used in \"Foo\" which is a proto3 message type.\n");
}

TEST_F(ValidationErrorTest, ValidateProto3OpenEnum) {
// Define an open enum in an editions file.
const FileDescriptor* foo =
BuildFile(R"pb(name: 'foo.proto'
package: 'foo'
syntax: 'editions'
edition: EDITION_2023
enum_type {
name: 'FooEnum'
value { name: 'DEFAULT_OPTION' number: 0 }
})pb");
const EnumDescriptor* enm = foo->enum_type(0);
ASSERT_NE(enm, nullptr);

const FileDescriptor* bar = BuildFile(
R"pb(name: 'bar.proto'
dependency: 'foo.proto'
syntax: 'proto3'
message_type {
name: 'Foo'
field {
name: 'bar'
number: 1
label: LABEL_OPTIONAL
type: TYPE_ENUM
type_name: 'foo.FooEnum'
}
})pb");
ASSERT_NE(bar, nullptr);

EXPECT_EQ(bar->message_type(0)->field(0)->enum_type(), enm);
}

TEST_F(ValidationErrorTest, ValidateProto3Extension) {
// Valid for options.
DescriptorPool pool;
Expand Down

0 comments on commit c3bae39

Please sign in to comment.