From c0bd3dd984399d3302a8f7cf69e7c9330b362ac4 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 24 Feb 2023 19:47:10 -0800 Subject: [PATCH] Implement helper function FieldDescriptor::legacy_enum_field_treated_as_closed to support legacy enum field behavior. Proto2 fields with types corresponding to proto3 enums are treated as closed today, despite the EnumDescriptor reporting that they're open. This will allow syntax users to depend on this non-conformant behavior which will be getting it's own feature in Edition zero. PiperOrigin-RevId: 512218773 --- src/google/protobuf/descriptor.h | 24 +++++++++ src/google/protobuf/descriptor_unittest.cc | 60 ++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index 22bc03c745..c3be72bd34 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -769,6 +769,25 @@ class PROTOBUF_EXPORT FieldDescriptor : private internal::SymbolBase { // parse. bool requires_utf8_validation() const; + // Determines if the given enum field is treated as closed based on legacy + // non-conformant behavior. + // + // Conformant behavior determines closedness based on the enum and + // can be queried using EnumDescriptor::is_closed(). + // + // Some runtimes currently have a quirk where non-closed enums are + // treated as closed when used as the type of fields defined in a + // `syntax = proto2;` file. This quirk is not present in all runtimes; as of + // writing, we know that: + // + // - C++, Java, and C++-based Python share this quirk. + // - UPB and UPB-based Python do not. + // - PHP and Ruby treat all enums as open regardless of declaration. + // + // Care should be taken when using this function to respect the target + // runtime's enum handling quirks. + bool legacy_enum_field_treated_as_closed() const; + // Index of this field within the message's field array, or the file or // extension scope's extensions array. int index() const; @@ -2443,6 +2462,11 @@ inline bool FieldDescriptor::requires_utf8_validation() const { file()->syntax() == FileDescriptor::SYNTAX_PROTO3; } +inline bool FieldDescriptor::legacy_enum_field_treated_as_closed() const { + return type() == TYPE_ENUM && + file()->syntax() == FileDescriptor::SYNTAX_PROTO2; +} + // To save space, index() is computed by looking at the descriptor's position // in the parent's array of children. inline int FieldDescriptor::index() const { diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 64e6b2c52a..c4c7e933c0 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -1120,6 +1120,66 @@ TEST_F(DescriptorTest, NeedsUtf8Check) { EXPECT_FALSE(bar3->requires_utf8_validation()); } +TEST_F(DescriptorTest, EnumFieldTreatedAsClosed) { + // Make an open enum definition. + FileDescriptorProto open_enum_file; + open_enum_file.set_name("open_enum.proto"); + open_enum_file.set_syntax("proto3"); + AddEnumValue(AddEnum(&open_enum_file, "TestEnumOpen"), "TestEnumOpen_VALUE0", + 0); + + const EnumDescriptor* open_enum = + pool_.BuildFile(open_enum_file)->enum_type(0); + EXPECT_FALSE(open_enum->is_closed()); + + // Create a message that treats enum fields as closed. + FileDescriptorProto closed_file; + closed_file.set_name("closed_enum_field.proto"); + closed_file.add_dependency("open_enum.proto"); + closed_file.add_dependency("foo.proto"); + + DescriptorProto* message = AddMessage(&closed_file, "TestClosedEnumField"); + AddField(message, "int_field", 1, FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_INT32); + AddField(message, "open_enum", 2, FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_ENUM) + ->set_type_name("TestEnumOpen"); + AddField(message, "closed_enum", 3, FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_ENUM) + ->set_type_name("TestEnum"); + const Descriptor* closed_message = + pool_.BuildFile(closed_file)->message_type(0); + + EXPECT_FALSE(closed_message->FindFieldByName("int_field") + ->legacy_enum_field_treated_as_closed()); + EXPECT_TRUE(closed_message->FindFieldByName("closed_enum") + ->legacy_enum_field_treated_as_closed()); + EXPECT_TRUE(closed_message->FindFieldByName("open_enum") + ->legacy_enum_field_treated_as_closed()); +} + +TEST_F(DescriptorTest, EnumFieldTreatedAsOpen) { + FileDescriptorProto open_enum_file; + open_enum_file.set_name("open_enum.proto"); + open_enum_file.set_syntax("proto3"); + AddEnumValue(AddEnum(&open_enum_file, "TestEnumOpen"), "TestEnumOpen_VALUE0", + 0); + DescriptorProto* message = AddMessage(&open_enum_file, "TestOpenEnumField"); + AddField(message, "int_field", 1, FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_INT32); + AddField(message, "open_enum", 2, FieldDescriptorProto::LABEL_OPTIONAL, + FieldDescriptorProto::TYPE_ENUM) + ->set_type_name("TestEnumOpen"); + const FileDescriptor* open_enum_file_desc = pool_.BuildFile(open_enum_file); + const Descriptor* open_message = open_enum_file_desc->message_type(0); + const EnumDescriptor* open_enum = open_enum_file_desc->enum_type(0); + EXPECT_FALSE(open_enum->is_closed()); + EXPECT_FALSE(open_message->FindFieldByName("int_field") + ->legacy_enum_field_treated_as_closed()); + EXPECT_FALSE(open_message->FindFieldByName("open_enum") + ->legacy_enum_field_treated_as_closed()); +} + TEST_F(DescriptorTest, IsMap) { EXPECT_TRUE(map_->is_map()); EXPECT_FALSE(baz_->is_map());