Validate Feature Support on Custom Options

PiperOrigin-RevId: 864384082

Author: karenwuz

src/google/protobuf/compiler/command_line_interface_unittest.cc

@@ -1652,6 +1652,36 @@ TEST_F(CommandLineInterfaceTest, ValidateFeatureSupportValid) {
   ExpectNoErrors();
 }
 
+TEST_F(CommandLineInterfaceTest, ValidateFeatureSupportLifetimesOptionRemoved) {
+  CreateTempFile("google/protobuf/descriptor.proto",
+                 google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
+  CreateTempFile("foo.proto",
+                 R"schema(
+    edition = "2024";
+    import "google/protobuf/descriptor.proto";
+
+    option features.field_presence = IMPLICIT;
+
+    extend google.protobuf.MessageOptions {
+      bool removed_option = 7733026 [feature_support = {
+        edition_removed: EDITION_2023
+        removal_error: "removed_option removal error"}];
+    }
+    message Foo {
+      option (removed_option) = true;
+      int32 bar = 1 [
+        feature_support = {
+          edition_removed: EDITION_2023
+          removal_error: "Custom removal error"
+        }
+      ];
+    })schema");
+  Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir foo.proto");
+  ExpectErrorSubstring(
+      "removed_option has been removed in edition 2023: removed_option removal "
+      "error");
+}
+
 TEST_F(CommandLineInterfaceTest, FeatureValidationError) {
   CreateTempFile("foo.proto",
                  R"schema(

src/google/protobuf/descriptor.cc

@@ -1608,7 +1608,6 @@ class DescriptorPool::DeferredValidation {
   struct LifetimesInfo {
     const Message* option_to_validate;  // features or options to validate
     const Message* proto;
-    bool is_feature;
     absl::string_view full_name;
     absl::string_view filename;
   };
@@ -1637,20 +1636,16 @@ class DescriptorPool::DeferredValidation {
       return true;
     }
 
-    static absl::string_view feature_set_name = "google.protobuf.FeatureSet";
     bool has_errors = false;
     for (const auto& it : lifetimes_info_map_) {
       const FileDescriptor* file = it.first;
 
       for (const auto& info : it.second) {
-        const Descriptor* feature_set_descriptor = nullptr;
-        // TODO: Support custom options
-        if (info.is_feature) {
-          feature_set_descriptor =
-              pool_->FindMessageTypeByName(feature_set_name);
-        }
+        const Descriptor* option_descriptor = nullptr;
+        option_descriptor = pool_->FindMessageTypeByName(
+            info.option_to_validate->GetTypeName());
         auto results = FeatureResolver::ValidateFeatureLifetimes(
-            file->edition(), *info.option_to_validate, feature_set_descriptor);
+            file->edition(), *info.option_to_validate, option_descriptor);
         for (const auto& error : results.errors) {
           has_errors = true;
           if (error_collector_ == nullptr) {
@@ -6690,15 +6685,13 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
         *result, proto, [&](const auto& descriptor, const auto& desc_proto) {
           if (!IsDefaultInstance(*descriptor.proto_features_)) {
             deferred_validation_.ValidateFeatureLifetimes(
-                GetFile(descriptor),
-                {descriptor.proto_features_, &desc_proto, /*is_feature=*/true,
-                 GetFullName(descriptor), proto.name()});
+                GetFile(descriptor), {descriptor.proto_features_, &desc_proto,
+                                      GetFullName(descriptor), proto.name()});
           }
           if (!IsDefaultInstance(*descriptor.options_)) {
             deferred_validation_.ValidateFeatureLifetimes(
-                GetFile(descriptor),
-                {descriptor.options_, &desc_proto,
-                 /*is_feature=*/false, GetFullName(descriptor), proto.name()});
+                GetFile(descriptor), {descriptor.options_, &desc_proto,
+                                      GetFullName(descriptor), proto.name()});
           }
         });
   }

src/google/protobuf/descriptor_test_utils.cc

@@ -135,6 +135,15 @@ void ValidationErrorTest::ParseAndBuildFileWithErrorSubstr(
   EXPECT_THAT(error_collector.text_, HasSubstr(expected_errors));
 }
 
+void ValidationErrorTest::ParseAndBuildFileWithWarningSubstr(
+    absl::string_view file_name, absl::string_view file_text,
+    absl::string_view expected_warning) {
+  MockErrorCollector error_collector;
+  EXPECT_TRUE(pool_.BuildFileCollectingErrors(ParseFile(file_name, file_text),
+                                              &error_collector));
+  EXPECT_THAT(error_collector.warning_text_, HasSubstr(expected_warning));
+}
+
 // Parse file_text as a FileDescriptorProto in text format and add it
 // to the DescriptorPool.  Expect errors to be produced which match the
 // given warning text.

src/google/protobuf/descriptor_test_utils.h

@@ -97,6 +97,10 @@ class ValidationErrorTest : public testing::Test {
                                         absl::string_view file_text,
                                         absl::string_view expected_errors);
 
+  void ParseAndBuildFileWithWarningSubstr(absl::string_view file_name,
+                                          absl::string_view file_text,
+                                          absl::string_view expected_warning);
+
   // Parse file_text as a FileDescriptorProto in text format and add it
   // to the DescriptorPool.  Expect errors to be produced which match the
   // given warning text.

src/google/protobuf/descriptor_unittest.cc

@@ -4052,7 +4052,7 @@ TEST_P(AllowUnknownDependenciesTest, MissingTypeAggregateOption) {
   EXPECT_EQ(file->options().uninterpreted_option_size(), 1);
 }
 
-TEST_P(AllowUnknownDependenciesTest, MissinNestedTypeAggregateOption) {
+TEST_P(AllowUnknownDependenciesTest, MissingNestedTypeAggregateOption) {
   // Test that we can use an aggregate custom option with a missing type.
 
   FileDescriptorProto file_proto;
@@ -4112,15 +4112,15 @@ TEST_P(AllowUnknownDependenciesTest, MissinNestedTypeAggregateOption) {
   const Descriptor* message = file->FindMessageTypeByName("MessageMissing");
   ASSERT_NE(message, nullptr);
   std::vector<const FieldDescriptor*> fields;
-  file->options().GetReflection()->ListFields(message->options(), &fields);
+  message->options().GetReflection()->ListFields(message->options(), &fields);
   EXPECT_EQ(fields.size(), 1);
   EXPECT_EQ(message->options().unknown_fields().field_count(), 0);
   EXPECT_EQ(message->options().uninterpreted_option_size(), 1);
 
   // Aggregate options without missing types should be resolved.
   message = file->FindMessageTypeByName("MessageValid");
   ASSERT_NE(message, nullptr);
-  file->options().GetReflection()->ListFields(message->options(), &fields);
+  message->options().GetReflection()->ListFields(message->options(), &fields);
   EXPECT_EQ(fields.size(), 0);
   EXPECT_EQ(message->options().unknown_fields().field_count(), 1);
   EXPECT_EQ(message->options().uninterpreted_option_size(), 0);
@@ -12907,6 +12907,61 @@ TEST_F(FeaturesTest, ExistingUnstableFeatureDefault) {
       pb::UNSTABLE3);
 }
 
+TEST_F(FeaturesTest, FeatureLifetimesOptionRemoved) {
+  BuildDescriptorMessagesInTestPool();
+  ParseAndBuildFileWithErrorSubstr(
+      "featurelifetimes.proto", R"schema(
+    edition = "2024";
+    package featurelifetimes;
+    import "google/protobuf/descriptor.proto";
+
+    extend google.protobuf.MessageOptions {
+      bool removed_option = 7733026 [feature_support = {
+        edition_removed: EDITION_2023
+        removal_error: "removed_option removal error"
+      }];
+    }
+    message SomeMessage {
+      option (removed_option) = true;
+    }
+  )schema",
+      "featurelifetimes.removed_option has been removed in edition 2023: "
+      "removed_option removal error");
+}
+
+TEST_F(FeaturesTest, FeatureLifetimesOptionDeprecated) {
+  BuildDescriptorMessagesInTestPool();
+
+  ParseAndBuildFile("featurelifetimes.proto", R"schema(
+    edition = "2024";
+    package featurelifetimes;
+    import "google/protobuf/descriptor.proto";
+
+    extend google.protobuf.MessageOptions {
+       bool deprecated_option = 7733026 [feature_support = {
+        edition_deprecated: EDITION_2023
+        deprecation_warning: "deprecated_option deprecation warning"
+      }];
+    }
+  )schema");
+
+  pool_.AddDirectInputFile("some_message.proto");
+  ParseAndBuildFileWithWarningSubstr(
+      "some_message.proto",
+      R"schema(
+      edition = "2024";
+      package some_message;
+      import "google/protobuf/descriptor.proto";
+      import "featurelifetimes.proto";
+
+      message SomeMessage {
+        option (featurelifetimes.deprecated_option) = true;
+      }
+    )schema",
+      "featurelifetimes.deprecated_option has been deprecated in edition 2023: "
+      "deprecated_option deprecation warning");
+}
+
 // Test that the result of FileDescriptor::DebugString() can be used to create
 // the original descriptors.
 class FeaturesDebugStringTest

src/google/protobuf/feature_resolver.cc

@@ -340,50 +340,62 @@ void ValidateSingleFeatureLifetimes(
     results.errors.emplace_back(std::move(error_message));
   } else if (feature_support.has_edition_deprecated() &&
              edition >= feature_support.edition_deprecated()) {
-    std::string error_message = absl::Substitute(
+    std::string warning_message = absl::Substitute(
         "$0 has been deprecated in edition "
         "$1: $2",
         full_name, feature_support.edition_deprecated(),
         feature_support.deprecation_warning());
-    results.warnings.emplace_back(std::move(error_message));
+    results.warnings.emplace_back(std::move(warning_message));
   }
 }
 
 void ValidateFeatureLifetimesImpl(Edition edition, const Message& message,
-                                  FeatureResolver::ValidationResults& results,
-                                  bool is_feature) {
+                                  FeatureResolver::ValidationResults& results) {
   std::vector<const FieldDescriptor*> fields;
   message.GetReflection()->ListFields(message, &fields);
   for (const FieldDescriptor* field : fields) {
-    // TODO: Support repeated option enum values and custom options
-    // feature support
-    if (is_feature) {
-      // Recurse into message extension.
-      if (field->is_extension() &&
-          field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
+    const Reflection* reflector = message.GetReflection();
+    // Recurse into all Messages to be validated
+    if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
+      // Recursing into repeated Messages
+      if (field->is_repeated()) {
+        for (int index = 0; index < reflector->FieldSize(message, field);
+             index++) {
+          ValidateFeatureLifetimesImpl(
+              edition, reflector->GetRepeatedMessage(message, field, index),
+              results);
+        }
+      } else {
         ValidateFeatureLifetimesImpl(
-            edition, message.GetReflection()->GetMessage(message, field),
-            results, is_feature);
-        continue;
+            edition, reflector->GetMessage(message, field), results);
       }
-
-      if (field->enum_type() != nullptr) {
-        int number = message.GetReflection()->GetEnumValue(message, field);
+    }
+    // Validating ENUM value
+    if (field->enum_type() != nullptr) {
+      // Handling repeated enum values. Ex: OptionTargetType option
+      if (field->is_repeated()) {
+        for (int index = 0; index < reflector->FieldSize(message, field);
+             index++) {
+          int number = reflector->GetRepeatedEnumValue(message, field, index);
+          auto value = field->enum_type()->FindValueByNumber(number);
+          if (value == nullptr) {
+            continue;
+          }
+          ValidateSingleFeatureLifetimes(edition, value->full_name(),
+                                         value->options().feature_support(),
+                                         results);
+        }
+      } else {
+        int number = reflector->GetEnumValue(message, field);
         auto value = field->enum_type()->FindValueByNumber(number);
         if (value == nullptr) {
-          results.errors.emplace_back(absl::StrCat(
-              "Feature ", field->full_name(), " has no known value ", number));
           continue;
         }
         ValidateSingleFeatureLifetimes(edition, value->full_name(),
                                        value->options().feature_support(),
                                        results);
       }
     }
-    // TODO: Support custom options
-    if (field->is_extension()) {
-      continue;
-    }
     ValidateSingleFeatureLifetimes(edition, field->full_name(),
                                    field->options().feature_support(), results);
   }
@@ -521,10 +533,7 @@ FeatureResolver::ValidationResults FeatureResolver::ValidateFeatureLifetimes(
   const Message* pool_option = nullptr;
   DynamicMessageFactory factory;
   std::unique_ptr<Message> message_storage;
-  bool is_feature =
-      option.GetDescriptor()->name() == FeatureSet::descriptor()->name();
-  // TODO: Support custom options
-  if (pool_descriptor != nullptr && is_feature) {
+  if (pool_descriptor != nullptr) {
     // Move the messages back to the current pool so that we can reflect on
     // any extensions.
     message_storage =
@@ -543,7 +552,7 @@ FeatureResolver::ValidationResults FeatureResolver::ValidateFeatureLifetimes(
 
   ValidationResults results;
   // Validate feature support
-  ValidateFeatureLifetimesImpl(edition, *pool_option, results, is_feature);
+  ValidateFeatureLifetimesImpl(edition, *pool_option, results);
 
   return results;
 }