Skip to content

Commit 242db71

Browse files
protobuf-github-botcopybara-github
protobuf-github-bot
authored and
copybara-github
committed
Internal changes
PiperOrigin-RevId: 824605974
1 parent ec86098 commit 242db71

File tree

4 files changed

+52
-40
lines changed

4 files changed

+52
-40
lines changed

‎src/google/protobuf/descriptor_unittest.cc‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12200,9 +12200,9 @@ TEST_F(FeaturesTest, DeprecatedFeature) {
1220012200
}
1220112201
}
1220212202
)pb",
12203-
"foo.proto: foo.proto: NAME: Feature "
12204-
"pb.TestFeatures.removed_feature has been deprecated in edition 2023: "
12205-
"Custom feature deprecation warning\n");
12203+
"foo.proto: foo.proto: NAME: "
12204+
"pb.TestFeatures.removed_feature has been deprecated in edition 2023 and "
12205+
"above. Custom feature deprecation warning\n");
1220612206
const FileDescriptor* file = pool_.FindFileByName("foo.proto");
1220712207
ASSERT_THAT(file, NotNull());
1220812208

@@ -12287,9 +12287,9 @@ TEST_F(FeaturesTest, RemovedFeature) {
1228712287
}
1228812288
}
1228912289
)pb",
12290-
"foo.proto: foo.proto: NAME: Feature "
12291-
"pb.TestFeatures.removed_feature has been removed in edition 2024 and "
12292-
"can't be used in edition 2024\n");
12290+
"foo.proto: foo.proto: NAME: "
12291+
"pb.TestFeatures.removed_feature has been removed in edition 2024. "
12292+
"Custom feature removal error\n");
1229312293
}
1229412294

1229512295
TEST_F(FeaturesTest, RemovedFeatureDefault) {
@@ -12319,9 +12319,9 @@ TEST_F(FeaturesTest, FutureFeature) {
1231912319
}
1232012320
}
1232112321
)pb",
12322-
"foo.proto: foo.proto: NAME: Feature "
12323-
"pb.TestFeatures.future_feature wasn't introduced until edition 2024 and "
12324-
"can't be used in edition 2023\n");
12322+
"foo.proto: foo.proto: NAME: "
12323+
"pb.TestFeatures.future_feature is only available in edition 2024 and "
12324+
"onwards.\n");
1232512325
}
1232612326

1232712327
TEST_F(FeaturesTest, FutureFeatureDefault) {
@@ -14122,9 +14122,9 @@ TEST_F(DatabaseBackedPoolTest, FeatureLifetimeError) {
1412214122

1412314123
EXPECT_TRUE(pool.FindMessageTypeByName("FooFeatures") == nullptr);
1412414124
EXPECT_EQ(error_collector.text_,
14125-
"features.proto: FooFeatures: NAME: Feature "
14126-
"pb.TestFeatures.future_feature wasn't introduced until edition "
14127-
"2024 and can't be used in edition 2023\n");
14125+
"features.proto: FooFeatures: NAME: "
14126+
"pb.TestFeatures.future_feature is only available in edition 2024 "
14127+
"and onwards.\n");
1412814128
}
1412914129

1413014130
TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) {
@@ -14193,9 +14193,9 @@ TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) {
1419314193
error_collector.text_.clear();
1419414194
ASSERT_EQ(pool.FindExtensionByName("foo_extension"), nullptr);
1419514195
EXPECT_EQ(error_collector.text_,
14196-
"option.proto: foo_extension: NAME: Feature "
14197-
"pb.TestFeatures.legacy_feature has been removed in edition 2023 "
14198-
"and can't be used in edition 2023\n");
14196+
"option.proto: foo_extension: NAME: "
14197+
"pb.TestFeatures.legacy_feature has been removed in edition 2023. "
14198+
"Custom feature removal error\n");
1419914199
}
1420014200

