Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1762,8 +1762,9 @@ TEST_F(CommandLineInterfaceTest, Plugin_DeprecatedFeature) {
Run("protocol_compiler --test_out=$tmpdir "
"--proto_path=$tmpdir foo.proto");
ExpectWarningSubstring(
"foo.proto:4:5: warning: Feature pb.TestFeatures.removed_feature has "
"been deprecated in edition 2023: Custom feature deprecation warning\n");
"foo.proto:4:5: warning: pb.TestFeatures.removed_feature has "
"been deprecated in edition 2023: Custom feature "
"deprecation warning\n");
}

TEST_F(CommandLineInterfaceTest, Plugin_TransitiveDeprecatedFeature) {
Expand Down Expand Up @@ -2031,8 +2032,8 @@ TEST_F(CommandLineInterfaceTest, GeneratorFeatureLifetimeError) {
Run("protocol_compiler --experimental_editions --proto_path=$tmpdir "
"--test_out=$tmpdir foo.proto");
ExpectErrorSubstring(
"foo.proto:6:13: Feature pb.TestFeatures.removed_feature has been "
"removed in edition 2024");
"foo.proto:6:13: pb.TestFeatures.removed_feature has been "
"removed in edition 2024:");
}

TEST_F(CommandLineInterfaceTest, PluginFeatureLifetimeError) {
Expand Down Expand Up @@ -2064,8 +2065,8 @@ TEST_F(CommandLineInterfaceTest, PluginFeatureLifetimeError) {
"foo.proto --plugin=prefix-gen-fake_plugin=",
plugin_path));
ExpectErrorSubstring(
"foo.proto:6:13: Feature pb.TestFeatures.future_feature wasn't "
"introduced until edition 2024");
"foo.proto:6:13: pb.TestFeatures.future_feature wasn't introduced "
"until edition 2024 and can't be used in edition 2023");
}

TEST_F(CommandLineInterfaceTest, GeneratorNoEditionsSupport) {
Expand Down
6 changes: 3 additions & 3 deletions src/google/protobuf/compiler/cpp/generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ TEST_F(CppGeneratorTest, LegacyClosedEnum) {
"protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir foo.proto");

ExpectWarningSubstring(
"foo.proto:9:16: warning: Feature pb.CppFeatures.legacy_closed_enum has "
"been deprecated in edition 2023");
"foo.proto:9:16: warning: pb.CppFeatures.legacy_closed_enum has "
"been deprecated in edition 2023:");
}

TEST_F(CppGeneratorTest, LegacyClosedEnumInherited) {
Expand All @@ -123,7 +123,7 @@ TEST_F(CppGeneratorTest, LegacyClosedEnumInherited) {
"protocol_compiler --proto_path=$tmpdir --cpp_out=$tmpdir foo.proto");

ExpectWarningSubstring(
"foo.proto: warning: Feature pb.CppFeatures.legacy_closed_enum has "
"foo.proto: warning: pb.CppFeatures.legacy_closed_enum has "
"been deprecated in edition 2023");
}

Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/java/generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ BBB = 1;
"--experimental_editions");

ExpectErrorSubstring(
"foo.proto:6:6: Feature pb.JavaFeatures.large_enum wasn't introduced "
"foo.proto:6:6: pb.JavaFeatures.large_enum wasn't introduced "
"until edition 2024 and can't be used in edition 2023");
}

Expand Down
29 changes: 15 additions & 14 deletions src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12200,7 +12200,7 @@ TEST_F(FeaturesTest, DeprecatedFeature) {
}
}
)pb",
"foo.proto: foo.proto: NAME: Feature "
"foo.proto: foo.proto: NAME: "
"pb.TestFeatures.removed_feature has been deprecated in edition 2023: "
"Custom feature deprecation warning\n");
const FileDescriptor* file = pool_.FindFileByName("foo.proto");
Expand Down Expand Up @@ -12287,9 +12287,9 @@ TEST_F(FeaturesTest, RemovedFeature) {
}
}
)pb",
"foo.proto: foo.proto: NAME: Feature "
"pb.TestFeatures.removed_feature has been removed in edition 2024 and "
"can't be used in edition 2024\n");
"foo.proto: foo.proto: NAME: "
"pb.TestFeatures.removed_feature has been removed in edition 2024: "
"Custom feature removal error\n");
}

