Skip to content

Commit

Permalink
Stop supporting EDITION_PROTO2 as an alias for EDITION_LEGACY.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 638902499
  • Loading branch information
@mkruskal-google @copybara-github
mkruskal-google authored and copybara-github committed May 31, 2024
1 parent fac847c commit 35b3425
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 26 deletions.
12 changes: 6 additions & 6 deletions editions/defaults_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ TEST(DefaultsTest, Check2023) {
ASSERT_EQ(defaults->minimum_edition(), EDITION_2023);
ASSERT_EQ(defaults->maximum_edition(), EDITION_2023);

EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_PROTO2);
EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_LEGACY);
EXPECT_EQ(defaults->defaults()[1].edition(), EDITION_PROTO3);
EXPECT_EQ(defaults->defaults()[2].edition(), EDITION_2023);
EXPECT_EQ(defaults->defaults()[2].overridable_features().field_presence(),
Expand All @@ -72,7 +72,7 @@ TEST(DefaultsTest, CheckFuture) {
ASSERT_EQ(defaults->minimum_edition(), EDITION_2023);
ASSERT_EQ(defaults->maximum_edition(), EDITION_99997_TEST_ONLY);

EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_PROTO2);
EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_LEGACY);
EXPECT_EQ(defaults->defaults()[1].edition(), EDITION_PROTO3);
EXPECT_EQ(defaults->defaults()[2].edition(), EDITION_2023);
EXPECT_EQ(defaults->defaults()[2].overridable_features().field_presence(),
Expand Down Expand Up @@ -107,7 +107,7 @@ TEST(DefaultsTest, CheckFarFuture) {
ASSERT_EQ(defaults->minimum_edition(), EDITION_99997_TEST_ONLY);
ASSERT_EQ(defaults->maximum_edition(), EDITION_99999_TEST_ONLY);

EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_PROTO2);
EXPECT_EQ(defaults->defaults()[0].edition(), EDITION_LEGACY);
EXPECT_EQ(defaults->defaults()[1].edition(), EDITION_PROTO3);
EXPECT_EQ(defaults->defaults()[2].edition(), EDITION_2023);
EXPECT_EQ(defaults->defaults()[2].overridable_features().field_presence(),
Expand Down Expand Up @@ -152,7 +152,7 @@ TEST(DefaultsTest, Embedded) {
ASSERT_EQ(defaults.minimum_edition(), EDITION_2023);
ASSERT_EQ(defaults.maximum_edition(), EDITION_2023);

EXPECT_EQ(defaults.defaults()[0].edition(), EDITION_PROTO2);
EXPECT_EQ(defaults.defaults()[0].edition(), EDITION_LEGACY);
EXPECT_EQ(defaults.defaults()[1].edition(), EDITION_PROTO3);
EXPECT_EQ(defaults.defaults()[2].edition(), EDITION_2023);
EXPECT_EQ(defaults.defaults()[2].overridable_features().field_presence(),
Expand All @@ -176,7 +176,7 @@ TEST(DefaultsTest, EmbeddedBase64) {
ASSERT_EQ(defaults.minimum_edition(), EDITION_2023);
ASSERT_EQ(defaults.maximum_edition(), EDITION_2023);

EXPECT_EQ(defaults.defaults()[0].edition(), EDITION_PROTO2);
EXPECT_EQ(defaults.defaults()[0].edition(), EDITION_LEGACY);
EXPECT_EQ(defaults.defaults()[1].edition(), EDITION_PROTO3);
EXPECT_EQ(defaults.defaults()[2].edition(), EDITION_2023);
EXPECT_EQ(defaults.defaults()[2].overridable_features().field_presence(),
Expand Down Expand Up @@ -206,7 +206,7 @@ TEST_F(OverridableDefaultsTest, Proto2) {
ASSERT_OK(feature_defaults);
ASSERT_GE(feature_defaults->defaults().size(), 1);
auto defaults = feature_defaults->defaults(0);
ASSERT_EQ(defaults.edition(), EDITION_PROTO2);
ASSERT_EQ(defaults.edition(), EDITION_LEGACY);


EXPECT_THAT(defaults.overridable_features(), EqualsProto(R"pb([pb.cpp] {}
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/code_generator_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ TEST_F(CodeGeneratorTest, BuildFeatureSetDefaults) {
EXPECT_THAT(generator.BuildFeatureSetDefaults(),
IsOkAndHolds(EqualsProto(R"pb(
defaults {
edition: EDITION_PROTO2
edition: EDITION_LEGACY
overridable_features {}
fixed_features {
field_presence: EXPLICIT
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1872,7 +1872,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaults) {
FeatureSetDefaults defaults = ReadEditionDefaults("defaults");
EXPECT_THAT(defaults, EqualsProto(R"pb(
defaults {
edition: EDITION_PROTO2
edition: EDITION_LEGACY
overridable_features {}
fixed_features {
field_presence: EXPLICIT
Expand Down Expand Up @@ -1924,7 +1924,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithMaximum) {
FeatureSetDefaults defaults = ReadEditionDefaults("defaults");
EXPECT_THAT(defaults, EqualsProto(R"pb(
defaults {
edition: EDITION_PROTO2
edition: EDITION_LEGACY
overridable_features {}
fixed_features {
field_presence: EXPLICIT
Expand Down Expand Up @@ -1977,7 +1977,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithMinimum) {
FeatureSetDefaults defaults = ReadEditionDefaults("defaults");
EXPECT_THAT(defaults, EqualsProto(R"pb(
defaults {
edition: EDITION_PROTO2
edition: EDITION_LEGACY
overridable_features {}
fixed_features {
field_presence: EXPLICIT
Expand Down Expand Up @@ -2032,7 +2032,7 @@ TEST_F(CommandLineInterfaceTest, EditionDefaultsWithExtension) {
EXPECT_EQ(defaults.minimum_edition(), EDITION_PROTO2);
EXPECT_EQ(defaults.maximum_edition(), EDITION_99999_TEST_ONLY);
ASSERT_EQ(defaults.defaults_size(), 6);
EXPECT_EQ(defaults.defaults(0).edition(), EDITION_PROTO2);
EXPECT_EQ(defaults.defaults(0).edition(), EDITION_LEGACY);
EXPECT_EQ(defaults.defaults(2).edition(), EDITION_2023);
EXPECT_EQ(defaults.defaults(3).edition(), EDITION_2024);
EXPECT_EQ(defaults.defaults(4).edition(), EDITION_99997_TEST_ONLY);
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/cpp_edition_defaults.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// the C++ runtime. This is used for feature resolution under Editions.
// NOLINTBEGIN
// clang-format off
#define PROTOBUF_INTERNAL_CPP_EDITION_DEFAULTS "\n\035\030\346\007\"\003\302>\000*\023\010\001\020\002\030\002 \003(\0010\002\302>\004\010\001\020\003\n\035\030\347\007\"\003\302>\000*\023\010\002\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003\n\035\030\350\007\"\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003*\003\302>\000\n\035\030\351\007\"\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\001*\003\302>\000 \346\007(\351\007"
#define PROTOBUF_INTERNAL_CPP_EDITION_DEFAULTS "\n\035\030\204\007\"\003\302>\000*\023\010\001\020\002\030\002 \003(\0010\002\302>\004\010\001\020\003\n\035\030\347\007\"\003\302>\000*\023\010\002\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003\n\035\030\350\007\"\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\003*\003\302>\000\n\035\030\351\007\"\023\010\001\020\001\030\001 \002(\0010\001\302>\004\010\000\020\001*\003\302>\000 \346\007(\351\007"
// clang-format on
// NOLINTEND

Expand Down
15 changes: 5 additions & 10 deletions src/google/protobuf/feature_resolver.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ absl::Status ValidateDescriptor(const Descriptor& descriptor) {

bool has_legacy_default = false;
for (const auto& d : field.options().edition_defaults()) {
if (d.edition() == Edition::EDITION_LEGACY ||
// TODO Remove this once all features use EDITION_LEGACY.
d.edition() == Edition::EDITION_PROTO2) {
if (d.edition() == Edition::EDITION_LEGACY) {
has_legacy_default = true;
continue;
}
Expand Down Expand Up @@ -194,11 +192,6 @@ void CollectEditions(const Descriptor& descriptor, Edition maximum_edition,
for (int i = 0; i < descriptor.field_count(); ++i) {
for (const auto& def : descriptor.field(i)->options().edition_defaults()) {
if (maximum_edition < def.edition()) continue;
// TODO Remove this once all features use EDITION_LEGACY.
if (def.edition() == Edition::EDITION_LEGACY) {
editions.insert(Edition::EDITION_PROTO2);
continue;
}
editions.insert(def.edition());
}
}
Expand Down Expand Up @@ -363,8 +356,10 @@ absl::StatusOr<FeatureSetDefaults> FeatureResolver::CompileDefaults(
}
// Sanity check validation conditions above.
ABSL_CHECK(!editions.empty());
// TODO Check that this is always EDITION_LEGACY.
ABSL_CHECK_LE(*editions.begin(), EDITION_PROTO2);
if (*editions.begin() != EDITION_LEGACY) {
return Error("Minimum edition ", *editions.begin(),
" is not EDITION_LEGACY");
}

if (*editions.begin() > minimum_edition) {
return Error("Minimum edition ", minimum_edition,
Expand Down
8 changes: 4 additions & 4 deletions src/google/protobuf/feature_resolver_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ TEST(FeatureResolverTest, CompileDefaultsOverridable) {

TEST(FeatureResolverTest, CreateFromUnsortedDefaults) {
auto valid_defaults = FeatureResolver::CompileDefaults(
FeatureSet::descriptor(), {}, EDITION_PROTO2, EDITION_2023);
FeatureSet::descriptor(), {}, EDITION_LEGACY, EDITION_2023);
ASSERT_OK(valid_defaults);
FeatureSetDefaults defaults = *valid_defaults;

Expand All @@ -360,7 +360,7 @@ TEST(FeatureResolverTest, CreateFromUnsortedDefaults) {
EXPECT_THAT(FeatureResolver::Create(EDITION_2023, defaults),
HasError(AllOf(HasSubstr("not strictly increasing."),
HasSubstr("Edition PROTO3 is greater "
"than or equal to edition PROTO2"))));
"than or equal to edition LEGACY"))));
}

TEST(FeatureResolverTest, CreateUnknownEdition) {
Expand Down Expand Up @@ -1281,7 +1281,7 @@ TEST_F(FeatureResolverPoolTest, CompileDefaultsInvalidDefaultsTooEarly) {
EXPECT_THAT(
FeatureResolver::CompileDefaults(feature_set_, {ext}, EDITION_2023,
EDITION_2023),
HasError(HasSubstr("No valid default found for edition 2_TEST_ONLY")));
HasError(HasSubstr("Minimum edition 2_TEST_ONLY is not EDITION_LEGACY")));
}

TEST_F(FeatureResolverPoolTest, CompileDefaultsMinimumTooEarly) {
Expand Down Expand Up @@ -1345,7 +1345,7 @@ TEST_F(FeatureResolverPoolTest, CompileDefaultsMinimumCovered) {
minimum_edition: EDITION_99997_TEST_ONLY
maximum_edition: EDITION_99999_TEST_ONLY
defaults {
edition: EDITION_PROTO2
edition: EDITION_LEGACY
overridable_features {
[pb.test] {}
}
Expand Down

0 comments on commit 35b3425

Please sign in to comment.