From 03223d0cd6a25d0488d3bb3af591c480ce08c243 Mon Sep 17 00:00:00 2001 From: Karen Wu Date: Mon, 1 Dec 2025 12:33:20 -0800 Subject: [PATCH] Generalizing and implementing ValidateFeatureSupport for both Options and Features during proto parsing PiperOrigin-RevId: 838888348 --- src/google/protobuf/descriptor.cc | 89 +- src/google/protobuf/descriptor_unittest.cc | 905 ++++++++++++++++++ src/google/protobuf/feature_resolver.cc | 143 ++- src/google/protobuf/feature_resolver.h | 3 + src/google/protobuf/feature_resolver_test.cc | 502 +--------- .../protobuf/unittest_custom_features.proto | 2 + src/google/protobuf/unittest_features.proto | 2 + 7 files changed, 1061 insertions(+), 585 deletions(-) diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 2931c67298618..688d051129b3e 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -1637,7 +1637,9 @@ class DescriptorPool::DeferredValidation { } bool Validate() { - if (lifetimes_info_map_.empty()) return true; + if (lifetimes_info_map_.empty()) { + return true; + } static absl::string_view feature_set_name = "google.protobuf.FeatureSet"; const Descriptor* feature_set = @@ -5014,6 +5016,9 @@ class DescriptorBuilder { const DescriptorProto::ExtensionRange& proto) {} void ValidateExtensionRangeOptions(const DescriptorProto& proto, const Descriptor& message); + void ValidateFeatureSupport( + const FieldOptions::FeatureSupport& feature_support, const Message& proto, + absl::string_view full_name); void ValidateExtensionDeclaration( absl::string_view full_name, const RepeatedPtrField& declarations, @@ -8487,12 +8492,84 @@ void DescriptorBuilder::ValidateOptions(const OneofDescriptor* /*oneof*/, } +void DescriptorBuilder::ValidateFeatureSupport( + const FieldOptions::FeatureSupport& feature_support, const Message& proto, + absl::string_view full_name) { + std::string feature_support_error( + FeatureResolver::ValidateFeatureSupport(feature_support, full_name) + .message()); + if (!feature_support_error.empty()) { + AddError(full_name, proto, DescriptorPool::ErrorCollector::OPTION_NAME, + feature_support_error.c_str()); + } +} + void DescriptorBuilder::ValidateOptions(const FieldDescriptor* field, const FieldDescriptorProto& proto) { if (pool_->lazily_build_dependencies_ && (!field || !field->message_type())) { return; } + if (field->options().has_feature_support()) { + const FieldOptions::FeatureSupport field_feature_support = + field->options().feature_support(); + ValidateFeatureSupport(field_feature_support, proto, field->full_name()); + if (field->enum_type() != nullptr) { + for (int i = 0; i < field->enum_type()->value_count(); i++) { + const EnumValueDescriptor& value = *field->enum_type()->value(i); + // We allow missing support windows on feature values, and they'll + // inherit from the feature spec. + if (!value.options().has_feature_support()) { + continue; + } + FieldOptions::FeatureSupport value_feature_support = + field_feature_support; + value_feature_support.MergeFrom(value.options().feature_support()); + ValidateFeatureSupport(value_feature_support, proto, value.full_name()); + + // Make sure enum values don't expand any bounds + if (field_feature_support.edition_introduced() > + value_feature_support.edition_introduced()) { + AddError(value.full_name(), proto, + DescriptorPool::ErrorCollector::OPTION_NAME, + absl::Substitute("value $0 was introduced before $1 was.", + value.full_name(), field->full_name()) + .c_str()); + } + if (field_feature_support.has_edition_removed() && + field_feature_support.edition_removed() < + value_feature_support.edition_removed()) { + AddError(value.full_name(), proto, + DescriptorPool::ErrorCollector::OPTION_NAME, + absl::Substitute("value $0 was removed after $1 was.", + value.full_name(), field->full_name()) + .c_str()); + } + if (field_feature_support.has_edition_deprecated() && + field_feature_support.edition_deprecated() < + value_feature_support.edition_deprecated()) { + AddError(value.full_name(), proto, + DescriptorPool::ErrorCollector::OPTION_NAME, + absl::Substitute("value $0 was deprecated after $1 was.", + value.full_name(), field->full_name()) + .c_str()); + } + } + } + } else if (field->enum_type() != nullptr) { + for (int i = 0; i < field->enum_type()->value_count(); i++) { + const EnumValueDescriptor& value = *field->enum_type()->value(i); + if (value.options().has_feature_support()) { + AddError(value.full_name(), proto, + DescriptorPool::ErrorCollector::OPTION_NAME, + absl::Substitute( + "value $0 is not allowed to have feature support " + "because its field $1 doesn't have feature support.", + value.full_name(), field->full_name()) + .c_str()); + } + } + } ValidateFieldFeatures(field, proto); @@ -8822,10 +8899,12 @@ void DescriptorBuilder::ValidateOptions(const EnumDescriptor* enm, } } -void DescriptorBuilder::ValidateOptions( - const EnumValueDescriptor* /* enum_value */, - const EnumValueDescriptorProto& /* proto */) { - // Nothing to do so far. +void DescriptorBuilder::ValidateOptions(const EnumValueDescriptor* enum_value, + const EnumValueDescriptorProto& proto) { + if (enum_value->options().has_feature_support()) { + ValidateFeatureSupport(enum_value->options().feature_support(), proto, + enum_value->full_name()); + } } namespace { diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 6ab352c60c506..23f4678ec654d 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -4693,6 +4693,911 @@ TEST(CustomOptions, DebugString) { descriptor->DebugString()); } +TEST(CustomOptions, FeatureSupportMissingDeprecationWarning) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { edition_deprecated: EDITION_99997_TEST_ONLY } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: proto" + "2_unittest.Foo.Bar is deprecated but does not specify a " + "deprecation warning.\n"); +} + +TEST(CustomOptions, FeatureSupportMissingDeprecation) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { + deprecation_warning: "Custom deprecation warning" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: proto" + "2_unittest.Foo.Bar specifies a deprecation warning but is not " + "marked deprecated in any edition.\n"); +} + +TEST(CustomOptions, FeatureSupportMissingRemovalError) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { edition_removed: EDITION_99997_TEST_ONLY } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: proto" + "2_unittest.Foo.Bar has been removed but does not specify a " + "removal error.\n"); +} + +TEST(CustomOptions, FeatureSupportMissingRemoval) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { removal_error: "Custom removal error" } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: proto" + "2_unittest.Foo.Bar specifies a removal error but is not marked " + "removed in any edition.\n"); +} + +TEST(CustomOptions, FeatureSupportMissingRemovalErrorException) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { + edition_introduced: EDITION_99997_TEST_ONLY + edition_removed: EDITION_99997_TEST_ONLY + } + } + } + })pb", + &file_proto)); + + EXPECT_NE(pool.BuildFile(file_proto), nullptr); +} + +TEST(CustomOptions, FeatureSupportInvalidDeprecatedBeforeIntroduced) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { + edition_introduced: EDITION_2024 + edition_deprecated: EDITION_2023 + deprecation_warning: "warning" + edition_removed: EDITION_2024 + removal_error: "Custom feature removal error" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: " + "proto2_unittest.Foo.Bar " + "was deprecated before it was introduced.\n"); +} + +TEST(CustomOptions, FeatureSupportInvalidDeprecatedAfterRemoved) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { + edition_introduced: EDITION_2023 + edition_deprecated: EDITION_2024 + deprecation_warning: "warning" + edition_removed: EDITION_2024 + removal_error: "Custom feature removal error" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: " + "proto2_unittest.Foo.Bar " + "was deprecated after it was removed.\n"); +} + +TEST(CustomOptions, FeatureSupportInvalidRemovedBeforeIntroduced) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + message_type { + name: "Foo" + field { + name: "Bar" + number: 9 + label: LABEL_OPTIONAL + type: TYPE_INT32 + options { + feature_support { + edition_introduced: EDITION_2024 + edition_removed: EDITION_2023 + removal_error: "Custom feature removal error" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_EQ(error_collector.text_, + "foo.proto: proto2_unittest.Foo.Bar: OPTION_NAME: " + "proto2_unittest.Foo.Bar " + "was removed before it was introduced.\n"); +} + +TEST(CustomOptions, FeatureSupportInvalidValueWithFeatureSupport) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { feature_support { edition_deprecated: EDITION_2023 } } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr( + "value proto2_unittest.VALUE is not allowed to have feature " + "support because its field proto2_unittest.Bar.bool_field " + "doesn't have feature support.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueWithMissingDeprecationWarning) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { feature_support { edition_deprecated: EDITION_2023 } } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr( + "foo.proto: proto2_unittest.VALUE: OPTION_NAME: " + "proto2_unittest.VALUE is " + "deprecated but does not specify a deprecation warning.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueWithMissingDeprecation) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + deprecation_warning: "Custom deprecation warning" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT( + error_collector.text_, + testing::HasSubstr( + "foo.proto: proto2_unittest.VALUE: OPTION_NAME: " + "proto2_unittest.VALUE specifies a deprecation warning but is not " + "marked deprecated in any edition.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueWithMissingRemovalError) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { feature_support { edition_removed: EDITION_2023 } } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT( + error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: OPTION_NAME: " + "proto2_unittest.VALUE has been removed but does not " + "specify a removal error.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueWithMissingRemoval) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { removal_error: "Custom removal error" } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT( + error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: OPTION_NAME: " + "proto2_unittest.VALUE specifies a removal error but " + "is not marked removed in any " + "edition.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueDeprecatedBeforeIntroduced) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + edition_introduced: EDITION_2024 + edition_deprecated: EDITION_2023 + deprecation_warning: "warning" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: proto2_unittest.VALUE " + "was deprecated before it was introduced.\n")); +} + +TEST(CustomOptions, + FeatureSupportInvalidValueDeprecatedBeforeIntroducedInherited) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + edition_deprecated: EDITION_2023 + deprecation_warning: "warning" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2024 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: proto2_unittest.VALUE " + "was deprecated before it was introduced.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueDeprecatedAfterRemoved) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + edition_introduced: EDITION_2023 + edition_deprecated: EDITION_2024 + deprecation_warning: "warning" + edition_removed: EDITION_2024 + removal_error: "Custom feature removal error" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: proto2_unittest.VALUE " + "was deprecated after it was removed.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueRemovedBeforeIntroduced) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + edition_introduced: EDITION_2024 + edition_removed: EDITION_2023 + removal_error: "Custom feature removal error" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2023 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: proto2_unittest.VALUE " + "was removed before it was introduced.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueIntroducedBeforeOption) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { feature_support { edition_introduced: EDITION_2023 } } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { feature_support { edition_introduced: EDITION_2024 } } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr( + "foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: value proto2_unittest.VALUE was introduced " + "before proto2_unittest.Bar.bool_field was.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueIntroducedAfterOptionRemoved) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { edition_introduced: EDITION_99997_TEST_ONLY } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { + feature_support { + edition_introduced: EDITION_2023 + edition_removed: EDITION_2024 + removal_error: "Custom feature removal error" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr("foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: proto2_unittest.VALUE was " + "removed before it was introduced.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueRemovedAfterFeature) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + edition_removed: EDITION_99997_TEST_ONLY + removal_error: "Custom feature removal error" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { + feature_support { + edition_introduced: EDITION_2023 + edition_removed: EDITION_2024 + removal_error: "Custom feature removal error" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr( + "foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: value proto2_unittest.VALUE was " + "removed after proto2_unittest.Bar.bool_field was.\n")); +} + +TEST(CustomOptions, FeatureSupportInvalidValueDeprecatedAfterOption) { + DescriptorPool pool; + + FileDescriptorProto file_proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&file_proto); + ASSERT_TRUE(pool.BuildFile(file_proto) != nullptr); + + ASSERT_TRUE(TextFormat::ParseFromString( + R"pb( + name: "foo.proto" + edition: EDITION_2024 + package: "proto2_unittest" + enum_type { + name: "Foo" + value { name: "UNKNOWN" number: 0 } + value { + name: "VALUE" + number: 1 + options { + feature_support { + edition_deprecated: EDITION_99997_TEST_ONLY + deprecation_warning: "warning" + } + } + } + } + message_type { + name: "Bar" + field { + name: "bool_field" + number: 1 + label: LABEL_OPTIONAL + type: TYPE_ENUM + type_name: "Foo" + options { + feature_support { + edition_introduced: EDITION_2023 + edition_deprecated: EDITION_2024 + deprecation_warning: "warning" + } + } + } + })pb", + &file_proto)); + + MockErrorCollector error_collector; + EXPECT_FALSE(pool.BuildFileCollectingErrors(file_proto, &error_collector)); + EXPECT_THAT(error_collector.text_, + testing::HasSubstr( + "foo.proto: proto2_unittest.VALUE: " + "OPTION_NAME: value proto2_unittest.VALUE was " + "deprecated after proto2_unittest.Bar.bool_field was.\n")); +} + // =================================================================== TEST_F(ValidationErrorTest, AlreadyDefined) { diff --git a/src/google/protobuf/feature_resolver.cc b/src/google/protobuf/feature_resolver.cc index 1ea904722ae53..21b961045dc96 100644 --- a/src/google/protobuf/feature_resolver.cc +++ b/src/google/protobuf/feature_resolver.cc @@ -54,38 +54,6 @@ absl::Status Error(Args... args) { return absl::FailedPreconditionError(absl::StrCat(args...)); } -absl::Status ValidateFeatureSupport(const FieldOptions::FeatureSupport& support, - absl::string_view full_name) { - if (support.has_edition_deprecated()) { - if (support.edition_deprecated() < support.edition_introduced()) { - return Error("Feature ", full_name, - " was deprecated before it was introduced."); - } - if (!support.has_deprecation_warning()) { - return Error( - "Feature ", full_name, - " is deprecated but does not specify a deprecation warning."); - } - } - if (!support.has_edition_deprecated() && support.has_deprecation_warning()) { - return Error("Feature ", full_name, - " specifies a deprecation warning but is not marked " - "deprecated in any edition."); - } - if (support.has_edition_removed()) { - if (support.edition_deprecated() >= support.edition_removed()) { - return Error("Feature ", full_name, - " was deprecated after it was removed."); - } - if (support.edition_removed() < support.edition_introduced()) { - return Error("Feature ", full_name, - " was removed before it was introduced."); - } - } - - return absl::OkStatus(); -} - absl::Status ValidateFieldFeatureSupport(const FieldDescriptor& field) { if (!field.options().has_feature_support()) { return Error("Feature field ", field.full_name(), @@ -98,7 +66,6 @@ absl::Status ValidateFieldFeatureSupport(const FieldDescriptor& field) { return Error("Feature field ", field.full_name(), " does not specify the edition it was introduced in."); } - RETURN_IF_ERROR(ValidateFeatureSupport(support, field.full_name())); // Validate edition defaults specification wrt support windows. for (const auto& d : field.options().edition_defaults()) { @@ -123,54 +90,6 @@ absl::Status ValidateFieldFeatureSupport(const FieldDescriptor& field) { return absl::OkStatus(); } -absl::Status ValidateValueFeatureSupport( - const FieldOptions::FeatureSupport& parent, - const EnumValueDescriptor& value, absl::string_view field_name) { - if (!value.options().has_feature_support()) { - // We allow missing support windows on feature values, and they'll inherit - // from the feature spec. - return absl::OkStatus(); - } - - FieldOptions::FeatureSupport support = parent; - support.MergeFrom(value.options().feature_support()); - RETURN_IF_ERROR(ValidateFeatureSupport(support, value.full_name())); - - // Make sure the value doesn't expand any bounds. - if (support.edition_introduced() < parent.edition_introduced()) { - return Error("Feature value ", value.full_name(), - " was introduced before feature ", field_name, " was."); - } - if (parent.has_edition_removed() && - support.edition_removed() > parent.edition_removed()) { - return Error("Feature value ", value.full_name(), - " was removed after feature ", field_name, " was."); - } - if (parent.has_edition_deprecated() && - support.edition_deprecated() > parent.edition_deprecated()) { - return Error("Feature value ", value.full_name(), - " was deprecated after feature ", field_name, " was."); - } - - return absl::OkStatus(); -} - -absl::Status ValidateValuesFeatureSupport(const FieldDescriptor& field) { - // This only applies to enum features. - ABSL_CHECK(field.enum_type() != nullptr); - - const FieldOptions::FeatureSupport& parent = - field.options().feature_support(); - - for (int i = 0; i < field.enum_type()->value_count(); ++i) { - const EnumValueDescriptor& value = *field.enum_type()->value(i); - RETURN_IF_ERROR( - ValidateValueFeatureSupport(parent, value, field.full_name())); - } - - return absl::OkStatus(); -} - absl::Status ValidateDescriptor(const Descriptor& descriptor) { if (descriptor.oneof_decl_count() > 0) { return Error("Type ", descriptor.full_name(), @@ -211,9 +130,6 @@ absl::Status ValidateDescriptor(const Descriptor& descriptor) { } RETURN_IF_ERROR(ValidateFieldFeatureSupport(field)); - if (field.enum_type() != nullptr) { - RETURN_IF_ERROR(ValidateValuesFeatureSupport(field)); - } } return absl::OkStatus(); @@ -585,6 +501,65 @@ FeatureResolver::ValidationResults FeatureResolver::ValidateFeatureLifetimes( return results; } +absl::Status FeatureResolver::ValidateFeatureSupport( + const FieldOptions::FeatureSupport& support, absl::string_view full_name) { + if (support.has_edition_deprecated()) { + if (support.edition_deprecated() < support.edition_introduced()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute("$0 was deprecated before it was introduced.", + full_name)); + } + if (!support.has_deprecation_warning()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute( + "$0 is deprecated but does not specify a deprecation warning.", + full_name)); + } + } + if (!support.has_edition_deprecated() && support.has_deprecation_warning()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute("$0 specifies a deprecation warning but is not marked " + "deprecated in any edition.", + full_name)); + } + if (support.has_edition_removed()) { + if (support.edition_deprecated() >= support.edition_removed()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute("$0 was deprecated after it was removed.", + full_name)); + } + if (support.edition_removed() < support.edition_introduced()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute("$0 was removed before it was introduced.", + full_name)); + } + // Not enforcing removal errors on features or options that have been + // introduced and removed in the same edition + if ((support.edition_introduced() != support.edition_removed()) && + !support.has_removal_error()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute( + "$0 has been removed but does not specify a removal error.", + full_name)); + } + } + if (!support.has_edition_removed() && support.has_removal_error()) { + return absl::Status( + absl::StatusCode::kInternal, + absl::Substitute( + "$0 specifies a removal error but is not marked removed in any " + "edition.", + full_name)); + } + return absl::OkStatus(); +} + namespace internal { absl::StatusOr GetEditionFeatureSetDefaults( Edition edition, const FeatureSetDefaults& defaults) { diff --git a/src/google/protobuf/feature_resolver.h b/src/google/protobuf/feature_resolver.h index d66c156e2f66d..194300a2aff52 100644 --- a/src/google/protobuf/feature_resolver.h +++ b/src/google/protobuf/feature_resolver.h @@ -73,6 +73,9 @@ class PROTOBUF_EXPORT FeatureResolver { Edition edition, const FeatureSet& features, const Descriptor* pool_descriptor); + static absl::Status ValidateFeatureSupport( + const FieldOptions::FeatureSupport& support, absl::string_view full_name); + private: explicit FeatureResolver(FeatureSet defaults) : defaults_(std::move(defaults)) {} diff --git a/src/google/protobuf/feature_resolver_test.cc b/src/google/protobuf/feature_resolver_test.cc index 6d5c658406b1c..2a23b724d32cc 100644 --- a/src/google/protobuf/feature_resolver_test.cc +++ b/src/google/protobuf/feature_resolver_test.cc @@ -1133,157 +1133,6 @@ TEST_F(FeatureResolverPoolTest, HasSubstr("it was introduced in")))); } -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidWithMissingDeprecationWarning) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - message Foo { - optional bool bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support = { - edition_introduced: EDITION_2023 - edition_deprecated: EDITION_2023 - }, - edition_defaults = { edition: EDITION_LEGACY, value: "true" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.Foo.bool_field"), - HasSubstr("deprecation warning")))); -} - -TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidWithMissingDeprecation) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - message Foo { - optional bool bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support = { - edition_introduced: EDITION_2023 - deprecation_warning: "some message" - }, - edition_defaults = { edition: EDITION_LEGACY, value: "true" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.Foo.bool_field"), - HasSubstr("is not marked deprecated")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidDeprecatedBeforeIntroduced) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - message Foo { - optional bool bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support = { - edition_introduced: EDITION_2024 - edition_deprecated: EDITION_2023 - deprecation_warning: "warning" - }, - edition_defaults = { edition: EDITION_LEGACY, value: "true" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT( - FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023, - EDITION_2023), - HasError(AllOf(HasSubstr("test.Foo.bool_field"), - HasSubstr("deprecated before it was introduced")))); -} - -TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidDeprecatedAfterRemoved) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - message Foo { - optional bool bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support = { - edition_introduced: EDITION_2023 - edition_deprecated: EDITION_2024 - deprecation_warning: "warning" - edition_removed: EDITION_2024 - }, - edition_defaults = { edition: EDITION_LEGACY, value: "true" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.Foo.bool_field"), - HasSubstr("deprecated after it was removed")))); -} - -TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidRemovedBeforeIntroduced) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - message Foo { - optional bool bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support = { - edition_introduced: EDITION_2024 - edition_removed: EDITION_2023 - }, - edition_defaults = { edition: EDITION_LEGACY, value: "true" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.Foo.bool_field"), - HasSubstr("removed before it was introduced")))); -} - TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidMissingLegacyDefaults) { const FileDescriptor* file = ParseSchema(R"schema( syntax = "proto2"; @@ -1359,6 +1208,7 @@ TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidDefaultsAfterRemoved) { feature_support = { edition_introduced: EDITION_PROTO2 edition_removed: EDITION_2023 + removal_error: "Custom feature removal error" }, edition_defaults = { edition: EDITION_LEGACY, value: "true" }, edition_defaults = { edition: EDITION_2024, value: "true" } @@ -1461,349 +1311,6 @@ TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidDefaultsTooEarly) { HasError(HasSubstr("Minimum edition 2_TEST_ONLY is not EDITION_LEGACY"))); } -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueWithMissingDeprecationWarning) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support.edition_deprecated = EDITION_2023]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("deprecation warning")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueWithMissingDeprecation) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support.deprecation_warning = "some message"]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("is not marked deprecated")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueDeprecatedBeforeIntroduced) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_introduced: EDITION_2024 - edition_deprecated: EDITION_2023 - deprecation_warning: "warning" - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT( - FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023, - EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("deprecated before it was introduced")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueDeprecatedBeforeIntroducedInherited) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_deprecated: EDITION_2023 - deprecation_warning: "warning" - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2024, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT( - FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023, - EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("deprecated before it was introduced")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueDeprecatedAfterRemoved) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_introduced: EDITION_2023 - edition_deprecated: EDITION_2024 - deprecation_warning: "warning" - edition_removed: EDITION_2024 - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("deprecated after it was removed")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueRemovedBeforeIntroduced) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_introduced: EDITION_2024 - edition_removed: EDITION_2023 - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("removed before it was introduced")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueIntroducedBeforeFeature) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_introduced: EDITION_2023 - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2024, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT( - FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023, - EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), HasSubstr("introduced before"), - HasSubstr("test.Foo.bool_field")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueIntroducedAfterFeatureRemoved) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_introduced: EDITION_99997_TEST_ONLY - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - feature_support.edition_removed = EDITION_2024, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT(FeatureResolver::CompileDefaults(feature_set_, {ext}, - EDITION_2023, EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), - HasSubstr("removed before it was introduced")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueRemovedAfterFeature) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_removed: EDITION_99997_TEST_ONLY - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - feature_support.edition_removed = EDITION_2024, - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT( - FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023, - EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), HasSubstr("removed after"), - HasSubstr("test.Foo.bool_field")))); -} - -TEST_F(FeatureResolverPoolTest, - CompileDefaultsInvalidValueDeprecatedAfterFeature) { - const FileDescriptor* file = ParseSchema(R"schema( - syntax = "proto2"; - package test; - import "google/protobuf/descriptor.proto"; - - extend google.protobuf.FeatureSet { - optional Foo bar = 9999; - } - enum FooValues { - UNKNOWN = 0; - VALUE = 1 [feature_support = { - edition_deprecated: EDITION_99997_TEST_ONLY - deprecation_warning: "warning" - }]; - } - message Foo { - optional FooValues bool_field = 1 [ - targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - feature_support.edition_deprecated = EDITION_2024, - feature_support.deprecation_warning = "warning", - edition_defaults = { edition: EDITION_LEGACY, value: "UNKNOWN" } - ]; - } - )schema"); - ASSERT_NE(file, nullptr); - - const FieldDescriptor* ext = file->extension(0); - EXPECT_THAT( - FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023, - EDITION_2023), - HasError(AllOf(HasSubstr("test.VALUE"), HasSubstr("deprecated after"), - HasSubstr("test.Foo.bool_field")))); -} - TEST_F(FeatureResolverPoolTest, CompileDefaultsMinimumTooEarly) { const FileDescriptor* file = ParseSchema(R"schema( syntax = "proto2"; @@ -1847,8 +1354,11 @@ TEST_F(FeatureResolverPoolTest, CompileDefaultsRemovedOnly) { message Foo { optional Bar file_feature = 1 [ targets = TARGET_TYPE_FIELD, - feature_support.edition_introduced = EDITION_2023, - feature_support.edition_removed = EDITION_99998_TEST_ONLY, + feature_support = { + edition_introduced: EDITION_2023 + edition_removed: EDITION_99998_TEST_ONLY + removal_error: "Custom feature removal error" + }, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; } diff --git a/src/google/protobuf/unittest_custom_features.proto b/src/google/protobuf/unittest_custom_features.proto index 1d5307fbe2898..f8d39b3771e35 100644 --- a/src/google/protobuf/unittest_custom_features.proto +++ b/src/google/protobuf/unittest_custom_features.proto @@ -50,6 +50,7 @@ enum ValueLifetimeFeature { edition_deprecated: EDITION_99998_TEST_ONLY deprecation_warning: "Custom feature deprecation warning" edition_removed: EDITION_99999_TEST_ONLY + removal_error: "Custom feature removal error" }]; VALUE_LIFETIME_EMPTY_SUPPORT = 3 [feature_support = {}]; VALUE_LIFETIME_FUTURE = 4 @@ -60,6 +61,7 @@ enum ValueLifetimeFeature { }]; VALUE_LIFETIME_REMOVED = 6 [feature_support = { edition_deprecated: EDITION_2023 + deprecation_warning: "Custom feature deprecation warning" edition_removed: EDITION_99997_TEST_ONLY removal_error: "Custom feature removal error" }]; diff --git a/src/google/protobuf/unittest_features.proto b/src/google/protobuf/unittest_features.proto index 7cdb11dcf6465..b1488fba4550a 100644 --- a/src/google/protobuf/unittest_features.proto +++ b/src/google/protobuf/unittest_features.proto @@ -63,6 +63,7 @@ enum ValueLifetimeFeature { edition_deprecated: EDITION_99998_TEST_ONLY deprecation_warning: "Custom feature deprecation warning" edition_removed: EDITION_99999_TEST_ONLY + removal_error: "Custom feature removal error" }]; VALUE_LIFETIME_EMPTY_SUPPORT = 3 [feature_support = {}]; VALUE_LIFETIME_FUTURE = 4 @@ -73,6 +74,7 @@ enum ValueLifetimeFeature { }]; VALUE_LIFETIME_REMOVED = 6 [feature_support = { edition_deprecated: EDITION_2023 + deprecation_warning: "Custom feature deprecation warning" edition_removed: EDITION_99997_TEST_ONLY removal_error: "Custom feature removal error" }];