diff --git a/editions/defaults_test.cc b/editions/defaults_test.cc index b3fa62f5e765..3e4861cd0b64 100644 --- a/editions/defaults_test.cc +++ b/editions/defaults_test.cc @@ -103,7 +103,7 @@ TEST(DefaultsTest, CheckFuture) { TEST(DefaultsTest, CheckFarFuture) { auto defaults = ReadDefaults("test_defaults_far_future"); ASSERT_OK(defaults); - ASSERT_EQ(defaults->defaults().size(), 6); + ASSERT_EQ(defaults->defaults().size(), 7); ASSERT_EQ(defaults->minimum_edition(), EDITION_99997_TEST_ONLY); ASSERT_EQ(defaults->maximum_edition(), EDITION_99999_TEST_ONLY); diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index 2c8423efac79..02361195a620 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -1474,6 +1474,7 @@ class ReturnObject: file = descriptor_pb2.FileDescriptorProto() descriptor_pb2.DESCRIPTOR.CopyToProto(file) ret.pool.Add(file) + file.Clear() unittest_features_pb2.DESCRIPTOR.CopyToProto(file) ret.pool.Add(file) diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index fc30827647b4..84e727f424cb 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -2097,7 +2097,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithExtension) { FeatureSetDefaults defaults = ReadEditionDefaults("defaults"); EXPECT_EQ(defaults.minimum_edition(), EDITION_PROTO2); EXPECT_EQ(defaults.maximum_edition(), EDITION_99999_TEST_ONLY); - ASSERT_EQ(defaults.defaults_size(), 6); + ASSERT_EQ(defaults.defaults_size(), 7); EXPECT_EQ(defaults.defaults(0).edition(), EDITION_LEGACY); EXPECT_EQ(defaults.defaults(2).edition(), EDITION_2023); EXPECT_EQ(defaults.defaults(3).edition(), EDITION_2024); diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 2f224e7f7440..30d15ddee5ff 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -10970,7 +10970,8 @@ TEST_F(FeaturesTest, RemovedFeature) { } )pb", "foo.proto: foo.proto: NAME: Feature " - "pb.TestFeatures.removed_feature has been removed in edition 2024\n"); + "pb.TestFeatures.removed_feature has been removed in edition 2024 and " + "can't be used in edition 2024\n"); } TEST_F(FeaturesTest, RemovedFeatureDefault) { @@ -11001,7 +11002,8 @@ TEST_F(FeaturesTest, FutureFeature) { } )pb", "foo.proto: foo.proto: NAME: Feature " - "pb.TestFeatures.future_feature wasn't introduced until edition 2024\n"); + "pb.TestFeatures.future_feature wasn't introduced until edition 2024 and " + "can't be used in edition 2023\n"); } TEST_F(FeaturesTest, FutureFeatureDefault) { @@ -12381,14 +12383,20 @@ TEST_F(DatabaseBackedPoolTest, UnittestProto) { } TEST_F(DatabaseBackedPoolTest, FeatureResolution) { - FileDescriptorProto proto; - FileDescriptorProto::descriptor()->file()->CopyTo(&proto); - std::string text_proto; - google::protobuf::TextFormat::PrintToString(proto, &text_proto); - AddToDatabase(&database_, text_proto); - pb::TestFeatures::descriptor()->file()->CopyTo(&proto); - google::protobuf::TextFormat::PrintToString(proto, &text_proto); - AddToDatabase(&database_, text_proto); + { + FileDescriptorProto proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&proto); + std::string text_proto; + google::protobuf::TextFormat::PrintToString(proto, &text_proto); + AddToDatabase(&database_, text_proto); + } + { + FileDescriptorProto proto; + pb::TestFeatures::descriptor()->file()->CopyTo(&proto); + std::string text_proto; + google::protobuf::TextFormat::PrintToString(proto, &text_proto); + AddToDatabase(&database_, text_proto); + } AddToDatabase(&database_, R"pb( name: "features.proto" syntax: "editions" @@ -12432,14 +12440,20 @@ TEST_F(DatabaseBackedPoolTest, FeatureResolution) { } TEST_F(DatabaseBackedPoolTest, FeatureLifetimeError) { - FileDescriptorProto proto; - FileDescriptorProto::descriptor()->file()->CopyTo(&proto); - std::string text_proto; - google::protobuf::TextFormat::PrintToString(proto, &text_proto); - AddToDatabase(&database_, text_proto); - pb::TestFeatures::descriptor()->file()->CopyTo(&proto); - google::protobuf::TextFormat::PrintToString(proto, &text_proto); - AddToDatabase(&database_, text_proto); + { + FileDescriptorProto proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&proto); + std::string text_proto; + google::protobuf::TextFormat::PrintToString(proto, &text_proto); + AddToDatabase(&database_, text_proto); + } + { + FileDescriptorProto proto; + pb::TestFeatures::descriptor()->file()->CopyTo(&proto); + std::string text_proto; + google::protobuf::TextFormat::PrintToString(proto, &text_proto); + AddToDatabase(&database_, text_proto); + } AddToDatabase(&database_, R"pb( name: "features.proto" syntax: "editions" @@ -12458,21 +12472,27 @@ TEST_F(DatabaseBackedPoolTest, FeatureLifetimeError) { DescriptorPool pool(&database_, &error_collector); EXPECT_TRUE(pool.FindMessageTypeByName("FooFeatures") == nullptr); - EXPECT_EQ( - error_collector.text_, - "features.proto: FooFeatures: NAME: Feature " - "pb.TestFeatures.future_feature wasn't introduced until edition 2024\n"); + EXPECT_EQ(error_collector.text_, + "features.proto: FooFeatures: NAME: Feature " + "pb.TestFeatures.future_feature wasn't introduced until edition " + "2024 and can't be used in edition 2023\n"); } TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) { - FileDescriptorProto proto; - FileDescriptorProto::descriptor()->file()->CopyTo(&proto); - std::string text_proto; - google::protobuf::TextFormat::PrintToString(proto, &text_proto); - AddToDatabase(&database_, text_proto); - pb::TestFeatures::descriptor()->file()->CopyTo(&proto); - google::protobuf::TextFormat::PrintToString(proto, &text_proto); - AddToDatabase(&database_, text_proto); + { + FileDescriptorProto proto; + FileDescriptorProto::descriptor()->file()->CopyTo(&proto); + std::string text_proto; + google::protobuf::TextFormat::PrintToString(proto, &text_proto); + AddToDatabase(&database_, text_proto); + } + { + FileDescriptorProto proto; + pb::TestFeatures::descriptor()->file()->CopyTo(&proto); + std::string text_proto; + google::protobuf::TextFormat::PrintToString(proto, &text_proto); + AddToDatabase(&database_, text_proto); + } AddToDatabase(&database_, R"pb( name: "option.proto" syntax: "editions" @@ -12522,10 +12542,10 @@ TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) { // Verify that the extension does trigger a lifetime error. error_collector.text_.clear(); ASSERT_EQ(pool.FindExtensionByName("foo_extension"), nullptr); - EXPECT_EQ( - error_collector.text_, - "option.proto: foo_extension: NAME: Feature " - "pb.TestFeatures.legacy_feature has been removed in edition 2023\n"); + EXPECT_EQ(error_collector.text_, + "option.proto: foo_extension: NAME: Feature " + "pb.TestFeatures.legacy_feature has been removed in edition 2023 " + "and can't be used in edition 2023\n"); } TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) { diff --git a/src/google/protobuf/feature_resolver.cc b/src/google/protobuf/feature_resolver.cc index bd46eb137e62..7bb393a6a7bc 100644 --- a/src/google/protobuf/feature_resolver.cc +++ b/src/google/protobuf/feature_resolver.cc @@ -53,6 +53,123 @@ 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(), + " has no feature support specified."); + } + + const FieldOptions::FeatureSupport& support = + field.options().feature_support(); + if (!support.has_edition_introduced()) { + 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()) { + if (d.edition() < Edition::EDITION_2023) { + // Allow defaults to be specified in proto2/proto3, predating + // editions. + continue; + } + if (d.edition() < support.edition_introduced()) { + return Error("Feature field ", field.full_name(), + " has a default specified for edition ", d.edition(), + ", before it was introduced."); + } + if (support.has_edition_removed() && + d.edition() > support.edition_removed()) { + return Error("Feature field ", field.full_name(), + " has a default specified for edition ", d.edition(), + ", after it was removed."); + } + } + + 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(), @@ -92,62 +209,9 @@ absl::Status ValidateDescriptor(const Descriptor& descriptor) { "was introduced."); } - if (!field.options().has_feature_support()) { - return Error("Feature field ", field.full_name(), - " has no feature support specified."); - } - - const FieldOptions::FeatureSupport& support = - field.options().feature_support(); - if (!support.has_edition_introduced()) { - return Error("Feature field ", field.full_name(), - " does not specify the edition it was introduced in."); - } - if (support.has_edition_deprecated()) { - if (!support.has_deprecation_warning()) { - return Error( - "Feature field ", field.full_name(), - " is deprecated but does not specify a deprecation warning."); - } - if (support.edition_deprecated() < support.edition_introduced()) { - return Error("Feature field ", field.full_name(), - " was deprecated before it was introduced."); - } - } - if (!support.has_edition_deprecated() && - support.has_deprecation_warning()) { - return Error("Feature field ", field.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 field ", field.full_name(), - " was deprecated after it was removed."); - } - if (support.edition_removed() < support.edition_introduced()) { - return Error("Feature field ", field.full_name(), - " was removed before it was introduced."); - } - } - - for (const auto& d : field.options().edition_defaults()) { - if (d.edition() < Edition::EDITION_2023) { - // Allow defaults to be specified in proto2/proto3, predating - // editions. - continue; - } - if (d.edition() < support.edition_introduced()) { - return Error("Feature field ", field.full_name(), - " has a default specified for edition ", d.edition(), - ", before it was introduced."); - } - if (support.has_edition_removed() && - d.edition() > support.edition_removed()) { - return Error("Feature field ", field.full_name(), - " has a default specified for edition ", d.edition(), - ", after it was removed."); - } + RETURN_IF_ERROR(ValidateFieldFeatureSupport(field)); + if (field.enum_type() != nullptr) { + RETURN_IF_ERROR(ValidateValuesFeatureSupport(field)); } } @@ -293,15 +357,40 @@ absl::Status ValidateMergedFeatures(const FeatureSet& features) { return absl::OkStatus(); } -void CollectLifetimeResults(Edition edition, const Message& message, - FeatureResolver::ValidationResults& results) { +void ValidateSingleFeatureLifetimes( + Edition edition, absl::string_view full_name, + const FieldOptions::FeatureSupport& support, + FeatureResolver::ValidationResults& results) { + // Skip fields that don't have feature support specified. + if (&support == &FieldOptions::FeatureSupport::default_instance()) return; + + if (edition < support.edition_introduced()) { + results.errors.emplace_back( + absl::StrCat("Feature ", full_name, " wasn't introduced until edition ", + support.edition_introduced(), + " and can't be used in edition ", edition)); + } + if (support.has_edition_removed() && edition >= support.edition_removed()) { + results.errors.emplace_back(absl::StrCat( + "Feature ", full_name, " has been removed in edition ", + support.edition_removed(), " and can't be used in edition ", edition)); + } else if (support.has_edition_deprecated() && + edition >= support.edition_deprecated()) { + results.warnings.emplace_back(absl::StrCat( + "Feature ", full_name, " has been deprecated in edition ", + support.edition_deprecated(), ": ", support.deprecation_warning())); + } +} + +void ValidateFeatureLifetimesImpl(Edition edition, const Message& message, + FeatureResolver::ValidationResults& results) { std::vector fields; message.GetReflection()->ListFields(message, &fields); for (const FieldDescriptor* field : fields) { // Recurse into message extension. if (field->is_extension() && field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { - CollectLifetimeResults( + ValidateFeatureLifetimesImpl( edition, message.GetReflection()->GetMessage(message, field), results); continue; @@ -310,39 +399,18 @@ void CollectLifetimeResults(Edition edition, const Message& message, if (field->enum_type() != nullptr) { int number = message.GetReflection()->GetEnumValue(message, field); auto value = field->enum_type()->FindValueByNumber(number); - if (value != nullptr) { - const FieldOptions::FeatureSupport& support = - value->options().feature_support(); - if (value->options().has_feature_support() && - support.edition_introduced() > edition) { - results.errors.emplace_back( - absl::StrCat("Feature ", value->full_name(), - " wasn't introduced until edition ", - support.edition_introduced())); - } + 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); } - // Skip fields that don't have feature support specified. - if (!field->options().has_feature_support()) continue; - - const FieldOptions::FeatureSupport& support = - field->options().feature_support(); - if (edition < support.edition_introduced()) { - results.errors.emplace_back(absl::StrCat( - "Feature ", field->full_name(), " wasn't introduced until edition ", - support.edition_introduced())); - } - if (support.has_edition_removed() && edition >= support.edition_removed()) { - results.errors.emplace_back(absl::StrCat("Feature ", field->full_name(), - " has been removed in edition ", - support.edition_removed())); - } else if (support.has_edition_deprecated() && - edition >= support.edition_deprecated()) { - results.warnings.emplace_back(absl::StrCat( - "Feature ", field->full_name(), " has been deprecated in edition ", - support.edition_deprecated(), ": ", support.deprecation_warning())); - } + ValidateSingleFeatureLifetimes(edition, field->full_name(), + field->options().feature_support(), results); } } @@ -504,7 +572,7 @@ FeatureResolver::ValidationResults FeatureResolver::ValidateFeatureLifetimes( ABSL_CHECK(pool_features != nullptr); ValidationResults results; - CollectLifetimeResults(edition, *pool_features, results); + ValidateFeatureLifetimesImpl(edition, *pool_features, results); return results; } diff --git a/src/google/protobuf/feature_resolver_test.cc b/src/google/protobuf/feature_resolver_test.cc index 6d8fad8ca093..60b1f7b565fd 100644 --- a/src/google/protobuf/feature_resolver_test.cc +++ b/src/google/protobuf/feature_resolver_test.cc @@ -636,11 +636,16 @@ TEST(FeatureResolverLifetimesTest, MultipleErrors) { TEST(FeatureResolverLifetimesTest, DynamicPool) { DescriptorPool pool; - FileDescriptorProto file; - FileDescriptorProto::GetDescriptor()->file()->CopyTo(&file); - ASSERT_NE(pool.BuildFile(file), nullptr); - pb::TestFeatures::GetDescriptor()->file()->CopyTo(&file); - ASSERT_NE(pool.BuildFile(file), nullptr); + { + FileDescriptorProto file; + FileDescriptorProto::GetDescriptor()->file()->CopyTo(&file); + ASSERT_NE(pool.BuildFile(file), nullptr); + } + { + FileDescriptorProto file; + pb::TestFeatures::GetDescriptor()->file()->CopyTo(&file); + ASSERT_NE(pool.BuildFile(file), nullptr); + } const Descriptor* feature_set = pool.FindMessageTypeByName("google.protobuf.FeatureSet"); ASSERT_NE(feature_set, nullptr); @@ -656,9 +661,9 @@ TEST(FeatureResolverLifetimesTest, DynamicPool) { ElementsAre(HasSubstr("pb.TestFeatures.removed_feature"))); } -TEST(FeatureResolverLifetimesTest, EmptyValueSupportInvalid2023) { +TEST(FeatureResolverLifetimesTest, EmptyValueSupportValid) { FeatureSet features = ParseTextOrDie(R"pb( - [pb.test] { file_feature: VALUE_EMPTY_SUPPORT } + [pb.test] { value_lifetime_feature: VALUE_LIFETIME_EMPTY_SUPPORT } )pb"); auto results = FeatureResolver::ValidateFeatureLifetimes(EDITION_2023, features, nullptr); @@ -666,19 +671,86 @@ TEST(FeatureResolverLifetimesTest, EmptyValueSupportInvalid2023) { EXPECT_THAT(results.warnings, IsEmpty()); } -TEST(FeatureResolverLifetimesTest, ValueSupportInvalid2023) { +TEST(FeatureResolverLifetimesTest, ValueSupportValid) { + FeatureSet features = ParseTextOrDie(R"pb( + [pb.test] { value_lifetime_feature: VALUE_LIFETIME_SUPPORT } + )pb"); + auto results = FeatureResolver::ValidateFeatureLifetimes( + EDITION_99997_TEST_ONLY, features, nullptr); + EXPECT_THAT(results.errors, IsEmpty()); + EXPECT_THAT(results.warnings, IsEmpty()); +} + +TEST(FeatureResolverLifetimesTest, ValueSupportBeforeIntroduced) { FeatureSet features = ParseTextOrDie(R"pb( - [pb.test] { file_feature: VALUE_FUTURE } + [pb.test] { value_lifetime_feature: VALUE_LIFETIME_FUTURE } )pb"); auto results = FeatureResolver::ValidateFeatureLifetimes(EDITION_2023, features, nullptr); EXPECT_THAT(results.errors, ElementsAre(AllOf( - HasSubstr("pb.VALUE_FUTURE"), + HasSubstr("pb.VALUE_LIFETIME_FUTURE"), HasSubstr("introduced until edition 99997_TEST_ONLY")))); EXPECT_THAT(results.warnings, IsEmpty()); } +TEST(FeatureResolverLifetimesTest, ValueSupportAfterRemoved) { + FeatureSet features = ParseTextOrDie(R"pb( + [pb.test] { value_lifetime_feature: VALUE_LIFETIME_REMOVED } + )pb"); + auto results = FeatureResolver::ValidateFeatureLifetimes( + EDITION_99997_TEST_ONLY, features, nullptr); + EXPECT_THAT( + results.errors, + ElementsAre(AllOf(HasSubstr("pb.VALUE_LIFETIME_REMOVED"), + HasSubstr("removed in edition 99997_TEST_ONLY")))); + EXPECT_THAT(results.warnings, IsEmpty()); +} + +TEST(FeatureResolverLifetimesTest, ValueSupportDeprecated) { + FeatureSet features = ParseTextOrDie(R"pb( + [pb.test] { value_lifetime_feature: VALUE_LIFETIME_DEPRECATED } + )pb"); + auto results = FeatureResolver::ValidateFeatureLifetimes( + EDITION_99997_TEST_ONLY, features, nullptr); + EXPECT_THAT(results.errors, IsEmpty()); + EXPECT_THAT( + results.warnings, + ElementsAre(AllOf(HasSubstr("pb.VALUE_LIFETIME_DEPRECATED"), + HasSubstr("deprecated in edition 99997_TEST_ONLY"), + HasSubstr("Custom feature deprecation warning")))); +} + +TEST(FeatureResolverLifetimesTest, ValueAndFeatureSupportDeprecated) { + FeatureSet features = ParseTextOrDie(R"pb( + [pb.test] { value_lifetime_feature: VALUE_LIFETIME_DEPRECATED } + )pb"); + auto results = FeatureResolver::ValidateFeatureLifetimes( + EDITION_99998_TEST_ONLY, features, nullptr); + EXPECT_THAT(results.errors, IsEmpty()); + EXPECT_THAT(results.warnings, + UnorderedElementsAre( + AllOf(HasSubstr("pb.VALUE_LIFETIME_DEPRECATED"), + HasSubstr("deprecated in edition 99997_TEST_ONLY"), + HasSubstr("Custom feature deprecation warning")), + AllOf(HasSubstr("pb.TestFeatures.value_lifetime_feature"), + HasSubstr("deprecated in edition 99998_TEST_ONLY"), + HasSubstr("Custom feature deprecation warning")))); +} + +TEST(FeatureResolverLifetimesTest, ValueSupportInvalidNumber) { + FeatureSet features; + features.MutableExtension(pb::test)->set_value_lifetime_feature( + static_cast(1234)); + auto results = FeatureResolver::ValidateFeatureLifetimes(EDITION_2023, + features, nullptr); + EXPECT_THAT( + results.errors, + ElementsAre(AllOf(HasSubstr("pb.TestFeatures.value_lifetime_feature"), + HasSubstr("1234")))); + EXPECT_THAT(results.warnings, IsEmpty()); +} + class FakeErrorCollector : public io::ErrorCollector { public: FakeErrorCollector() = default; @@ -1284,6 +1356,349 @@ 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"; diff --git a/src/google/protobuf/unittest_features.proto b/src/google/protobuf/unittest_features.proto index ddf510c6b6ce..b4fef7606bf5 100644 --- a/src/google/protobuf/unittest_features.proto +++ b/src/google/protobuf/unittest_features.proto @@ -5,23 +5,23 @@ // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd -syntax = "proto2"; +edition = "2023"; package pb; import "google/protobuf/descriptor.proto"; extend google.protobuf.FeatureSet { - optional TestFeatures test = 9999; + TestFeatures test = 9999; } message TestMessage { extend google.protobuf.FeatureSet { - optional TestFeatures test_message = 9998; + TestFeatures test_message = 9998; } message Nested { extend google.protobuf.FeatureSet { - optional TestFeatures test_nested = 9997; + TestFeatures test_nested = 9997; } } } @@ -43,17 +43,32 @@ enum EnumFeature { VALUE13 = 13; VALUE14 = 14; VALUE15 = 15; - VALUE_EMPTY_SUPPORT = 98 [feature_support = {}]; - VALUE_FUTURE = 99 [feature_support = { +} + +enum ValueLifetimeFeature { + TEST_VALUE_LIFETIME_UNKNOWN = 0; + VALUE_LIFETIME_INHERITED = 1; + VALUE_LIFETIME_SUPPORT = 2 [feature_support = { edition_introduced: EDITION_99997_TEST_ONLY edition_deprecated: EDITION_99998_TEST_ONLY deprecation_warning: "Custom feature deprecation warning" edition_removed: EDITION_99999_TEST_ONLY }]; + VALUE_LIFETIME_EMPTY_SUPPORT = 3 [feature_support = {}]; + VALUE_LIFETIME_FUTURE = 4 + [feature_support.edition_introduced = EDITION_99997_TEST_ONLY]; + VALUE_LIFETIME_DEPRECATED = 5 [feature_support = { + edition_deprecated: EDITION_99997_TEST_ONLY + deprecation_warning: "Custom feature deprecation warning" + }]; + VALUE_LIFETIME_REMOVED = 6 [feature_support = { + edition_deprecated: EDITION_2023 + edition_removed: EDITION_99997_TEST_ONLY + }]; } message TestFeatures { - optional EnumFeature file_feature = 1 [ + EnumFeature file_feature = 1 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FILE, feature_support.edition_introduced = EDITION_2023, @@ -63,55 +78,55 @@ message TestFeatures { edition_defaults = { edition: EDITION_99997_TEST_ONLY, value: "VALUE4" }, edition_defaults = { edition: EDITION_99998_TEST_ONLY, value: "VALUE5" } ]; - optional EnumFeature extension_range_feature = 2 [ + EnumFeature extension_range_feature = 2 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_EXTENSION_RANGE, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature message_feature = 3 [ + EnumFeature message_feature = 3 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_MESSAGE, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature field_feature = 4 [ + EnumFeature field_feature = 4 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FIELD, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature oneof_feature = 5 [ + EnumFeature oneof_feature = 5 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_ONEOF, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature enum_feature = 6 [ + EnumFeature enum_feature = 6 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_ENUM, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature enum_entry_feature = 7 [ + EnumFeature enum_entry_feature = 7 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_ENUM_ENTRY, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature service_feature = 8 [ + EnumFeature service_feature = 8 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_SERVICE, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature method_feature = 9 [ + EnumFeature method_feature = 9 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_METHOD, feature_support.edition_introduced = EDITION_2023, edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature multiple_feature = 10 [ + EnumFeature multiple_feature = 10 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FILE, targets = TARGET_TYPE_FIELD, @@ -126,7 +141,7 @@ message TestFeatures { edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional bool bool_field_feature = 11 [ + bool bool_field_feature = 11 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FIELD, feature_support.edition_introduced = EDITION_2023, @@ -134,7 +149,7 @@ message TestFeatures { edition_defaults = { edition: EDITION_99997_TEST_ONLY, value: "true" } ]; - optional EnumFeature source_feature = 15 [ + EnumFeature source_feature = 15 [ retention = RETENTION_SOURCE, targets = TARGET_TYPE_FILE, targets = TARGET_TYPE_FIELD, @@ -149,7 +164,7 @@ message TestFeatures { edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature source_feature2 = 16 [ + EnumFeature source_feature2 = 16 [ retention = RETENTION_SOURCE, targets = TARGET_TYPE_FILE, targets = TARGET_TYPE_FIELD, @@ -164,7 +179,7 @@ message TestFeatures { edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" } ]; - optional EnumFeature removed_feature = 17 [ + EnumFeature removed_feature = 17 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FILE, targets = TARGET_TYPE_FIELD, @@ -179,7 +194,7 @@ message TestFeatures { edition_defaults = { edition: EDITION_2024, value: "VALUE3" } ]; - optional EnumFeature future_feature = 18 [ + EnumFeature future_feature = 18 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FILE, targets = TARGET_TYPE_FIELD, @@ -188,7 +203,7 @@ message TestFeatures { edition_defaults = { edition: EDITION_2024, value: "VALUE2" } ]; - optional EnumFeature legacy_feature = 19 [ + EnumFeature legacy_feature = 19 [ retention = RETENTION_RUNTIME, targets = TARGET_TYPE_FILE, targets = TARGET_TYPE_FIELD, @@ -199,4 +214,29 @@ message TestFeatures { edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" }, edition_defaults = { edition: EDITION_2023, value: "VALUE2" } ]; + + ValueLifetimeFeature value_lifetime_feature = 20 [ + retention = RETENTION_RUNTIME, + targets = TARGET_TYPE_FILE, + feature_support = { + edition_introduced: EDITION_2023 + edition_deprecated: EDITION_99998_TEST_ONLY + deprecation_warning: "Custom feature deprecation warning" + edition_removed: EDITION_99999_TEST_ONLY + }, + edition_defaults = { + edition: EDITION_LEGACY, + value: "VALUE_LIFETIME_INHERITED" + }, + // Verify edition defaults can use future values. + edition_defaults = { + edition: EDITION_2023, + value: "VALUE_LIFETIME_FUTURE" + }, + // Verify edition defaults can use removed values. + edition_defaults = { + edition: EDITION_99999_TEST_ONLY, + value: "VALUE_LIFETIME_FUTURE" + } + ]; }