Clarify map behaviors in editions.

Map fields should remain length-prefixed for now, even if DELIMITED is inherited.  Field presence will remain unchanged, but unit-tests are added to make sure proto2/proto3 behaviors stay consistent.

Closes #16549

PiperOrigin-RevId: 630191163

Author: mkruskal-google

src/google/protobuf/descriptor.cc

@@ -4362,7 +4362,8 @@ class DescriptorBuilder {
       DescriptorPool::ErrorCollector::ErrorLocation error_location,
       bool force_merge = false);
 
-  void PostProcessFieldFeatures(FieldDescriptor& field);
+  void PostProcessFieldFeatures(FieldDescriptor& field,
+                                const FieldDescriptorProto& proto);
 
   // Allocates an array of two strings, the first one is a copy of
   // `proto_name`, and the second one is the full name. Full proto name is
@@ -5542,7 +5543,8 @@ void DescriptorBuilder::ResolveFeatures(const FileDescriptorProto& proto,
                       /*force_merge=*/true);
 }
 
-void DescriptorBuilder::PostProcessFieldFeatures(FieldDescriptor& field) {
+void DescriptorBuilder::PostProcessFieldFeatures(
+    FieldDescriptor& field, const FieldDescriptorProto& proto) {
   // TODO This can be replace by a runtime check in `is_required`
   // once the `label` getter is hidden.
   if (field.features().field_presence() == FeatureSet::LEGACY_REQUIRED &&
@@ -5552,8 +5554,15 @@ void DescriptorBuilder::PostProcessFieldFeatures(FieldDescriptor& field) {
   // TODO This can be replace by a runtime check of `is_delimited`
   // once the `TYPE_GROUP` value is removed.
   if (field.type_ == FieldDescriptor::TYPE_MESSAGE &&
+      !field.containing_type()->options().map_entry() &&
       field.features().message_encoding() == FeatureSet::DELIMITED) {
-    field.type_ = FieldDescriptor::TYPE_GROUP;
+    Symbol type =
+        LookupSymbol(proto.type_name(), field.full_name(),
+                     DescriptorPool::PLACEHOLDER_MESSAGE, LOOKUP_TYPES, false);
+    if (type.descriptor() == nullptr ||
+        !type.descriptor()->options().map_entry()) {
+      field.type_ = FieldDescriptor::TYPE_GROUP;
+    }
   }
 }
 
@@ -6099,9 +6108,11 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
         });
 
     // Post-process cleanup for field features.
-    internal::VisitDescriptors(*result, [&](const FieldDescriptor& field) {
-      PostProcessFieldFeatures(const_cast<FieldDescriptor&>(field));
-    });
+    internal::VisitDescriptors(
+        *result, proto,
+        [&](const FieldDescriptor& field, const FieldDescriptorProto& proto) {
+          PostProcessFieldFeatures(const_cast<FieldDescriptor&>(field), proto);
+        });
 
     // Interpret any remaining uninterpreted options gathered into
     // options_to_interpret_ during descriptor building.  Cross-linking has made

src/google/protobuf/descriptor_unittest.cc

@@ -4059,6 +4059,22 @@ class ValidationErrorTest : public testing::Test {
     return ABSL_DIE_IF_NULL(pool_.BuildFile(file_proto));
   }
 
+  const FileDescriptor* ParseAndBuildFile(absl::string_view file_name,
+                                          absl::string_view file_text) {
+    io::ArrayInputStream input_stream(file_text.data(), file_text.size());
+    SimpleErrorCollector error_collector;
+    io::Tokenizer tokenizer(&input_stream, &error_collector);
+    compiler::Parser parser;
+    parser.RecordErrorsTo(&error_collector);
+    FileDescriptorProto proto;
+    ABSL_CHECK(parser.Parse(&tokenizer, &proto))
+        << error_collector.last_error() << "\n"
+        << file_text;
+    ABSL_CHECK_EQ("", error_collector.last_error());
+    proto.set_name(file_name);
+    return pool_.BuildFile(proto);
+  }
+
 
   // Add file_proto to the DescriptorPool. Expect errors to be produced which
   // match the given error text.
@@ -8684,7 +8700,9 @@ TEST_F(FeaturesTest, OneofFieldFeaturesOverride) {
 }
 
 TEST_F(FeaturesTest, MapFieldFeaturesOverride) {
-  constexpr absl::string_view kProtoFile = R"schema(
+  BuildDescriptorMessagesInTestPool();
+  BuildFileInTestPool(pb::TestFeatures::descriptor()->file());
+  const FileDescriptor* file = ParseAndBuildFile("foo.proto", R"schema(
     edition = "2023";
 
     import "google/protobuf/unittest_features.proto";
@@ -8701,22 +8719,7 @@ TEST_F(FeaturesTest, MapFieldFeaturesOverride) {
         features.(pb.test).multiple_feature = VALUE3
       ];
     }
-  )schema";
-  io::ArrayInputStream input_stream(kProtoFile.data(), kProtoFile.size());
-  SimpleErrorCollector error_collector;
-  io::Tokenizer tokenizer(&input_stream, &error_collector);
-  compiler::Parser parser;
-  parser.RecordErrorsTo(&error_collector);
-  FileDescriptorProto proto;
-  ASSERT_TRUE(parser.Parse(&tokenizer, &proto))
-      << error_collector.last_error() << "\n"
-      << kProtoFile;
-  ASSERT_EQ("", error_collector.last_error());
-  proto.set_name("foo.proto");
-
-  BuildDescriptorMessagesInTestPool();
-  BuildFileInTestPool(pb::TestFeatures::descriptor()->file());
-  const FileDescriptor* file = pool_.BuildFile(proto);
+  )schema");
   ASSERT_THAT(file, NotNull());
 
   const FieldDescriptor* map_field = file->message_type(0)->field(0);
