From 35b34252c79d661fb1e9ceecdc6457295f28b9b1 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 30 May 2024 22:11:40 -0700 Subject: [PATCH] Stop supporting EDITION_PROTO2 as an alias for EDITION_LEGACY. PiperOrigin-RevId: 638902499 --- editions/defaults_test.cc | 12 ++++++------ .../protobuf/compiler/code_generator_unittest.cc | 2 +- .../compiler/command_line_interface_unittest.cc | 8 ++++---- src/google/protobuf/cpp_edition_defaults.h | 2 +- src/google/protobuf/feature_resolver.cc | 15 +++++---------- src/google/protobuf/feature_resolver_test.cc | 8 ++++---- 6 files changed, 21 insertions(+), 26 deletions(-) diff --git a/editions/defaults_test.cc b/editions/defaults_test.cc index 38d0e93356..b9d001d41b 100644 --- a/editions/defaults_test.cc +++ b/editions/defaults_test.cc @@ -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(), @@ -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(), @@ -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(), @@ -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(), @@ -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(), @@ -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] {} diff --git a/src/google/protobuf/compiler/code_generator_unittest.cc b/src/google/protobuf/compiler/code_generator_unittest.cc index c88bb53092..3e5381bce5 100644 --- a/src/google/protobuf/compiler/code_generator_unittest.cc +++ b/src/google/protobuf/compiler/code_generator_unittest.cc @@ -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 diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index e9d4dedd00..8ab3740cb7 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -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 @@ -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 @@ -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 @@ -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); diff --git a/src/google/protobuf/cpp_edition_defaults.h b/src/google/protobuf/cpp_edition_defaults.h index f5e1d76675..33cdbab8a3 100644 --- a/src/google/protobuf/cpp_edition_defaults.h +++ b/src/google/protobuf/cpp_edition_defaults.h @@ -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 diff --git a/src/google/protobuf/feature_resolver.cc b/src/google/protobuf/feature_resolver.cc index bf24e18a17..5faadf2834 100644 --- a/src/google/protobuf/feature_resolver.cc +++ b/src/google/protobuf/feature_resolver.cc @@ -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; } @@ -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()); } } @@ -363,8 +356,10 @@ absl::StatusOr 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, diff --git a/src/google/protobuf/feature_resolver_test.cc b/src/google/protobuf/feature_resolver_test.cc index 8a56ea8788..e5b92be9a9 100644 --- a/src/google/protobuf/feature_resolver_test.cc +++ b/src/google/protobuf/feature_resolver_test.cc @@ -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; @@ -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) { @@ -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) { @@ -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] {} }