1420114201
TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) {

‎src/google/protobuf/feature_resolver.cc‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "absl/strings/str_join.h"
2727
#include "absl/strings/str_split.h"
2828
#include "absl/strings/string_view.h"
29+
#include "absl/strings/substitute.h"
2930
#include "absl/types/span.h"
3031
#include "google/protobuf/cpp_features.pb.h"
3132
#include "google/protobuf/descriptor.h"
@@ -364,26 +365,32 @@ absl::Status ValidateMergedFeatures(const FeatureSet& features) {
364365

365366
void ValidateSingleFeatureLifetimes(
366367
Edition edition, absl::string_view full_name,
367-
const FieldOptions::FeatureSupport& support,
368+
const FieldOptions::FeatureSupport& feature_support,
368369
FeatureResolver::ValidationResults& results) {
369370
// Skip fields that don't have feature support specified.
370-
if (&support == &FieldOptions::FeatureSupport::default_instance()) return;
371-
372-
if (edition < support.edition_introduced()) {
373-
results.errors.emplace_back(
374-
absl::StrCat("Feature ", full_name, " wasn't introduced until edition ",
375-
support.edition_introduced(),
376-
" and can't be used in edition ", edition));
377-
}
378-
if (support.has_edition_removed() && edition >= support.edition_removed()) {
379-
results.errors.emplace_back(absl::StrCat(
380-
"Feature ", full_name, " has been removed in edition ",
381-
support.edition_removed(), " and can't be used in edition ", edition));
382-
} else if (support.has_edition_deprecated() &&
383-
edition >= support.edition_deprecated()) {
384-
results.warnings.emplace_back(absl::StrCat(
385-
"Feature ", full_name, " has been deprecated in edition ",
386-
support.edition_deprecated(), ": ", support.deprecation_warning()));
371+
if (&feature_support == &FieldOptions::FeatureSupport::default_instance())
372+
return;
373+
// safe guarding new features that aren't available yet
374+
if (edition < feature_support.edition_introduced()) {
375+
const std::string error_message =
376+
absl::Substitute("$0 is only available in edition $1 and onwards.",
377+
full_name, feature_support.edition_introduced());
378+
results.errors.emplace_back(error_message.c_str());
379+
}
380+
if (feature_support.has_edition_removed() &&
381+
edition >= feature_support.edition_removed()) {
382+
const std::string error_message = absl::Substitute(
383+
"$0 has been removed in edition $1. $2", full_name,
384+
feature_support.edition_removed(), feature_support.removal_error());
385+
results.errors.emplace_back(error_message.c_str());
386+
} else if (feature_support.has_edition_deprecated() &&
387+
edition >= feature_support.edition_deprecated()) {
388+
const std::string error_message = absl::Substitute(
389+
"$0 has been deprecated in edition "
390+
"$1 and above. $2",
391+
full_name, feature_support.edition_deprecated(),
392+
feature_support.deprecation_warning());
393+
results.warnings.emplace_back(error_message.c_str());
387394
}
388395
}
389396

‎src/google/protobuf/feature_resolver_test.cc‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,8 @@ TEST(FeatureResolverLifetimesTest, RemovedFeature) {
634634
features, nullptr);
635635
EXPECT_THAT(results.errors,
636636
ElementsAre(AllOf(HasSubstr("pb.TestFeatures.removed_feature"),
637-
HasSubstr("removed in edition 2024"))));
637+
HasSubstr("removed in edition 2024"),
638+
HasSubstr("Custom feature removal error"))));
638639
EXPECT_THAT(results.warnings, IsEmpty());
639640
}
640641

@@ -646,7 +647,7 @@ TEST(FeatureResolverLifetimesTest, NotIntroduced) {
646647
features, nullptr);
647648
EXPECT_THAT(results.errors,
648649
ElementsAre(AllOf(HasSubstr("pb.TestFeatures.future_feature"),
649-
HasSubstr("introduced until edition 2024"))));
650+
HasSubstr("only available in edition 2024"))));
650651
EXPECT_THAT(results.warnings, IsEmpty());
651652
}
652653

@@ -730,7 +731,7 @@ TEST(FeatureResolverLifetimesTest, ValueSupportBeforeIntroduced) {
730731
EXPECT_THAT(results.errors,
731732
ElementsAre(AllOf(
732733
HasSubstr("pb.VALUE_LIFETIME_FUTURE"),
733-
HasSubstr("introduced until edition 99997_TEST_ONLY"))));
734+
HasSubstr("only available in edition 99997_TEST_ONLY"))));
734735
EXPECT_THAT(results.warnings, IsEmpty());
735736
}
736737

@@ -740,10 +741,10 @@ TEST(FeatureResolverLifetimesTest, ValueSupportAfterRemoved) {
740741
)pb");
741742
auto results = FeatureResolver::ValidateFeatureLifetimes(
742743
EDITION_99997_TEST_ONLY, features, nullptr);
743-
EXPECT_THAT(
744-
results.errors,
745-
ElementsAre(AllOf(HasSubstr("pb.VALUE_LIFETIME_REMOVED"),
746-
HasSubstr("removed in edition 99997_TEST_ONLY"))));
744+
EXPECT_THAT(results.errors,
745+
ElementsAre(AllOf(HasSubstr("pb.VALUE_LIFETIME_REMOVED"),
746+
HasSubstr("removed in edition 99997_TEST_ONLY"),
747+
HasSubstr("Custom feature removal error"))));
747748
EXPECT_THAT(results.warnings, IsEmpty());
748749
}
749750

‎src/google/protobuf/unittest_features.proto‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ enum ValueLifetimeFeature {
6666
VALUE_LIFETIME_REMOVED = 6 [feature_support = {
6767
edition_deprecated: EDITION_2023
6868
edition_removed: EDITION_99997_TEST_ONLY
69+
removal_error: "Custom feature removal error"
6970
}];
7071
}
7172

@@ -190,6 +191,7 @@ message TestFeatures {
190191
edition_deprecated: EDITION_2023
191192
deprecation_warning: "Custom feature deprecation warning"
192193
edition_removed: EDITION_2024
194+
removal_error: "Custom feature removal error"
193195
},
194196
edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" },
195197
edition_defaults = { edition: EDITION_2023, value: "VALUE2" },
@@ -212,6 +214,7 @@ message TestFeatures {
212214
feature_support = {
213215
edition_introduced: EDITION_PROTO3
214216
edition_removed: EDITION_2023
217+
removal_error: "Custom feature removal error"
215218
},
216219
edition_defaults = { edition: EDITION_LEGACY, value: "VALUE1" },
217220
edition_defaults = { edition: EDITION_2023, value: "VALUE2" }
@@ -225,6 +228,7 @@ message TestFeatures {
225228
edition_deprecated: EDITION_99998_TEST_ONLY
226229
deprecation_warning: "Custom feature deprecation warning"
227230
edition_removed: EDITION_99999_TEST_ONLY
231+
removal_error: "Customer feature removal error"
228232
},
229233
edition_defaults = {
230234
edition: EDITION_LEGACY,

0 commit comments

Comments
 (0)