@@ -8744,7 +8747,8 @@ TEST_F(FeaturesTest, MapFieldFeaturesOverride) {
 }
 
 TEST_F(FeaturesTest, MapFieldFeaturesStringValidation) {
-  constexpr absl::string_view kProtoFile = R"schema(
+  BuildDescriptorMessagesInTestPool();
+  const FileDescriptor* file = ParseAndBuildFile("foo.proto", R"schema(
     edition = "2023";
 
     message Foo {
@@ -8758,21 +8762,7 @@ TEST_F(FeaturesTest, MapFieldFeaturesStringValidation) {
         features.utf8_validation = NONE
       ];
     }
-  )schema";
-  io::ArrayInputStream input_stream(kProtoFile.data(), kProtoFile.size());
-  SimpleErrorCollector error_collector;
-  io::Tokenizer tokenizer(&input_stream, &error_collector);
-  compiler::Parser parser;
-  parser.RecordErrorsTo(&error_collector);
-  FileDescriptorProto proto;
-  ASSERT_TRUE(parser.Parse(&tokenizer, &proto))
-      << error_collector.last_error() << "\n"
-      << kProtoFile;
-  ASSERT_EQ("", error_collector.last_error());
-  proto.set_name("foo.proto");
-
-  BuildDescriptorMessagesInTestPool();
-  const FileDescriptor* file = pool_.BuildFile(proto);
+  )schema");
   ASSERT_THAT(file, NotNull());
 
   auto validate_map_field = [](const FieldDescriptor* field) {
@@ -8789,6 +8779,109 @@ TEST_F(FeaturesTest, MapFieldFeaturesStringValidation) {
   validate_map_field(file->message_type(0)->field(2));
 }
 
+TEST_F(FeaturesTest, MapFieldFeaturesImplicitPresence) {
+  BuildDescriptorMessagesInTestPool();
+  const FileDescriptor* editions = ParseAndBuildFile("editions.proto", R"schema(
+    edition = "2023";
+
+    option features.field_presence = IMPLICIT;
+
+    message Foo {
+      map<string, Foo> message_map = 1;
+      map<string, string> string_map = 2;
+    }
+  )schema");
+  ASSERT_THAT(editions, NotNull());
+  const FileDescriptor* proto3 = ParseAndBuildFile("proto3.proto", R"schema(
+    syntax = "proto3";
+
+    message Bar {
+      map<string, Bar> message_map = 1;
+      map<string, string> string_map = 2;
+    }
+  )schema");
+  ASSERT_THAT(proto3, NotNull());
+
+  auto validate_maps = [](const FileDescriptor* file) {
+    const FieldDescriptor* message_map = file->message_type(0)->field(0);
+    EXPECT_FALSE(message_map->has_presence());
+    EXPECT_FALSE(message_map->message_type()->field(0)->has_presence());
+    EXPECT_TRUE(message_map->message_type()->field(1)->has_presence());
+
+    const FieldDescriptor* string_map = file->message_type(0)->field(1);
+    EXPECT_FALSE(string_map->has_presence());
+    EXPECT_FALSE(string_map->message_type()->field(0)->has_presence());
+    EXPECT_FALSE(string_map->message_type()->field(1)->has_presence());
+  };
+  validate_maps(editions);
+  validate_maps(proto3);
+}
+
+TEST_F(FeaturesTest, MapFieldFeaturesExplicitPresence) {
+  BuildDescriptorMessagesInTestPool();
+  const FileDescriptor* editions = ParseAndBuildFile("editions.proto", R"schema(
+    edition = "2023";
+
+    message Foo {
+      map<string, Foo> message_map = 1;
+      map<string, string> string_map = 2;
+    }
+  )schema");
+  ASSERT_THAT(editions, NotNull());
+  const FileDescriptor* proto2 = ParseAndBuildFile("google.protobuf.proto", R"schema(
+    syntax = "proto2";
+
+    message Bar {
+      map<string, Bar> message_map = 1;
+      map<string, string> string_map = 2;
+    }
+  )schema");
+  ASSERT_THAT(proto2, NotNull());
+
+  auto validate_maps = [](const FileDescriptor* file) {
+    const FieldDescriptor* message_map = file->message_type(0)->field(0);
+    EXPECT_FALSE(message_map->has_presence());
+    EXPECT_TRUE(message_map->message_type()->field(0)->has_presence());
+    EXPECT_TRUE(message_map->message_type()->field(1)->has_presence());
+
+    const FieldDescriptor* string_map = file->message_type(0)->field(1);
+    EXPECT_FALSE(string_map->has_presence());
+    EXPECT_TRUE(string_map->message_type()->field(0)->has_presence());
+    EXPECT_TRUE(string_map->message_type()->field(1)->has_presence());
+  };
+  validate_maps(editions);
+  validate_maps(proto2);
+}
+
+TEST_F(FeaturesTest, MapFieldFeaturesInheritedMessageEncoding) {
+  BuildDescriptorMessagesInTestPool();
+  const FileDescriptor* file = ParseAndBuildFile("foo.proto", R"schema(
+    edition = "2023";
+    
+    option features.message_encoding = DELIMITED;
+
+    message Foo {
+      map<int32, Foo> message_map = 1;
+      map<string, string> string_map = 2;
+    }
+  )schema");
+  ASSERT_THAT(file, NotNull());
+
+  const FieldDescriptor* message_map = file->message_type(0)->field(0);
+  EXPECT_EQ(message_map->type(), FieldDescriptor::TYPE_MESSAGE);
+  EXPECT_EQ(message_map->message_type()->field(0)->type(),
+            FieldDescriptor::TYPE_INT32);
+  EXPECT_EQ(message_map->message_type()->field(1)->type(),
+            FieldDescriptor::TYPE_MESSAGE);
+
+  const FieldDescriptor* string_map = file->message_type(0)->field(1);
+  EXPECT_EQ(string_map->type(), FieldDescriptor::TYPE_MESSAGE);
+  EXPECT_EQ(string_map->message_type()->field(0)->type(),
+            FieldDescriptor::TYPE_STRING);
+  EXPECT_EQ(string_map->message_type()->field(1)->type(),
+            FieldDescriptor::TYPE_STRING);
+}
+
 TEST_F(FeaturesTest, RootExtensionFeaturesOverride) {
   BuildDescriptorMessagesInTestPool();
   BuildFileInTestPool(pb::TestFeatures::descriptor()->file());