From c3b72b99b4ce587b487674e33660494dc56b9432 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 6 Jul 2023 13:26:22 -0700 Subject: [PATCH] Migrate internal C++-only test protos to editions. This is some low hanging fruit that can increase our test coverage of editions. Since only the C++ backend is implemented today, we can't yet migrate any protos that are public, are used outside C++, or are depended on by any proto outside C++. PiperOrigin-RevId: 546076822 --- pkg/BUILD.bazel | 2 +- src/google/protobuf/BUILD.bazel | 36 +++++++++++++------ .../protobuf/compiler/allowlists/editions.cc | 2 +- src/google/protobuf/compiler/cpp/BUILD.bazel | 2 +- src/google/protobuf/map_proto3_unittest.proto | 5 ++- .../protobuf/preserve_unknown_enum_test.cc | 10 ------ src/google/protobuf/unittest_arena.proto | 6 ++-- .../unittest_drop_unknown_fields.proto | 4 ++- .../protobuf/unittest_lazy_dependencies.proto | 4 +-- ...test_lazy_dependencies_custom_option.proto | 6 ++-- .../unittest_lazy_dependencies_enum.proto | 5 ++- .../protobuf/unittest_no_field_presence.proto | 5 ++- .../unittest_preserve_unknown_enum.proto | 10 +++--- .../unittest_preserve_unknown_enum2.proto | 12 +++++-- 14 files changed, 68 insertions(+), 41 deletions(-) diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 046e0bca24..936edad57f 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -119,7 +119,7 @@ gen_file_lists( ":test_util": "test_util", # Tests and test-only protos: "//src/google/protobuf:full_test_srcs": "protobuf_test", - "//src/google/protobuf:test_proto_srcs": "protobuf_test_protos", + "//src/google/protobuf:test_proto_all_srcs": "protobuf_test_protos", "//src/google/protobuf:lite_test_srcs": "protobuf_lite_test", "//src/google/protobuf:lite_test_proto_srcs": "protobuf_lite_test_protos", "//src/google/protobuf/compiler:fake_plugin_srcs": "fake_plugin", diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index eab9edc1c6..f1d164510a 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -665,12 +665,9 @@ filegroup( srcs = [ "any_test.proto", "map_proto2_unittest.proto", - "map_proto3_unittest.proto", "map_unittest.proto", "unittest.proto", - "unittest_arena.proto", "unittest_custom_options.proto", - "unittest_drop_unknown_fields.proto", "unittest_embed_optimize_for.proto", "unittest_empty.proto", "unittest_enormous_descriptor.proto", @@ -678,17 +675,11 @@ filegroup( "unittest_import.proto", "unittest_import_public.proto", "unittest_invalid_features.proto", - "unittest_lazy_dependencies.proto", - "unittest_lazy_dependencies_custom_option.proto", - "unittest_lazy_dependencies_enum.proto", "unittest_lite_imports_nonlite.proto", "unittest_mset.proto", "unittest_mset_wire_format.proto", - "unittest_no_field_presence.proto", "unittest_no_generic_services.proto", "unittest_optimize_for.proto", - "unittest_preserve_unknown_enum.proto", - "unittest_preserve_unknown_enum2.proto", "unittest_proto3.proto", "unittest_proto3_arena.proto", "unittest_proto3_arena_lite.proto", @@ -701,9 +692,34 @@ filegroup( visibility = ["//:__subpackages__"], ) +filegroup( + name = "test_proto_editions_srcs", + srcs = [ + "map_proto3_unittest.proto", + "unittest_arena.proto", + "unittest_drop_unknown_fields.proto", + "unittest_lazy_dependencies.proto", + "unittest_lazy_dependencies_custom_option.proto", + "unittest_lazy_dependencies_enum.proto", + "unittest_no_field_presence.proto", + "unittest_preserve_unknown_enum.proto", + "unittest_preserve_unknown_enum2.proto", + ], + visibility = ["//:__subpackages__"], +) + +filegroup( + name = "test_proto_all_srcs", + srcs = [ + ":test_proto_editions_srcs", + ":test_proto_srcs", + ], + visibility = ["//:__subpackages__"], +) + proto_library( name = "test_protos", - srcs = [":test_proto_srcs"], + srcs = [":test_proto_all_srcs"], strip_import_prefix = "/src", visibility = ["//:__subpackages__"], deps = [ diff --git a/src/google/protobuf/compiler/allowlists/editions.cc b/src/google/protobuf/compiler/allowlists/editions.cc index 39bac65676..d50cfa2255 100644 --- a/src/google/protobuf/compiler/allowlists/editions.cc +++ b/src/google/protobuf/compiler/allowlists/editions.cc @@ -42,7 +42,7 @@ namespace compiler { static constexpr auto kEarlyEditionsFile = internal::MakeAllowlist( { // Intentionally left blank. - "google/protobuf/editions/", + "google/protobuf/", }, internal::AllowlistFlags::kMatchPrefix); diff --git a/src/google/protobuf/compiler/cpp/BUILD.bazel b/src/google/protobuf/compiler/cpp/BUILD.bazel index aa9777c99d..c1f0a7a395 100644 --- a/src/google/protobuf/compiler/cpp/BUILD.bazel +++ b/src/google/protobuf/compiler/cpp/BUILD.bazel @@ -141,7 +141,7 @@ cc_test( srcs = ["unittest.cc"], copts = COPTS, data = [ - "//:test_proto_srcs", + "//src/google/protobuf:test_proto_all_srcs", "//src/google/protobuf:testdata", ], deps = [ diff --git a/src/google/protobuf/map_proto3_unittest.proto b/src/google/protobuf/map_proto3_unittest.proto index cea8ef6e8b..bef092d7fb 100644 --- a/src/google/protobuf/map_proto3_unittest.proto +++ b/src/google/protobuf/map_proto3_unittest.proto @@ -28,7 +28,10 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -syntax = "proto3"; +edition = "2023"; + +// Treat all fields as implicit present by default (proto3 behavior). +option features.field_presence = IMPLICIT; // This file contains definitions that have different behavior in proto3. diff --git a/src/google/protobuf/preserve_unknown_enum_test.cc b/src/google/protobuf/preserve_unknown_enum_test.cc index 8d6d8c8915..aba76afe7c 100644 --- a/src/google/protobuf/preserve_unknown_enum_test.cc +++ b/src/google/protobuf/preserve_unknown_enum_test.cc @@ -276,15 +276,5 @@ TEST(PreserveUnknownEnumTest, Proto2CatchesUnknownValues) { EXPECT_EQ(message.unknown_fields().field(2).varint(), 4242); } -TEST(PreserveUnknownEnumTest, SupportsUnknownEnumValuesAPI) { - protobuf_unittest::TestAllTypes proto2_message; - proto3_preserve_unknown_enum_unittest::MyMessage new_message; - - const Reflection* proto2_reflection = proto2_message.GetReflection(); - const Reflection* new_reflection = new_message.GetReflection(); - - EXPECT_FALSE(proto2_reflection->SupportsUnknownEnumValues()); - EXPECT_TRUE(new_reflection->SupportsUnknownEnumValues()); -} } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/unittest_arena.proto b/src/google/protobuf/unittest_arena.proto index 7b3173996e..65e1f43c59 100644 --- a/src/google/protobuf/unittest_arena.proto +++ b/src/google/protobuf/unittest_arena.proto @@ -28,14 +28,16 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -syntax = "proto2"; +edition = "2023"; package proto2_arena_unittest; +// Use expanded encoding for repeated fields by default (proto2 behavior). +option features.repeated_field_encoding = EXPANDED; option cc_enable_arenas = true; message NestedMessage { - optional int32 d = 1; + int32 d = 1; } message ArenaMessage { diff --git a/src/google/protobuf/unittest_drop_unknown_fields.proto b/src/google/protobuf/unittest_drop_unknown_fields.proto index a8a98ad394..c4d0d77f53 100644 --- a/src/google/protobuf/unittest_drop_unknown_fields.proto +++ b/src/google/protobuf/unittest_drop_unknown_fields.proto @@ -28,10 +28,12 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -syntax = "proto3"; +edition = "2023"; package unittest_drop_unknown_fields; +// Treat all fields as implicit present by default (proto3 behavior). +option features.field_presence = IMPLICIT; option objc_class_prefix = "DropUnknowns"; option csharp_namespace = "Google.Protobuf.TestProtos"; diff --git a/src/google/protobuf/unittest_lazy_dependencies.proto b/src/google/protobuf/unittest_lazy_dependencies.proto index 244a88659b..42cabe024e 100644 --- a/src/google/protobuf/unittest_lazy_dependencies.proto +++ b/src/google/protobuf/unittest_lazy_dependencies.proto @@ -34,7 +34,7 @@ // // A proto file we will use for unit testing. -syntax = "proto2"; +edition = "2023"; import "google/protobuf/unittest_lazy_dependencies_custom_option.proto"; @@ -64,7 +64,7 @@ option java_outer_classname = "UnittestLazyImportsProto"; // descriptor lazily and return it. message ImportedMessage { - optional LazyMessage lazy_message = 1; + LazyMessage lazy_message = 1; } message MessageCustomOption {} diff --git a/src/google/protobuf/unittest_lazy_dependencies_custom_option.proto b/src/google/protobuf/unittest_lazy_dependencies_custom_option.proto index 466948ea56..47466705e7 100644 --- a/src/google/protobuf/unittest_lazy_dependencies_custom_option.proto +++ b/src/google/protobuf/unittest_lazy_dependencies_custom_option.proto @@ -34,7 +34,7 @@ // // A proto file we will use for unit testing. -syntax = "proto2"; +edition = "2023"; import "google/protobuf/descriptor.proto"; import "google/protobuf/unittest_lazy_dependencies_enum.proto"; @@ -59,9 +59,9 @@ option optimize_for = SPEED; option java_outer_classname = "UnittestLazyImportsCustomOptionProto"; message LazyMessage { - optional int32 a = 1; + int32 a = 1; } extend google.protobuf.MessageOptions { - optional LazyEnum lazy_enum_option = 138596335 [default = LAZY_ENUM_1]; + LazyEnum lazy_enum_option = 138596335 [default = LAZY_ENUM_1]; } diff --git a/src/google/protobuf/unittest_lazy_dependencies_enum.proto b/src/google/protobuf/unittest_lazy_dependencies_enum.proto index 5edcc1ffa8..d548d46e36 100644 --- a/src/google/protobuf/unittest_lazy_dependencies_enum.proto +++ b/src/google/protobuf/unittest_lazy_dependencies_enum.proto @@ -34,7 +34,10 @@ // // A proto file we will use for unit testing. -syntax = "proto2"; +edition = "2023"; + +// Treat all enums as closed by default (proto2 behavior). +option features.enum_type = CLOSED; // Some generic_services option(s) added automatically. // See: http://go/proto2-generic-services-default diff --git a/src/google/protobuf/unittest_no_field_presence.proto b/src/google/protobuf/unittest_no_field_presence.proto index c03b7a5e1f..c993afd8ed 100644 --- a/src/google/protobuf/unittest_no_field_presence.proto +++ b/src/google/protobuf/unittest_no_field_presence.proto @@ -30,7 +30,10 @@ // A proto file used to test a message type with no explicit field presence. -syntax = "proto3"; +edition = "2023"; + +// Treat all fields as implicit present by default (proto3 behavior). +option features.field_presence = IMPLICIT; // We want to test embedded proto2 messages, so include some proto2 types. package proto2_nofieldpresence_unittest; diff --git a/src/google/protobuf/unittest_preserve_unknown_enum.proto b/src/google/protobuf/unittest_preserve_unknown_enum.proto index ab51c001ed..0d4f815e58 100644 --- a/src/google/protobuf/unittest_preserve_unknown_enum.proto +++ b/src/google/protobuf/unittest_preserve_unknown_enum.proto @@ -28,10 +28,12 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -syntax = "proto3"; +edition = "2023"; package proto3_preserve_unknown_enum_unittest; +// Treat all fields as implicit present by default (proto3 behavior). +option features.field_presence = IMPLICIT; option objc_class_prefix = "UnknownEnums"; option csharp_namespace = "Google.Protobuf.TestProtos"; @@ -51,7 +53,7 @@ enum MyEnumPlusExtra { message MyMessage { MyEnum e = 1; repeated MyEnum repeated_e = 2; - repeated MyEnum repeated_packed_e = 3 [packed = true]; + repeated MyEnum repeated_packed_e = 3; repeated MyEnumPlusExtra repeated_packed_unexpected_e = 4; // not packed oneof o { MyEnum oneof_e_1 = 5; @@ -62,8 +64,8 @@ message MyMessage { message MyMessagePlusExtra { MyEnumPlusExtra e = 1; repeated MyEnumPlusExtra repeated_e = 2; - repeated MyEnumPlusExtra repeated_packed_e = 3 [packed = true]; - repeated MyEnumPlusExtra repeated_packed_unexpected_e = 4 [packed = true]; + repeated MyEnumPlusExtra repeated_packed_e = 3; + repeated MyEnumPlusExtra repeated_packed_unexpected_e = 4; oneof o { MyEnumPlusExtra oneof_e_1 = 5; MyEnumPlusExtra oneof_e_2 = 6; diff --git a/src/google/protobuf/unittest_preserve_unknown_enum2.proto b/src/google/protobuf/unittest_preserve_unknown_enum2.proto index dfce0e06b0..474989a4e7 100644 --- a/src/google/protobuf/unittest_preserve_unknown_enum2.proto +++ b/src/google/protobuf/unittest_preserve_unknown_enum2.proto @@ -28,10 +28,15 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -syntax = "proto2"; +edition = "2023"; package proto2_preserve_unknown_enum_unittest; +// Treat all enums as closed and use expanded encoding for repeated fields by +// default (proto2 behavior). +option features.enum_type = CLOSED; +option features.repeated_field_encoding = EXPANDED; + enum MyEnum { FOO = 0; BAR = 1; @@ -39,9 +44,10 @@ enum MyEnum { } message MyMessage { - optional MyEnum e = 1; + MyEnum e = 1; repeated MyEnum repeated_e = 2; - repeated MyEnum repeated_packed_e = 3 [packed = true]; + repeated MyEnum repeated_packed_e = 3 + [features.repeated_field_encoding = PACKED]; repeated MyEnum repeated_packed_unexpected_e = 4; // not packed oneof o { MyEnum oneof_e_1 = 5;