TEST_F(FeaturesTest, RemovedFeatureDefault) {
Expand Down Expand Up @@ -12319,9 +12319,9 @@ TEST_F(FeaturesTest, FutureFeature) {
}
}
)pb",
"foo.proto: foo.proto: NAME: Feature "
"pb.TestFeatures.future_feature wasn't introduced until edition 2024 and "
"can't be used in edition 2023\n");
"foo.proto: foo.proto: NAME: "
"pb.TestFeatures.future_feature wasn't introduced until edition "
"2024 and can't be used in edition 2023\n");
}

TEST_F(FeaturesTest, FutureFeatureDefault) {
Expand Down Expand Up @@ -14121,10 +14121,11 @@ 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 and can't be used in edition 2023\n");
EXPECT_EQ(
error_collector.text_,
"features.proto: FooFeatures: NAME: "
"pb.TestFeatures.future_feature wasn't introduced until edition 2024 "
"and can't be used in edition 2023\n");
}

TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) {
Expand Down Expand Up @@ -14193,9 +14194,9 @@ TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) {
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 "
"and can't be used in edition 2023\n");
"option.proto: foo_extension: NAME: "
"pb.TestFeatures.legacy_feature has been removed in edition 2023: "
"Custom feature removal error\n");
}

TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) {
Expand Down
47 changes: 29 additions & 18 deletions src/google/protobuf/feature_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "absl/strings/substitute.h"
#include "absl/types/span.h"
#include "google/protobuf/cpp_features.pb.h"
#include "google/protobuf/descriptor.h"
Expand Down Expand Up @@ -364,26 +365,36 @@ absl::Status ValidateMergedFeatures(const FeatureSet& features) {

void ValidateSingleFeatureLifetimes(
Edition edition, absl::string_view full_name,
const FieldOptions::FeatureSupport& support,
const FieldOptions::FeatureSupport& feature_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()));
if (&feature_support == &FieldOptions::FeatureSupport::default_instance())
return;
// safe guarding new features that aren't available yet
if (edition < feature_support.edition_introduced()) {
std::string error_message = absl::Substitute(
"$0 wasn't introduced until edition $1 and can't be used in "
"edition $2",
full_name, feature_support.edition_introduced(), edition);
results.errors.emplace_back(std::move(error_message));
}
if (feature_support.has_edition_removed() &&
edition >= feature_support.edition_removed()) {
std::string error_message = absl::Substitute(
"$0 has been removed in edition $1$2", full_name,
feature_support.edition_removed(),
(feature_support.has_removal_error())
? absl::StrCat(": ", feature_support.removal_error())
: "");
results.errors.emplace_back(std::move(error_message));
} else if (feature_support.has_edition_deprecated() &&
edition >= feature_support.edition_deprecated()) {
std::string error_message = absl::Substitute(
"$0 has been deprecated in edition "
"$1: $2",
full_name, feature_support.edition_deprecated(),
feature_support.deprecation_warning());
results.warnings.emplace_back(std::move(error_message));
}
}

Expand Down
35 changes: 26 additions & 9 deletions src/google/protobuf/feature_resolver_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,21 @@ TEST(FeatureResolverLifetimesTest, RemovedFeature) {
features, nullptr);
EXPECT_THAT(results.errors,
ElementsAre(AllOf(HasSubstr("pb.TestFeatures.removed_feature"),
HasSubstr("removed in edition 2024"))));
HasSubstr("removed in edition 2024:"),
HasSubstr("Custom feature removal error"))));
EXPECT_THAT(results.warnings, IsEmpty());
}

