Preserve features in type resolver

mkruskal-google · copybara-github · commit c7030f4f24a9 · 2025-08-05T15:32:53.000-07:00
While we previously preserved unresolved features set directly on message and enum types and the edition specified in the file, and features set at the file or nesting scopes were discarded.  This throws away important semantic information, resulting in incorrect Type and Enum objects under editions.

This change solves the problem by partially resolving features down to the scope returned.  Users will still need to figure out the defaults for each edition on their own, but these can now be converted back into descriptors and built correctly.

PiperOrigin-RevId: 791393429
diff --git a/src/google/protobuf/util/type_resolver_util.cc b/src/google/protobuf/util/type_resolver_util.cc
@@ -21,6 +21,7 @@
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/strip.h"
+#include "google/protobuf/descriptor.h"
 #include "google/protobuf/io/strtod.h"
 #include "google/protobuf/util/type_resolver.h"
 
@@ -362,6 +363,30 @@ class DescriptorPoolTypeResolver : public TypeResolver {
   const DescriptorPool* pool_;
 };
 
+template <typename DescriptorT, typename DescriptorProtoT>
+void PartiallyResolveFeatures(const FileDescriptorProto& file,
+                              const DescriptorT& descriptor,
+                              DescriptorProtoT& proto) {
+  FeatureSet features = file.options().features();
+  std::vector<FeatureSet> nested_features;
+  const Descriptor* parent = descriptor.containing_type();
+  while (parent != nullptr) {
+    DescriptorProto parent_proto;
+    parent->CopyHeadingTo(&parent_proto);
+    if (parent_proto.options().has_features()) {
+      nested_features.push_back(parent_proto.options().features());
+    }
+    parent = parent->containing_type();
+  }
+  for (int i = nested_features.size() - 1; i >= 0; --i) {
+    features.MergeFrom(nested_features[i]);
+  }
+  if (features.ByteSizeLong() > 0) {
+    features.MergeFrom(proto.options().features());
+    *proto.mutable_options()->mutable_features() = features;
+  }
+}
+
 }  // namespace
 
 TypeResolver* NewTypeResolverForDescriptorPool(absl::string_view url_prefix,
@@ -376,6 +401,7 @@ Type ConvertDescriptorToType(absl::string_view url_prefix,
   FileDescriptorProto proto;
   descriptor.file()->CopyHeadingTo(&proto);
   descriptor.CopyTo(proto.add_message_type());
+  PartiallyResolveFeatures(proto, descriptor, *proto.mutable_message_type(0));
   ConvertDescriptor(url_prefix, descriptor, proto, proto.message_type(0),
                     &type);
   return type;
@@ -387,6 +413,7 @@ Enum ConvertDescriptorToType(const EnumDescriptor& descriptor) {
   FileDescriptorProto proto;
   descriptor.file()->CopyHeadingTo(&proto);
   descriptor.CopyTo(proto.add_enum_type());
+  PartiallyResolveFeatures(proto, descriptor, *proto.mutable_enum_type(0));
   ConvertEnumDescriptor(descriptor, proto, proto.enum_type(0), &enum_type);
   return enum_type;
 }
diff --git a/src/google/protobuf/util/type_resolver_util_test.cc b/src/google/protobuf/util/type_resolver_util_test.cc
@@ -520,6 +520,62 @@ TEST_F(DescriptorPoolTypeResolverSyntaxTest, EditionsFieldFeatures) {
               )pb"));
 }
 
+TEST_F(DescriptorPoolTypeResolverSyntaxTest, EditionsInheritedMessageFeatures) {
+  ASSERT_NE(BuildFile(R"pb(
+              package: "test"
+              name: "foo"
+              syntax: "editions"
+              edition: EDITION_2023
+              options {
+                features {
+                  repeated_field_encoding: EXPANDED
+                  [pb.cpp] { string_type: CORD }
+                }
+              }
+              message_type {
+                name: "Parent"
+                options {
+                  features {
+                    field_presence: IMPLICIT
+                    [pb.cpp] { string_type: VIEW }
+                  }
+                }
+                nested_type {
+                  name: "MyMessage"
+                  field { name: "field" number: 1 type: TYPE_BYTES }
+                }
+              }
+            )pb"),
+            nullptr);
+
+  Type type;
+  ASSERT_TRUE(
+      resolver_->ResolveMessageType(GetTypeUrl("test.Parent.MyMessage"), &type)
+          .ok());
+  EXPECT_THAT(type, EqualsProto(R"pb(
+                name: "test.Parent.MyMessage"
+                fields {
+                  kind: TYPE_BYTES
+                  cardinality: CARDINALITY_OPTIONAL
+                  number: 1
+                  name: "field"
+                  json_name: "field"
+                }
+                options {
+                  name: "features"
+                  value {
+                    [type.googleapis.com/google.protobuf.FeatureSet] {
+                      field_presence: IMPLICIT
+                      repeated_field_encoding: EXPANDED
+                      [pb.cpp] { string_type: VIEW }
+                    }
+                  }
+                }
+                source_context { file_name: "foo" }
+                syntax: SYNTAX_EDITIONS
+                edition: "2023")pb"));
+}
+
 TEST_F(DescriptorPoolTypeResolverSyntaxTest, EditionsEnumFeatures) {
   BuildFile(R"pb(
     package: "test"
@@ -551,6 +607,47 @@ TEST_F(DescriptorPoolTypeResolverSyntaxTest, EditionsEnumFeatures) {
       )pb"));
 }
 
+TEST_F(DescriptorPoolTypeResolverSyntaxTest, EditionsEnumInheritedFeatures) {
+  ASSERT_NE(BuildFile(R"pb(
+              package: "test"
+              name: "foo"
+              syntax: "editions"
+              edition: EDITION_2023
+              options { features { repeated_field_encoding: EXPANDED } }
+              message_type {
+                name: "Parent"
+                options { features { field_presence: IMPLICIT } }
+                enum_type {
+                  name: "MyEnum"
+                  options { features { enum_type: CLOSED } }
+                  value: { name: "FOO" number: 1 }
+                }
+              }
+            )pb"),
+            nullptr);
+
+  Enum enm;
+  ASSERT_TRUE(
+      resolver_->ResolveEnumType(GetTypeUrl("test.Parent.MyEnum"), &enm).ok());
+  EXPECT_THAT(enm, EqualsProto(R"pb(
+                name: "test.Parent.MyEnum"
+                enumvalue { name: "FOO" number: 1 }
+                options {
+                  name: "features"
+                  value {
+                    [type.googleapis.com/google.protobuf.FeatureSet] {
+                      field_presence: IMPLICIT
+                      enum_type: CLOSED
+                      repeated_field_encoding: EXPANDED
+                    }
+                  }
+                }
+                source_context { file_name: "foo" }
+                syntax: SYNTAX_EDITIONS
+                edition: "2023"
+              )pb"));
+}
+
 TEST(ConvertDescriptorToTypeTest, TestAllTypes) {
   Type type = ConvertDescriptorToType(
       kUrlPrefix, *proto2_unittest::TestAllTypes::GetDescriptor());