TEST(FeatureResolverLifetimesTest, RemovedFeatureWithNoRemovalError) {
FeatureSet features = ParseTextOrDie(R"pb(
[pb.test] { same_edition_removed_feature: VALUE1 }
)pb");
auto results = FeatureResolver::ValidateFeatureLifetimes(EDITION_2023,
features, nullptr);
EXPECT_THAT(results.errors,
ElementsAre(AllOf(
HasSubstr("pb.TestFeatures.same_edition_removed_feature"),
HasSubstr("removed in edition 2023"), Not(HasSubstr(":")))));
EXPECT_THAT(results.warnings, IsEmpty());
}

Expand All @@ -644,9 +658,11 @@ TEST(FeatureResolverLifetimesTest, NotIntroduced) {
)pb");
auto results = FeatureResolver::ValidateFeatureLifetimes(EDITION_2023,
features, nullptr);
EXPECT_THAT(results.errors,
ElementsAre(AllOf(HasSubstr("pb.TestFeatures.future_feature"),
HasSubstr("introduced until edition 2024"))));
EXPECT_THAT(
results.errors,
ElementsAre(AllOf(HasSubstr("pb.TestFeatures.future_feature"),
HasSubstr("wasn't introduced until edition 2024"),
HasSubstr("can't be used in edition 2023"))));
EXPECT_THAT(results.warnings, IsEmpty());
}

Expand Down Expand Up @@ -730,7 +746,8 @@ TEST(FeatureResolverLifetimesTest, ValueSupportBeforeIntroduced) {
EXPECT_THAT(results.errors,
ElementsAre(AllOf(
HasSubstr("pb.VALUE_LIFETIME_FUTURE"),
HasSubstr("introduced until edition 99997_TEST_ONLY"))));
HasSubstr("wasn't introduced until edition 99997_TEST_ONLY"),
HasSubstr("can't be used in edition 2023"))));
EXPECT_THAT(results.warnings, IsEmpty());
}

Expand All @@ -740,10 +757,10 @@ TEST(FeatureResolverLifetimesTest, ValueSupportAfterRemoved) {
)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.errors,
ElementsAre(AllOf(HasSubstr("pb.VALUE_LIFETIME_REMOVED"),
HasSubstr("removed in edition 99997_TEST_ONLY"),
HasSubstr("Custom feature removal error"))));
EXPECT_THAT(results.warnings, IsEmpty());
}

Expand Down
15 changes: 15 additions & 0 deletions src/google/protobuf/unittest_features.proto
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ enum ValueLifetimeFeature {
VALUE_LIFETIME_REMOVED = 6 [feature_support = {
edition_deprecated: EDITION_2023
edition_removed: EDITION_99997_TEST_ONLY
removal_error: "Custom feature removal error"
}];
}

Expand Down Expand Up @@ -190,12 +191,24 @@ message TestFeatures {
edition_deprecated: EDITION_2023
deprecation_warning: "Custom feature deprecation warning"
edition_removed: EDITION_2024
removal_error: "Custom feature removal error"
},
edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" },
edition_defaults = { edition: EDITION_2023, value: "VALUE2" },
edition_defaults = { edition: EDITION_2024, value: "VALUE3" }
];

EnumFeature same_edition_removed_feature = 21 [
retention = RETENTION_RUNTIME,
targets = TARGET_TYPE_FILE,
targets = TARGET_TYPE_FIELD,
feature_support = {
edition_introduced: EDITION_2023
edition_removed: EDITION_2023
},
edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" }
];

EnumFeature future_feature = 18 [
retention = RETENTION_RUNTIME,
targets = TARGET_TYPE_FILE,
Expand All @@ -212,6 +225,7 @@ message TestFeatures {
feature_support = {
edition_introduced: EDITION_PROTO3
edition_removed: EDITION_2023
removal_error: "Custom feature removal error"
},
edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" },
edition_defaults = { edition: EDITION_2023, value: "VALUE2" }
Expand All @@ -225,6 +239,7 @@ message TestFeatures {
edition_deprecated: EDITION_99998_TEST_ONLY
deprecation_warning: "Custom feature deprecation warning"
edition_removed: EDITION_99999_TEST_ONLY
removal_error: "Custom feature removal error"
},
edition_defaults = {
edition: EDITION_LEGACY,
Expand Down
Loading