From 65c65c2d04b293225db150029d005056a9f078b8 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 22 Jan 2024 09:54:16 -0800 Subject: [PATCH] Breaking Change: Use Editions features in Java full runtimes. This change breaks compatibility with old generated code from previous major versions per the Cross Version Runtime policy: https://protobuf.dev/support/cross-version-runtime-guarantee. This includes old gencode from <4.26.x, which does not resolve features. See https://protobuf.dev/news/2023-12-05/ PiperOrigin-RevId: 600487923 --- BUILD.bazel | 6 + conformance/BUILD.bazel | 2 + conformance/ConformanceJava.java | 134 ++-- conformance/failure_list_java.txt | 73 ++- conformance/text_format_failure_list_java.txt | 8 + java/core/BUILD.bazel | 22 + java/core/generate-sources-build.xml | 2 + java/core/pom.xml | 6 + .../DescriptorMessageInfoFactory.java | 15 +- .../java/com/google/protobuf/Descriptors.java | 584 ++++++++++++++++-- .../google/protobuf/JavaEditionDefaults.java | 10 + .../JavaEditionDefaults.java.template | 9 + .../com/google/protobuf/DescriptorsTest.java | 472 +++++++++++++- java/pom.xml | 1 + python/build_targets.bzl | 1 + .../protobuf/internal/descriptor_test.py | 8 +- src/google/protobuf/BUILD.bazel | 22 + .../compiler/command_line_interface.cc | 8 + .../protobuf/compiler/java/generator.cc | 13 +- .../compiler/java/kotlin_generator.cc | 3 +- src/google/protobuf/editions/BUILD | 14 + .../protobuf/unittest_legacy_features.proto | 2 +- 22 files changed, 1263 insertions(+), 152 deletions(-) create mode 100644 java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java create mode 100644 java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java.template rename python/google/protobuf/internal/legacy_features.proto => src/google/protobuf/unittest_legacy_features.proto (94%) diff --git a/BUILD.bazel b/BUILD.bazel index c37fb11b032bf..af75be8c6de5f 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -420,6 +420,12 @@ alias( visibility = ["//:__subpackages__"], ) +alias( + name = "test_proto_editions_srcs", + actual = "//src/google/protobuf:test_proto_editions_srcs", # filegroup + visibility = ["//:__subpackages__"], +) + alias( name = "test_protos", actual = "//src/google/protobuf:test_protos", # proto_library diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index 056f8de40fe00..f9533eb469dc0 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -255,6 +255,8 @@ java_binary( "//:protobuf_java_util", "//:test_messages_proto2_java_proto", "//:test_messages_proto3_java_proto", + "//src/google/protobuf/editions:test_messages_proto2_editions_java_proto", + "//src/google/protobuf/editions:test_messages_proto3_editions_java_proto", ], ) diff --git a/conformance/ConformanceJava.java b/conformance/ConformanceJava.java index e76dade5973d1..950657ef83994 100644 --- a/conformance/ConformanceJava.java +++ b/conformance/ConformanceJava.java @@ -15,6 +15,8 @@ import com.google.protobuf.conformance.Conformance; import com.google.protobuf.util.JsonFormat; import com.google.protobuf.util.JsonFormat.TypeRegistry; +import com.google.protobuf_test_messages.editions.proto2.TestMessagesProto2Editions; +import com.google.protobuf_test_messages.editions.proto3.TestMessagesProto3Editions; import com.google.protobuf_test_messages.proto2.TestMessagesProto2; import com.google.protobuf_test_messages.proto2.TestMessagesProto2.TestAllTypesProto2; import com.google.protobuf_test_messages.proto3.TestMessagesProto3; @@ -205,42 +207,63 @@ private T parseBinary( return messages.get(0); } + private Class createTestMessage(String messageType) { + switch (messageType) { + case "protobuf_test_messages.proto3.TestAllTypesProto3": + return TestAllTypesProto3.class; + case "protobuf_test_messages.proto2.TestAllTypesProto2": + return TestAllTypesProto2.class; + case "protobuf_test_messages.editions.proto3.TestAllTypesProto3": + return TestMessagesProto3Editions.TestAllTypesProto3.class; + case "protobuf_test_messages.editions.proto2.TestAllTypesProto2": + return TestMessagesProto2Editions.TestAllTypesProto2.class; + default: + throw new IllegalArgumentException( + "Protobuf request has unexpected payload type: " + messageType); + } + } + + private Class createTestFile(String messageType) { + switch (messageType) { + case "protobuf_test_messages.proto3.TestAllTypesProto3": + return TestMessagesProto3.class; + case "protobuf_test_messages.proto2.TestAllTypesProto2": + return TestMessagesProto2.class; + case "protobuf_test_messages.editions.proto3.TestAllTypesProto3": + return TestMessagesProto3Editions.class; + case "protobuf_test_messages.editions.proto2.TestAllTypesProto2": + return TestMessagesProto2Editions.class; + default: + throw new IllegalArgumentException( + "Protobuf request has unexpected payload type: " + messageType); + } + } + + @SuppressWarnings("unchecked") private Conformance.ConformanceResponse doTest(Conformance.ConformanceRequest request) { AbstractMessage testMessage; String messageType = request.getMessageType(); - boolean isProto3 = messageType.equals("protobuf_test_messages.proto3.TestAllTypesProto3"); - boolean isProto2 = messageType.equals("protobuf_test_messages.proto2.TestAllTypesProto2"); switch (request.getPayloadCase()) { case PROTOBUF_PAYLOAD: { - if (isProto3) { - try { - ExtensionRegistry extensions = ExtensionRegistry.newInstance(); - TestMessagesProto3.registerAllExtensions(extensions); - testMessage = - parseBinary( - request.getProtobufPayload(), TestAllTypesProto3.parser(), extensions); - } catch (InvalidProtocolBufferException e) { - return Conformance.ConformanceResponse.newBuilder() - .setParseError(e.getMessage()) - .build(); - } - } else if (isProto2) { - try { - ExtensionRegistry extensions = ExtensionRegistry.newInstance(); - TestMessagesProto2.registerAllExtensions(extensions); - testMessage = - parseBinary( - request.getProtobufPayload(), TestAllTypesProto2.parser(), extensions); - } catch (InvalidProtocolBufferException e) { - return Conformance.ConformanceResponse.newBuilder() - .setParseError(e.getMessage()) - .build(); - } - } else { - throw new IllegalArgumentException( - "Protobuf request has unexpected payload type: " + messageType); + try { + ExtensionRegistry extensions = ExtensionRegistry.newInstance(); + createTestFile(messageType) + .getMethod("registerAllExtensions", ExtensionRegistry.class) + .invoke(null, extensions); + testMessage = + parseBinary( + request.getProtobufPayload(), + (Parser) + createTestMessage(messageType).getMethod("parser").invoke(null), + extensions); + } catch (InvalidProtocolBufferException e) { + return Conformance.ConformanceResponse.newBuilder() + .setParseError(e.getMessage()) + .build(); + } catch (Exception e) { + throw new RuntimeException(e); } break; } @@ -252,54 +275,34 @@ private Conformance.ConformanceResponse doTest(Conformance.ConformanceRequest re == Conformance.TestCategory.JSON_IGNORE_UNKNOWN_PARSING_TEST) { parser = parser.ignoringUnknownFields(); } - if (isProto3) { - TestMessagesProto3.TestAllTypesProto3.Builder builder = - TestMessagesProto3.TestAllTypesProto3.newBuilder(); - parser.merge(request.getJsonPayload(), builder); - testMessage = builder.build(); - } else if (isProto2) { - TestMessagesProto2.TestAllTypesProto2.Builder builder = - TestMessagesProto2.TestAllTypesProto2.newBuilder(); - parser.merge(request.getJsonPayload(), builder); - testMessage = builder.build(); - } else { - throw new IllegalArgumentException( - "Protobuf request has unexpected payload type: " + messageType); - } + AbstractMessage.Builder builder = + (AbstractMessage.Builder) + createTestMessage(messageType).getMethod("newBuilder").invoke(null); + parser.merge(request.getJsonPayload(), builder); + testMessage = (AbstractMessage) builder.build(); } catch (InvalidProtocolBufferException e) { return Conformance.ConformanceResponse.newBuilder() .setParseError(e.getMessage()) .build(); + } catch (Exception e) { + throw new RuntimeException(e); } break; } case TEXT_PAYLOAD: { - if (isProto3) { - try { - TestMessagesProto3.TestAllTypesProto3.Builder builder = - TestMessagesProto3.TestAllTypesProto3.newBuilder(); - TextFormat.merge(request.getTextPayload(), builder); - testMessage = builder.build(); - } catch (TextFormat.ParseException e) { - return Conformance.ConformanceResponse.newBuilder() - .setParseError(e.getMessage()) - .build(); - } - } else if (isProto2) { - try { - TestMessagesProto2.TestAllTypesProto2.Builder builder = - TestMessagesProto2.TestAllTypesProto2.newBuilder(); - TextFormat.merge(request.getTextPayload(), builder); - testMessage = builder.build(); + try { + AbstractMessage.Builder builder = + (AbstractMessage.Builder) + createTestMessage(messageType).getMethod("newBuilder").invoke(null); + TextFormat.merge(request.getTextPayload(), builder); + testMessage = (AbstractMessage) builder.build(); } catch (TextFormat.ParseException e) { return Conformance.ConformanceResponse.newBuilder() .setParseError(e.getMessage()) .build(); - } - } else { - throw new IllegalArgumentException( - "Protobuf request has unexpected payload type: " + messageType); + } catch (Exception e) { + throw new RuntimeException(e); } break; } @@ -378,6 +381,9 @@ public void run() throws Exception { typeRegistry = TypeRegistry.newBuilder() .add(TestMessagesProto3.TestAllTypesProto3.getDescriptor()) + .add( + com.google.protobuf_test_messages.editions.proto3.TestMessagesProto3Editions + .TestAllTypesProto3.getDescriptor()) .build(); while (doTestIo()) { this.testCount++; diff --git a/conformance/failure_list_java.txt b/conformance/failure_list_java.txt index 489fedce0f07b..cb2b5fdae816a 100644 --- a/conformance/failure_list_java.txt +++ b/conformance/failure_list_java.txt @@ -74,4 +74,75 @@ Required.Proto2.JsonInput.Int32FieldNegativeWithLeadingZero Required.Proto2.JsonInput.Int32FieldPlusSign Required.Proto2.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotBool Required.Proto2.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt -Required.Proto2.JsonInput.StringFieldNotAString \ No newline at end of file +Required.Proto2.JsonInput.StringFieldNotAString +Recommended.Editions_Proto3.FieldMaskNumbersDontRoundTrip.JsonOutput +Recommended.Editions_Proto3.FieldMaskPathsDontRoundTrip.JsonOutput +Recommended.Editions_Proto3.FieldMaskTooManyUnderscore.JsonOutput +Recommended.Editions_Proto3.JsonInput.BoolFieldAllCapitalFalse +Recommended.Editions_Proto3.JsonInput.BoolFieldAllCapitalTrue +Recommended.Editions_Proto3.JsonInput.BoolFieldCamelCaseFalse +Recommended.Editions_Proto3.JsonInput.BoolFieldCamelCaseTrue +Recommended.Editions_Proto3.JsonInput.BoolFieldDoubleQuotedFalse +Recommended.Editions_Proto3.JsonInput.BoolFieldDoubleQuotedTrue +Recommended.Editions_Proto3.JsonInput.BoolMapFieldKeyNotQuoted +Recommended.Editions_Proto3.JsonInput.DoubleFieldInfinityNotQuoted +Recommended.Editions_Proto3.JsonInput.DoubleFieldNanNotQuoted +Recommended.Editions_Proto3.JsonInput.DoubleFieldNegativeInfinityNotQuoted +Recommended.Editions_Proto3.JsonInput.FieldMaskInvalidCharacter +Recommended.Editions_Proto3.JsonInput.FieldNameDuplicate +Recommended.Editions_Proto3.JsonInput.FieldNameNotQuoted +Recommended.Editions_Proto3.JsonInput.FloatFieldInfinityNotQuoted +Recommended.Editions_Proto3.JsonInput.FloatFieldNanNotQuoted +Recommended.Editions_Proto3.JsonInput.FloatFieldNegativeInfinityNotQuoted +Recommended.Editions_Proto3.JsonInput.Int32MapFieldKeyNotQuoted +Recommended.Editions_Proto3.JsonInput.Int64MapFieldKeyNotQuoted +Recommended.Editions_Proto3.JsonInput.JsonWithComments +Recommended.Editions_Proto3.JsonInput.StringFieldSingleQuoteBoth +Recommended.Editions_Proto3.JsonInput.StringFieldSingleQuoteKey +Recommended.Editions_Proto3.JsonInput.StringFieldSingleQuoteValue +Recommended.Editions_Proto3.JsonInput.StringFieldSurrogateInWrongOrder +Recommended.Editions_Proto3.JsonInput.StringFieldUnpairedHighSurrogate +Recommended.Editions_Proto3.JsonInput.StringFieldUnpairedLowSurrogate +Recommended.Editions_Proto3.JsonInput.Uint32MapFieldKeyNotQuoted +Recommended.Editions_Proto3.JsonInput.Uint64MapFieldKeyNotQuoted +Recommended.Editions_Proto2.JsonInput.FieldNameExtension.Validator +Required.Editions_Proto3.JsonInput.EnumFieldNotQuoted +Required.Editions_Proto3.JsonInput.Int32FieldLeadingZero +Required.Editions_Proto3.JsonInput.Int32FieldNegativeWithLeadingZero +Required.Editions_Proto3.JsonInput.Int32FieldPlusSign +Required.Editions_Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotBool +Required.Editions_Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt +Required.Editions_Proto3.JsonInput.StringFieldNotAString +Recommended.Editions_Proto2.JsonInput.BoolFieldAllCapitalFalse +Recommended.Editions_Proto2.JsonInput.BoolFieldAllCapitalTrue +Recommended.Editions_Proto2.JsonInput.BoolFieldCamelCaseFalse +Recommended.Editions_Proto2.JsonInput.BoolFieldCamelCaseTrue +Recommended.Editions_Proto2.JsonInput.BoolFieldDoubleQuotedFalse +Recommended.Editions_Proto2.JsonInput.BoolFieldDoubleQuotedTrue +Recommended.Editions_Proto2.JsonInput.BoolMapFieldKeyNotQuoted +Recommended.Editions_Proto2.JsonInput.DoubleFieldInfinityNotQuoted +Recommended.Editions_Proto2.JsonInput.DoubleFieldNanNotQuoted +Recommended.Editions_Proto2.JsonInput.DoubleFieldNegativeInfinityNotQuoted +Recommended.Editions_Proto2.JsonInput.FieldNameDuplicate +Recommended.Editions_Proto2.JsonInput.FieldNameNotQuoted +Recommended.Editions_Proto2.JsonInput.FloatFieldInfinityNotQuoted +Recommended.Editions_Proto2.JsonInput.FloatFieldNanNotQuoted +Recommended.Editions_Proto2.JsonInput.FloatFieldNegativeInfinityNotQuoted +Recommended.Editions_Proto2.JsonInput.Int32MapFieldKeyNotQuoted +Recommended.Editions_Proto2.JsonInput.Int64MapFieldKeyNotQuoted +Recommended.Editions_Proto2.JsonInput.JsonWithComments +Recommended.Editions_Proto2.JsonInput.StringFieldSingleQuoteBoth +Recommended.Editions_Proto2.JsonInput.StringFieldSingleQuoteKey +Recommended.Editions_Proto2.JsonInput.StringFieldSingleQuoteValue +Recommended.Editions_Proto2.JsonInput.StringFieldSurrogateInWrongOrder +Recommended.Editions_Proto2.JsonInput.StringFieldUnpairedHighSurrogate +Recommended.Editions_Proto2.JsonInput.StringFieldUnpairedLowSurrogate +Recommended.Editions_Proto2.JsonInput.Uint32MapFieldKeyNotQuoted +Recommended.Editions_Proto2.JsonInput.Uint64MapFieldKeyNotQuoted +Required.Editions_Proto2.JsonInput.EnumFieldNotQuoted +Required.Editions_Proto2.JsonInput.Int32FieldLeadingZero +Required.Editions_Proto2.JsonInput.Int32FieldNegativeWithLeadingZero +Required.Editions_Proto2.JsonInput.Int32FieldPlusSign +Required.Editions_Proto2.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotBool +Required.Editions_Proto2.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt +Required.Editions_Proto2.JsonInput.StringFieldNotAString \ No newline at end of file diff --git a/conformance/text_format_failure_list_java.txt b/conformance/text_format_failure_list_java.txt index 793aae1281abe..8dea2862cdd4e 100644 --- a/conformance/text_format_failure_list_java.txt +++ b/conformance/text_format_failure_list_java.txt @@ -4,6 +4,14 @@ Recommended.Proto3.ProtobufInput.RepeatedUnknownFields_Drop.TextFormatOutput Recommended.Proto3.ProtobufInput.ScalarUnknownFields_Drop.TextFormatOutput Required.Proto3.TextFormatInput.AnyField.ProtobufOutput Required.Proto3.TextFormatInput.AnyField.TextFormatOutput +Recommended.Editions_Proto3.ProtobufInput.GroupUnknownFields_Drop.TextFormatOutput +Recommended.Editions_Proto3.ProtobufInput.MessageUnknownFields_Drop.TextFormatOutput +Recommended.Editions_Proto3.ProtobufInput.RepeatedUnknownFields_Drop.TextFormatOutput +Recommended.Editions_Proto3.ProtobufInput.ScalarUnknownFields_Drop.TextFormatOutput +Required.Editions_Proto3.TextFormatInput.AnyField.ProtobufOutput +Required.Editions_Proto3.TextFormatInput.AnyField.TextFormatOutput Required.Proto3.TextFormatInput.StringFieldBadUTF8Hex Required.Proto3.TextFormatInput.StringFieldBadUTF8Octal +Required.Editions_Proto3.TextFormatInput.StringFieldBadUTF8Hex +Required.Editions_Proto3.TextFormatInput.StringFieldBadUTF8Octal \ No newline at end of file diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel index af5691c0ffdb0..ace209190bbd6 100644 --- a/java/core/BUILD.bazel +++ b/java/core/BUILD.bazel @@ -7,6 +7,7 @@ load("//:protobuf_version.bzl", "PROTOBUF_JAVA_VERSION") load("//build_defs:java_opts.bzl", "protobuf_java_export", "protobuf_java_library", "protobuf_versioned_java_library") load("//conformance:defs.bzl", "conformance_test") load("//java/internal:testing.bzl", "junit_tests") +load("//src/google/protobuf/editions:defaults.bzl", "compile_edition_defaults", "embed_edition_defaults") LITE_SRCS = [ # Keep in sync with `//java/lite:pom.xml`. @@ -168,13 +169,21 @@ protobuf_java_library( proto_library( name = "java_features_proto", srcs = ["src/main/java/com/google/protobuf/java_features.proto"], + strip_import_prefix = "/java/core/src/main/java/com", visibility = ["//pkg:__pkg__"], deps = ["//:descriptor_proto"], ) +filegroup( + name = "java_features_proto_srcs", + srcs = ["src/main/java/com/google/protobuf/java_features.proto"], + visibility = ["//pkg:__pkg__"], +) + internal_gen_well_known_protos_java( name = "gen_well_known_protos_java", deps = [ + ":java_features_proto", "//:any_proto", "//:api_proto", "//:compiler_plugin_proto", @@ -240,6 +249,7 @@ protobuf_java_export( maven_coordinates = "com.google.protobuf:protobuf-java:%s" % PROTOBUF_JAVA_VERSION, pom_template = "pom_template.xml", resources = [ + ":java_features_proto_srcs", "//:well_known_type_protos", "//src/google/protobuf:descriptor_proto_srcs", ], @@ -266,6 +276,7 @@ proto_lang_toolchain( name = "toolchain", # keep this in sync w/ WELL_KNOWN_PROTO_MAP in //:BUILD blacklisted_protos = [ + ":java_features_proto", "//:any_proto", "//:api_proto", "//:compiler_plugin_proto", @@ -305,6 +316,14 @@ java_proto_library( deps = ["//src/google/protobuf:generic_test_protos"], ) +java_proto_library( + name = "generic_test_protos_editions_java_proto", + visibility = [ + "//java:__subpackages__", + ], + deps = ["//src/google/protobuf:generic_test_editions_protos"], +) + java_proto_library( name = "lite_test_protos_java_proto", visibility = [ @@ -355,6 +374,7 @@ build_test( conformance_test( name = "conformance_test", failure_list = "//conformance:failure_list_java.txt", + maximum_edition = "2023", testee = "//conformance:conformance_java", text_format_failure_list = "//conformance:text_format_failure_list_java.txt", ) @@ -374,6 +394,7 @@ junit_tests( data = ["//src/google/protobuf:testdata"], deps = [ ":core", + ":generic_test_protos_editions_java_proto", ":generic_test_protos_java_proto", ":java_test_protos_java_proto", ":lite_test_protos_java_proto", @@ -531,6 +552,7 @@ pkg_files( name = "dist_files", srcs = glob([ "src/main/java/com/google/protobuf/*.java", + "src/main/java/com/google/protobuf/*.proto", "src/test/java/**/*.java", "src/test/proto/**/*.proto", ]) + [ diff --git a/java/core/generate-sources-build.xml b/java/core/generate-sources-build.xml index 0996e5fff43c9..4ffd4384dc416 100644 --- a/java/core/generate-sources-build.xml +++ b/java/core/generate-sources-build.xml @@ -4,6 +4,8 @@ + + diff --git a/java/core/pom.xml b/java/core/pom.xml index c98d168792f99..b0a2bab5cc490 100644 --- a/java/core/pom.xml +++ b/java/core/pom.xml @@ -59,6 +59,12 @@ google/protobuf/compiler/plugin.proto + + ${protobuf.java_source.dir} + + main/java/com/google/protobuf/java_features.proto + + diff --git a/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java b/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java index 8578a7208301e..8ce15422c2a2d 100644 --- a/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java +++ b/java/core/src/main/java/com/google/protobuf/DescriptorMessageInfoFactory.java @@ -20,7 +20,6 @@ import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.FieldDescriptor.Type; -import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.OneofDescriptor; import java.lang.reflect.Field; import java.lang.reflect.Method; @@ -100,16 +99,14 @@ private static Descriptor descriptorForType(Class messageType) { return getDefaultInstance(messageType).getDescriptorForType(); } - private static ProtoSyntax convertSyntax(FileDescriptor.Syntax syntax) { - switch (syntax) { - case PROTO2: + private static ProtoSyntax convertSyntax(DescriptorProtos.Edition edition) { + switch (edition) { + case EDITION_PROTO2: return ProtoSyntax.PROTO2; - case PROTO3: + case EDITION_PROTO3: return ProtoSyntax.PROTO3; - case EDITIONS: - return ProtoSyntax.EDITIONS; default: - throw new IllegalArgumentException("Unsupported syntax: " + syntax); + return ProtoSyntax.EDITIONS; } } @@ -118,7 +115,7 @@ private static MessageInfo convert(Class messageType, Descriptor messageDescr StructuralMessageInfo.Builder builder = StructuralMessageInfo.newBuilder(fieldDescriptors.size()); builder.withDefaultInstance(getDefaultInstance(messageType)); - builder.withSyntax(convertSyntax(messageDescriptor.getFile().getSyntax())); + builder.withSyntax(convertSyntax(messageDescriptor.getFile().getEdition())); builder.withMessageSetWireFormat(messageDescriptor.getOptions().getMessageSetWireFormat()); OneofState oneofState = new OneofState(); diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java index 1a4380a13a588..d3c3c79731d0b 100644 --- a/java/core/src/main/java/com/google/protobuf/Descriptors.java +++ b/java/core/src/main/java/com/google/protobuf/Descriptors.java @@ -15,6 +15,9 @@ import com.google.protobuf.DescriptorProtos.EnumOptions; import com.google.protobuf.DescriptorProtos.EnumValueDescriptorProto; import com.google.protobuf.DescriptorProtos.EnumValueOptions; +import com.google.protobuf.DescriptorProtos.FeatureSet; +import com.google.protobuf.DescriptorProtos.FeatureSetDefaults; +import com.google.protobuf.DescriptorProtos.FeatureSetDefaults.FeatureSetEditionDefault; import com.google.protobuf.DescriptorProtos.FieldDescriptorProto; import com.google.protobuf.DescriptorProtos.FieldOptions; import com.google.protobuf.DescriptorProtos.FileDescriptorProto; @@ -26,7 +29,7 @@ import com.google.protobuf.DescriptorProtos.OneofOptions; import com.google.protobuf.DescriptorProtos.ServiceDescriptorProto; import com.google.protobuf.DescriptorProtos.ServiceOptions; -import com.google.protobuf.Descriptors.FileDescriptor.Syntax; +import com.google.protobuf.JavaFeaturesProto.JavaFeatures; import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.util.ArrayList; @@ -38,6 +41,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; /** @@ -65,6 +69,74 @@ public final class Descriptors { private static final EnumDescriptor[] EMPTY_ENUM_DESCRIPTORS = new EnumDescriptor[0]; private static final ServiceDescriptor[] EMPTY_SERVICE_DESCRIPTORS = new ServiceDescriptor[0]; private static final OneofDescriptor[] EMPTY_ONEOF_DESCRIPTORS = new OneofDescriptor[0]; + private static final ConcurrentHashMap FEATURE_CACHE = + new ConcurrentHashMap<>(); + + @SuppressWarnings("NonFinalStaticField") + private static volatile FeatureSetDefaults javaEditionDefaults = null; + + private static FeatureSet getEditionDefaults(Edition edition) { + // Force explicit initialization before synchronized block which can trigger initialization in + // `JavaFeaturesProto.registerAllExtensions()` and `FeatureSetdefaults.parseFrom()` calls. + // Otherwise, this can result in deadlock if another threads holds the static init block's + // implicit lock. This operation should be cheap if initialization has already occurred. + Descriptor unused1 = FeatureSetDefaults.getDescriptor(); + FileDescriptor unused2 = JavaFeaturesProto.getDescriptor(); + if (javaEditionDefaults == null) { + synchronized (Descriptors.class) { + if (javaEditionDefaults == null) { + try { + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + JavaFeaturesProto.registerAllExtensions(registry); + javaEditionDefaults = + FeatureSetDefaults.parseFrom( + JavaEditionDefaults.PROTOBUF_INTERNAL_JAVA_EDITION_DEFAULTS.getBytes( + Internal.ISO_8859_1), + registry); + } catch (Exception e) { + throw new AssertionError(e); + } + } + } + } + + if (edition.getNumber() < javaEditionDefaults.getMinimumEdition().getNumber()) { + throw new IllegalArgumentException( + "Edition " + + edition + + " is lower than the minimum supported edition " + + javaEditionDefaults.getMinimumEdition() + + "!"); + } + if (edition.getNumber() > javaEditionDefaults.getMaximumEdition().getNumber()) { + throw new IllegalArgumentException( + "Edition " + + edition + + " is greater than the maximum supported edition " + + javaEditionDefaults.getMaximumEdition() + + "!"); + } + FeatureSet found = null; + for (FeatureSetEditionDefault editionDefault : javaEditionDefaults.getDefaultsList()) { + if (editionDefault.getEdition().getNumber() > edition.getNumber()) { + break; + } + found = editionDefault.getFeatures(); + } + if (found == null) { + throw new IllegalArgumentException( + "Edition " + edition + " does not have a valid default FeatureSet!"); + } + return found; + } + + private static FeatureSet internFeatures(FeatureSet features) { + FeatureSet cached = FEATURE_CACHE.putIfAbsent(features.hashCode(), features); + if (cached == null) { + return features; + } + return cached; + } /** * Describes a {@code .proto} file, including everything defined within. That includes, in @@ -106,7 +178,21 @@ public String getPackage() { /** Get the {@code FileOptions}, defined in {@code descriptor.proto}. */ public FileOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + FileOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } /** Get a list of top-level message types declared in this file. */ @@ -139,47 +225,28 @@ public List getPublicDependencies() { return Collections.unmodifiableList(Arrays.asList(publicDependencies)); } - /** The syntax of the .proto file. */ - enum Syntax { - UNKNOWN("unknown"), - PROTO2("proto2"), - PROTO3("proto3"), - EDITIONS("editions"); - - Syntax(String name) { - this.name = name; - } - - private final String name; - } - - /** Get the syntax of the .proto file. */ - Syntax getSyntax() { - if (Syntax.PROTO3.name.equals(proto.getSyntax())) { - return Syntax.PROTO3; - } else if (Syntax.EDITIONS.name.equals(proto.getSyntax())) { - return Syntax.EDITIONS; - } - return Syntax.PROTO2; - } - /** Get the edition of the .proto file. */ Edition getEdition() { - return proto.getEdition(); + switch (proto.getSyntax()) { + case "editions": + return proto.getEdition(); + case "proto3": + return Edition.EDITION_PROTO3; + default: + return Edition.EDITION_PROTO2; + } } public void copyHeadingTo(FileDescriptorProto.Builder protoBuilder) { - protoBuilder.setName(getName()).setSyntax(getSyntax().name); + protoBuilder.setName(getName()).setSyntax(proto.getSyntax()); if (!getPackage().isEmpty()) { protoBuilder.setPackage(getPackage()); } - - if (getSyntax().equals(Syntax.EDITIONS)) { - protoBuilder.setEdition(getEdition()); + if (proto.getSyntax().equals("editions")) { + protoBuilder.setEdition(proto.getEdition()); } - - if (!getOptions().equals(FileOptions.getDefaultInstance())) { - protoBuilder.setOptions(getOptions()); + if (proto.hasOptions() && !proto.getOptions().equals(FileOptions.getDefaultInstance())) { + protoBuilder.setOptions(proto.getOptions()); } } @@ -306,6 +373,15 @@ public static FileDescriptor buildFrom(FileDescriptorProto proto, FileDescriptor public static FileDescriptor buildFrom( FileDescriptorProto proto, FileDescriptor[] dependencies, boolean allowUnknownDependencies) throws DescriptorValidationException { + return buildFrom(proto, dependencies, allowUnknownDependencies, false); + } + + private static FileDescriptor buildFrom( + FileDescriptorProto proto, + FileDescriptor[] dependencies, + boolean allowUnknownDependencies, + boolean allowUnresolvedFeatures) + throws DescriptorValidationException { // Building descriptors involves two steps: translating and linking. // In the translation step (implemented by FileDescriptor's // constructor), we build an object tree mirroring the @@ -319,6 +395,10 @@ public static FileDescriptor buildFrom( FileDescriptor result = new FileDescriptor(proto, dependencies, pool, allowUnknownDependencies); result.crossLink(); + // Skip feature resolution until later for calls from gencode. + if (!allowUnresolvedFeatures) { + result.resolveAllFeatures(); + } return result; } @@ -378,8 +458,8 @@ public static FileDescriptor internalBuildGeneratedFileFrom( try { // When building descriptors for generated code, we allow unknown - // dependencies by default. - return buildFrom(proto, dependencies, true); + // dependencies by default and delay feature resolution until later. + return buildFrom(proto, dependencies, true, true); } catch (DescriptorValidationException e) { throw new IllegalArgumentException( "Invalid embedded descriptor for \"" + proto.getName() + "\".", e); @@ -436,6 +516,7 @@ public interface InternalDescriptorAssigner { } private FileDescriptorProto proto; + private volatile FileOptions options; private final Descriptor[] messageTypes; private final EnumDescriptor[] enumTypes; private final ServiceDescriptor[] services; @@ -514,6 +595,7 @@ private FileDescriptor( /** Create a placeholder FileDescriptor for a message Descriptor. */ FileDescriptor(String packageName, Descriptor message) throws DescriptorValidationException { + this.parent = null; this.pool = new DescriptorPool(new FileDescriptor[0], true); this.proto = FileDescriptorProto.newBuilder() @@ -536,11 +618,67 @@ private FileDescriptor( /** * This method is to be called by generated code only. It resolves features for the descriptor * and all of its children. - * - *

TODO Implement and use this method in gencode once users using prebuilt jars - * with stale runtimes updated. */ - public void resolveAllFeatures() {} + public void resolveAllFeatures() { + if (this.features != null) { + return; + } + + synchronized (this) { + if (this.features != null) { + return; + } + this.features = resolveFeatures(proto.getOptions().getFeatures()); + + for (Descriptor messageType : messageTypes) { + messageType.resolveAllFeatures(); + } + + for (EnumDescriptor enumType : enumTypes) { + enumType.resolveAllFeatures(); + } + + for (ServiceDescriptor service : services) { + service.resolveAllFeatures(); + } + + for (FieldDescriptor extension : extensions) { + extension.resolveAllFeatures(); + } + } + } + + @Override + FeatureSet inferLegacyProtoFeatures() { + FeatureSet.Builder features = FeatureSet.newBuilder(); + if (getEdition().getNumber() >= Edition.EDITION_2023.getNumber()) { + return features.build(); + } + + if (getEdition() == Edition.EDITION_PROTO2) { + if (proto.getOptions().getJavaStringCheckUtf8()) { + features.setExtension( + JavaFeaturesProto.java, + JavaFeatures.newBuilder() + .setUtf8Validation(JavaFeatures.Utf8Validation.VERIFY) + .build()); + } + } + return features.build(); + } + + @Override + boolean hasInferredLegacyProtoFeatures() { + if (getEdition().getNumber() >= Edition.EDITION_2023.getNumber()) { + return false; + } + if (getEdition() == Edition.EDITION_PROTO2) { + if (proto.getOptions().getJavaStringCheckUtf8()) { + return true; + } + } + return false; + } /** Look up and cross-link all field types, etc. */ private void crossLink() throws DescriptorValidationException { @@ -567,6 +705,9 @@ private void crossLink() throws DescriptorValidationException { */ private void setProto(final FileDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); for (int i = 0; i < messageTypes.length; i++) { messageTypes[i].setProto(proto.getMessageType(i)); @@ -652,7 +793,21 @@ public Descriptor getContainingType() { /** Get the {@code MessageOptions}, defined in {@code descriptor.proto}. */ public MessageOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + MessageOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } /** Get a list of this message type's fields. */ @@ -787,6 +942,7 @@ public EnumDescriptor findEnumTypeByName(final String name) { private final int index; private DescriptorProto proto; + private volatile MessageOptions options; private final String fullName; private final FileDescriptor file; private final Descriptor containingType; @@ -830,6 +986,7 @@ public EnumDescriptor findEnumTypeByName(final String name) { // Create a placeholder FileDescriptor to hold this message. this.file = new FileDescriptor(packageName, this); + this.parent = this.file; extensionRangeLowerBounds = new int[] {1}; extensionRangeUpperBounds = new int[] {536870912}; @@ -841,6 +998,11 @@ private Descriptor( final Descriptor parent, final int index) throws DescriptorValidationException { + if (parent == null) { + this.parent = file; + } else { + this.parent = parent; + } this.index = index; this.proto = proto; fullName = computeFullName(file, parent, proto.getName()); @@ -934,6 +1096,31 @@ private Descriptor( } } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + + for (Descriptor nestedType : nestedTypes) { + nestedType.resolveAllFeatures(); + } + + for (EnumDescriptor enumType : enumTypes) { + enumType.resolveAllFeatures(); + } + + for (FieldDescriptor field : fields) { + field.resolveAllFeatures(); + } + + for (FieldDescriptor extension : extensions) { + extension.resolveAllFeatures(); + } + + for (OneofDescriptor oneof : oneofs) { + oneof.resolveAllFeatures(); + } + } + /** Look up and cross-link all field types, etc. */ private void crossLink() throws DescriptorValidationException { for (final Descriptor nestedType : nestedTypes) { @@ -972,6 +1159,9 @@ private void validateNoDuplicateFieldNumbers() throws DescriptorValidationExcept /** See {@link FileDescriptor#setProto}. */ private void setProto(final DescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); for (int i = 0; i < nestedTypes.length; i++) { nestedTypes[i].setProto(proto.getNestedType(i)); @@ -1079,6 +1269,14 @@ public FileDescriptor getFile() { /** Get the field's declared type. */ public Type getType() { + // Override delimited messages as legacy group type. Leaves unresolved messages as-is + // since these are used before feature resolution when parsing java feature set defaults + // (custom options) into unknown fields. + if (type == Type.MESSAGE + && this.features != null + && this.features.getMessageEncoding() == FeatureSet.MessageEncoding.DELIMITED) { + return Type.GROUP; + } return type; } @@ -1093,20 +1291,23 @@ public boolean needsUtf8Check() { if (type != Type.STRING) { return false; } - if (getContainingType().getOptions().getMapEntry()) { + if (getContainingType().toProto().getOptions().getMapEntry()) { // Always enforce strict UTF-8 checking for map fields. return true; } - if (getFile().getSyntax() == Syntax.PROTO3) { + if (this.features + .getExtension(JavaFeaturesProto.java) + .getUtf8Validation() + .equals(JavaFeatures.Utf8Validation.VERIFY)) { return true; } - return getFile().getOptions().getJavaStringCheckUtf8(); + return this.features.getUtf8Validation().equals(FeatureSet.Utf8Validation.VERIFY); } public boolean isMapField() { return getType() == Type.MESSAGE && isRepeated() - && getMessageType().getOptions().getMapEntry(); + && getMessageType().toProto().getOptions().getMapEntry(); } // I'm pretty sure values() constructs a new array every time, since there @@ -1116,7 +1317,8 @@ && isRepeated() /** Is this field declared required? */ public boolean isRequired() { - return proto.getLabel() == FieldDescriptorProto.Label.LABEL_REQUIRED; + return this.features.getFieldPresence() + == DescriptorProtos.FeatureSet.FieldPresence.LEGACY_REQUIRED; } /** Is this field declared optional? */ @@ -1139,11 +1341,9 @@ public boolean isPacked() { if (!isPackable()) { return false; } - if (getFile().getSyntax() == FileDescriptor.Syntax.PROTO2) { - return getOptions().getPacked(); - } else { - return !getOptions().hasPacked() || getOptions().getPacked(); - } + return this.features + .getRepeatedFieldEncoding() + .equals(FeatureSet.RepeatedFieldEncoding.PACKED); } /** Can this field be packed? That is, is it a repeated primitive field? */ @@ -1171,7 +1371,21 @@ public Object getDefaultValue() { /** Get the {@code FieldOptions}, defined in {@code descriptor.proto}. */ public FieldOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + FieldOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } /** Is this field an extension? */ @@ -1203,7 +1417,9 @@ public OneofDescriptor getRealContainingOneof() { */ boolean hasOptionalKeyword() { return isProto3Optional - || (file.getSyntax() == Syntax.PROTO2 && isOptional() && getContainingOneof() == null); + || (file.getEdition() == Edition.EDITION_PROTO2 + && isOptional() + && getContainingOneof() == null); } /** @@ -1223,7 +1439,7 @@ public boolean hasPresence() { return getType() == Type.MESSAGE || getType() == Type.GROUP || getContainingOneof() != null - || file.getSyntax() == Syntax.PROTO2; + || this.features.getFieldPresence() != DescriptorProtos.FeatureSet.FieldPresence.IMPLICIT; } /** @@ -1295,7 +1511,17 @@ public EnumDescriptor getEnumType() { * handling quirks. */ public boolean legacyEnumFieldTreatedAsClosed() { - return getType() == Type.ENUM && getFile().getSyntax() == Syntax.PROTO2; + // Don't check JavaFeaturesProto extension for files without dependencies. + // This is especially important for descriptor.proto since getting the JavaFeaturesProto + // extension itself involves calling legacyEnumFieldTreatedAsClosed() which would otherwise + // infinite loop. + if (getFile().getDependencies().isEmpty()) { + return getType() == Type.ENUM && enumType.isClosed(); + } + + return getType() == Type.ENUM + && (this.features.getExtension(JavaFeaturesProto.java).getLegacyClosedEnum() + || enumType.isClosed()); } /** @@ -1324,6 +1550,7 @@ public String toString() { private final int index; private FieldDescriptorProto proto; + private volatile FieldOptions options; private final String fullName; private String jsonName; private final FileDescriptor file; @@ -1442,6 +1669,7 @@ private FieldDescriptor( final int index, final boolean isExtension) throws DescriptorValidationException { + this.parent = parent; this.index = index; this.proto = proto; fullName = computeFullName(file, parent, proto.getName()); @@ -1499,6 +1727,66 @@ private FieldDescriptor( file.pool.addSymbol(this); } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + } + + @Override + FeatureSet inferLegacyProtoFeatures() { + FeatureSet.Builder features = FeatureSet.newBuilder(); + if (getFile().getEdition().getNumber() >= Edition.EDITION_2023.getNumber()) { + return features.build(); + } + + if (proto.getLabel() == FieldDescriptorProto.Label.LABEL_REQUIRED) { + features.setFieldPresence(FeatureSet.FieldPresence.LEGACY_REQUIRED); + } + + if (proto.getType() == FieldDescriptorProto.Type.TYPE_GROUP) { + features.setMessageEncoding(FeatureSet.MessageEncoding.DELIMITED); + } + + if (getFile().getEdition() == Edition.EDITION_PROTO2 && proto.getOptions().getPacked()) { + features.setRepeatedFieldEncoding(FeatureSet.RepeatedFieldEncoding.PACKED); + } + + if (getFile().getEdition() == Edition.EDITION_PROTO3) { + if (proto.getOptions().hasPacked() && !proto.getOptions().getPacked()) { + features.setRepeatedFieldEncoding(FeatureSet.RepeatedFieldEncoding.EXPANDED); + } + + } + return features.build(); + } + + @Override + boolean hasInferredLegacyProtoFeatures() { + if (getFile().getEdition().getNumber() >= Edition.EDITION_2023.getNumber()) { + return false; + } + + if (proto.getLabel() == FieldDescriptorProto.Label.LABEL_REQUIRED) { + return true; + } + + if (proto.getType() == FieldDescriptorProto.Type.TYPE_GROUP) { + return true; + } + + if (proto.getOptions().getPacked()) { + return true; + } + + if (getFile().getEdition() == Edition.EDITION_PROTO3) { + if (proto.getOptions().hasPacked() && !proto.getOptions().getPacked()) { + return true; + } + + } + return false; + } + /** Look up and cross-link all field types, etc. */ private void crossLink() throws DescriptorValidationException { if (proto.hasExtendee()) { @@ -1671,7 +1959,8 @@ private void crossLink() throws DescriptorValidationException { } } - if (containingType != null && containingType.getOptions().getMessageSetWireFormat()) { + if (containingType != null + && containingType.toProto().getOptions().getMessageSetWireFormat()) { if (isExtension()) { if (!isOptional() || getType() != Type.MESSAGE) { throw new DescriptorValidationException( @@ -1687,6 +1976,9 @@ private void crossLink() throws DescriptorValidationException { /** See {@link FileDescriptor#setProto}. */ private void setProto(final FieldDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); } /** For internal use only. This is to satisfy the FieldDescriptorLite interface. */ @@ -1765,7 +2057,7 @@ public FileDescriptor getFile() { * handling quirks. */ public boolean isClosed() { - return getFile().getSyntax() != Syntax.PROTO3; + return this.features.getEnumType() == DescriptorProtos.FeatureSet.EnumType.CLOSED; } /** If this is a nested type, get the outer descriptor, otherwise null. */ @@ -1775,7 +2067,21 @@ public Descriptor getContainingType() { /** Get the {@code EnumOptions}, defined in {@code descriptor.proto}. */ public EnumOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + EnumOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } /** Get a list of defined values for this enum. */ @@ -1886,6 +2192,7 @@ int getUnknownEnumValueDescriptorCount() { private final int index; private EnumDescriptorProto proto; + private volatile EnumOptions options; private final String fullName; private final FileDescriptor file; private final Descriptor containingType; @@ -1901,6 +2208,11 @@ private EnumDescriptor( final Descriptor parent, final int index) throws DescriptorValidationException { + if (parent == null) { + this.parent = file; + } else { + this.parent = parent; + } this.index = index; this.proto = proto; fullName = computeFullName(file, parent, proto.getName()); @@ -1934,9 +2246,20 @@ private EnumDescriptor( file.pool.addSymbol(this); } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + for (EnumValueDescriptor value : values) { + value.resolveAllFeatures(); + } + } + /** See {@link FileDescriptor#setProto}. */ private void setProto(final EnumDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); for (int i = 0; i < values.length; i++) { values[i].setProto(proto.getValue(i)); @@ -2025,11 +2348,26 @@ public EnumDescriptor getType() { /** Get the {@code EnumValueOptions}, defined in {@code descriptor.proto}. */ public EnumValueOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + EnumValueOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } private final int index; private EnumValueDescriptorProto proto; + private volatile EnumValueOptions options; private final String fullName; private final EnumDescriptor type; @@ -2039,12 +2377,11 @@ private EnumValueDescriptor( final EnumDescriptor parent, final int index) throws DescriptorValidationException { + this.parent = parent; this.index = index; this.proto = proto; - type = parent; - - fullName = parent.getFullName() + '.' + proto.getName(); - + this.type = parent; + this.fullName = parent.getFullName() + '.' + proto.getName(); file.pool.addSymbol(this); } @@ -2053,6 +2390,7 @@ private EnumValueDescriptor(final EnumDescriptor parent, final Integer number) { String name = "UNKNOWN_ENUM_VALUE_" + parent.getName() + "_" + number; EnumValueDescriptorProto proto = EnumValueDescriptorProto.newBuilder().setName(name).setNumber(number).build(); + this.parent = parent; this.index = -1; this.proto = proto; this.type = parent; @@ -2061,9 +2399,17 @@ private EnumValueDescriptor(final EnumDescriptor parent, final Integer number) { // Don't add this descriptor into pool. } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + } + /** See {@link FileDescriptor#setProto}. */ private void setProto(final EnumValueDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); } } @@ -2108,7 +2454,21 @@ public FileDescriptor getFile() { /** Get the {@code ServiceOptions}, defined in {@code descriptor.proto}. */ public ServiceOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + ServiceOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } /** Get a list of methods for this service. */ @@ -2133,6 +2493,7 @@ public MethodDescriptor findMethodByName(final String name) { private final int index; private ServiceDescriptorProto proto; + private volatile ServiceOptions options; private final String fullName; private final FileDescriptor file; private MethodDescriptor[] methods; @@ -2140,6 +2501,7 @@ public MethodDescriptor findMethodByName(final String name) { private ServiceDescriptor( final ServiceDescriptorProto proto, final FileDescriptor file, final int index) throws DescriptorValidationException { + this.parent = file; this.index = index; this.proto = proto; fullName = computeFullName(file, null, proto.getName()); @@ -2153,6 +2515,15 @@ private ServiceDescriptor( file.pool.addSymbol(this); } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + + for (MethodDescriptor method : methods) { + method.resolveAllFeatures(); + } + } + private void crossLink() throws DescriptorValidationException { for (final MethodDescriptor method : methods) { method.crossLink(); @@ -2162,6 +2533,9 @@ private void crossLink() throws DescriptorValidationException { /** See {@link FileDescriptor#setProto}. */ private void setProto(final ServiceDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); for (int i = 0; i < methods.length; i++) { methods[i].setProto(proto.getMethod(i)); @@ -2235,11 +2609,26 @@ public boolean isServerStreaming() { /** Get the {@code MethodOptions}, defined in {@code descriptor.proto}. */ public MethodOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + MethodOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } private final int index; private MethodDescriptorProto proto; + private volatile MethodOptions options; private final String fullName; private final FileDescriptor file; private final ServiceDescriptor service; @@ -2254,6 +2643,7 @@ private MethodDescriptor( final ServiceDescriptor parent, final int index) throws DescriptorValidationException { + this.parent = parent; this.index = index; this.proto = proto; this.file = file; @@ -2264,6 +2654,11 @@ private MethodDescriptor( file.pool.addSymbol(this); } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + } + private void crossLink() throws DescriptorValidationException { final GenericDescriptor input = getFile() @@ -2289,6 +2684,9 @@ private void crossLink() throws DescriptorValidationException { /** See {@link FileDescriptor#setProto}. */ private void setProto(final MethodDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); } } @@ -2315,7 +2713,6 @@ private static String computeFullName( * DescriptorPool}. */ public abstract static class GenericDescriptor { - // Private constructor to prevent subclasses outside of com.google.protobuf.Descriptors private GenericDescriptor() {} @@ -2326,6 +2723,35 @@ private GenericDescriptor() {} public abstract String getFullName(); public abstract FileDescriptor getFile(); + + FeatureSet resolveFeatures(FeatureSet unresolvedFeatures) { + if (this.parent != null + && unresolvedFeatures.equals(FeatureSet.getDefaultInstance()) + && !hasInferredLegacyProtoFeatures()) { + return this.parent.features; + } + FeatureSet.Builder features; + if (this.parent == null) { + Edition edition = getFile().getEdition(); + features = getEditionDefaults(edition).toBuilder(); + } else { + features = this.parent.features.toBuilder(); + } + features.mergeFrom(inferLegacyProtoFeatures()); + features.mergeFrom(unresolvedFeatures); + return internFeatures(features.build()); + } + + FeatureSet inferLegacyProtoFeatures() { + return FeatureSet.getDefaultInstance(); + } + + boolean hasInferredLegacyProtoFeatures() { + return false; + } + + GenericDescriptor parent; + FeatureSet features; } /** Thrown when building descriptors fails because the source DescriptorProtos are not valid. */ @@ -2753,7 +3179,21 @@ public int getFieldCount() { } public OneofOptions getOptions() { - return proto.getOptions(); + if (this.options == null) { + OneofOptions strippedOptions = this.proto.getOptions(); + if (strippedOptions.hasFeatures()) { + // Clients should be using feature accessor methods, not accessing features on the + // options + // proto. + strippedOptions = strippedOptions.toBuilder().clearFeatures().build(); + } + synchronized (this) { + if (this.options == null) { + this.options = strippedOptions; + } + } + } + return this.options; } /** Get a list of this message type's fields. */ @@ -2774,8 +3214,16 @@ boolean isSynthetic() { return fields.length == 1 && fields[0].isProto3Optional; } + /** See {@link FileDescriptor#resolveAllFeatures}. */ + private void resolveAllFeatures() { + this.features = resolveFeatures(proto.getOptions().getFeatures()); + } + private void setProto(final OneofDescriptorProto proto) { this.proto = proto; + this.features = null; + this.options = null; + this.features = resolveFeatures(proto.getOptions().getFeatures()); } private OneofDescriptor( @@ -2783,6 +3231,7 @@ private OneofDescriptor( final FileDescriptor file, final Descriptor parent, final int index) { + this.parent = parent; this.proto = proto; fullName = computeFullName(file, parent, proto.getName()); this.file = file; @@ -2794,6 +3243,7 @@ private OneofDescriptor( private final int index; private OneofDescriptorProto proto; + private volatile OneofOptions options; private final String fullName; private final FileDescriptor file; diff --git a/java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java b/java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java new file mode 100644 index 0000000000000..1cd713ba78cf9 --- /dev/null +++ b/java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java @@ -0,0 +1,10 @@ +// This file contains the serialized FeatureSetDefaults object corresponding to +// the Java runtime. This is used for feature resolution under Editions. + +package com.google.protobuf; + +public final class JavaEditionDefaults { + public static final String PROTOBUF_INTERNAL_JAVA_EDITION_DEFAULTS = "\n\030\022\023\010\001\020\002\030\002 \003(\0010\002\312>\004\010\001\020\001\030\346\007\n\030\022\023\010\002\020\001\030\001 \002(\0010\001\312>\004\010\000\020\001\030\347\007\n\030\022\023\010\001\020\001\030\001 \002(\0010\001\312>\004\010\000\020\001\030\350\007 \346\007(\350\007"; + + private JavaEditionDefaults() {} +} diff --git a/java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java.template b/java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java.template new file mode 100644 index 0000000000000..bd71f453ef3cd --- /dev/null +++ b/java/core/src/main/java/com/google/protobuf/JavaEditionDefaults.java.template @@ -0,0 +1,9 @@ +// This file contains the serialized FeatureSetDefaults object corresponding to +// the Java runtime. This is used for feature resolution under Editions. + +package com.google.protobuf; + +public final class JavaEditionDefaults { + public static final String PROTOBUF_INTERNAL_JAVA_EDITION_DEFAULTS = "DEFAULTS_VALUE"; +} + diff --git a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java index aae4a4dd13889..3f0e22edaf29a 100644 --- a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java +++ b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java @@ -9,6 +9,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertThrows; import com.google.protobuf.DescriptorProtos.DescriptorProto; import com.google.protobuf.DescriptorProtos.Edition; @@ -17,6 +18,9 @@ import com.google.protobuf.DescriptorProtos.FieldDescriptorProto; import com.google.protobuf.DescriptorProtos.FileDescriptorProto; import com.google.protobuf.DescriptorProtos.FileOptions; +import com.google.protobuf.DescriptorProtos.MethodDescriptorProto; +import com.google.protobuf.DescriptorProtos.OneofDescriptorProto; +import com.google.protobuf.DescriptorProtos.ServiceDescriptorProto; import com.google.protobuf.Descriptors.Descriptor; import com.google.protobuf.Descriptors.DescriptorValidationException; import com.google.protobuf.Descriptors.EnumDescriptor; @@ -29,6 +33,8 @@ import com.google.protobuf.test.UnittestImport; import com.google.protobuf.test.UnittestImport.ImportEnum; import com.google.protobuf.test.UnittestImport.ImportEnumForMap; +import legacy_features_unittest.UnittestLegacyFeatures; +import pb.UnittestFeatures; import protobuf_unittest.TestCustomOptions; import protobuf_unittest.UnittestCustomOptions; import protobuf_unittest.UnittestProto; @@ -44,8 +50,10 @@ import protobuf_unittest.UnittestProto.TestReservedFields; import protobuf_unittest.UnittestProto.TestService; import protobuf_unittest.UnittestRetention; +import proto3_unittest.UnittestProto3; import java.util.Collections; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -77,7 +85,6 @@ public void testFileDescriptor() throws Exception { FileDescriptor file = UnittestProto.getDescriptor(); assertThat(file.getName()).isEqualTo("google/protobuf/unittest.proto"); - assertThat(file.getSyntax()).isEqualTo(Descriptors.FileDescriptor.Syntax.PROTO2); assertThat(file.getPackage()).isEqualTo("protobuf_unittest"); assertThat(file.getOptions().getJavaOuterClassname()).isEqualTo("UnittestProto"); assertThat(file.toProto().getName()).isEqualTo("google/protobuf/unittest.proto"); @@ -127,14 +134,33 @@ public void testFileDescriptor() throws Exception { } @Test - public void testFileDescriptorGetSyntax() throws Exception { + public void testFileDescriptorGetEdition() throws Exception { FileDescriptorProto proto2 = FileDescriptorProto.newBuilder().setSyntax("proto2").build(); FileDescriptor file2 = Descriptors.FileDescriptor.buildFrom(proto2, new FileDescriptor[0]); - assertThat(file2.getSyntax()).isEqualTo(Descriptors.FileDescriptor.Syntax.PROTO2); + assertThat(file2.getEdition()).isEqualTo(Edition.EDITION_PROTO2); FileDescriptorProto proto3 = FileDescriptorProto.newBuilder().setSyntax("proto3").build(); FileDescriptor file3 = Descriptors.FileDescriptor.buildFrom(proto3, new FileDescriptor[0]); - assertThat(file3.getSyntax()).isEqualTo(Descriptors.FileDescriptor.Syntax.PROTO3); + assertThat(file3.getEdition()).isEqualTo(Edition.EDITION_PROTO3); + + FileDescriptorProto protoEdition = + FileDescriptorProto.newBuilder() + .setSyntax("editions") + .setEdition(Edition.EDITION_2023) + .build(); + FileDescriptor fileEdition = + Descriptors.FileDescriptor.buildFrom(protoEdition, new FileDescriptor[0]); + assertThat(fileEdition.getEdition()).isEqualTo(Edition.EDITION_2023); + + FileDescriptorProto protoMissingEdition = + FileDescriptorProto.newBuilder().setSyntax("editions").build(); + IllegalArgumentException exception = + assertThrows( + IllegalArgumentException.class, + () -> Descriptors.FileDescriptor.buildFrom(protoMissingEdition, new FileDescriptor[0])); + assertThat(exception) + .hasMessageThat() + .contains("Edition EDITION_UNKNOWN is lower than the minimum supported edition"); } @Test @@ -1044,4 +1070,442 @@ public void testExtensionRenamesKeywords() { public void testDefaultDescriptorExtensionRange() throws Exception { assertThat(new Descriptor("default").isExtensionNumber(1)).isTrue(); } + + @Test + public void testGetOptionsStripsFeatures() { + FieldDescriptor field = + UnittestLegacyFeatures.TestEditionsMessage.getDescriptor() + .findFieldByName("required_field"); + assertThat(field.getOptions().hasFeatures()).isFalse(); + } + + @Test + public void testLegacyRequiredTransform() { + Descriptor descriptor = UnittestLegacyFeatures.TestEditionsMessage.getDescriptor(); + assertThat(descriptor.findFieldByName("required_field").isRequired()).isTrue(); + } + + @Test + public void testLegacyGroupTransform() { + Descriptor descriptor = UnittestLegacyFeatures.TestEditionsMessage.getDescriptor(); + assertThat(descriptor.findFieldByName("delimited_field").getType()) + .isEqualTo(FieldDescriptor.Type.GROUP); + } + + @Test + public void testLegacyInferRequired() { + FieldDescriptor field = UnittestProto.TestRequired.getDescriptor().findFieldByName("a"); + assertThat(field.features.getFieldPresence()) + .isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.LEGACY_REQUIRED); + } + + @Test + public void testLegacyInferGroup() { + FieldDescriptor field = + UnittestProto.TestAllTypes.getDescriptor().findFieldByName("optionalgroup"); + assertThat(field.features.getMessageEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.MessageEncoding.DELIMITED); + } + + @Test + public void testLegacyInferProto2Packed() { + FieldDescriptor field = + UnittestProto.TestPackedTypes.getDescriptor().findFieldByName("packed_int32"); + assertThat(field.features.getRepeatedFieldEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.PACKED); + } + + @Test + public void testLegacyInferProto3Expanded() { + FieldDescriptor field = + UnittestProto3.TestUnpackedTypes.getDescriptor().findFieldByName("repeated_int32"); + assertThat(field.features.getRepeatedFieldEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.EXPANDED); + } + + @Test + public void testLegacyInferProto2Utf8Validation() throws Exception { + FileDescriptor file = + FileDescriptor.buildFrom( + FileDescriptorProto.newBuilder() + .setName("some/filename/some.proto") + .setPackage("protobuf_unittest") + .setSyntax("proto2") + .setOptions(FileOptions.newBuilder().setJavaStringCheckUtf8(true)) + .build(), + new FileDescriptor[0]); + assertThat(file.features.getExtension(JavaFeaturesProto.java).getUtf8Validation()) + .isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.VERIFY); + } + + @Test + public void testProto2Defaults() { + FieldDescriptor proto2Field = TestAllTypes.getDescriptor().findFieldByName("optional_int32"); + DescriptorProtos.FeatureSet features = proto2Field.features; + assertThat(features.getFieldPresence()) + .isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.EXPLICIT); + assertThat(features.getEnumType()).isEqualTo(DescriptorProtos.FeatureSet.EnumType.CLOSED); + assertThat(features.getRepeatedFieldEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.EXPANDED); + assertThat(features.getUtf8Validation()) + .isEqualTo(DescriptorProtos.FeatureSet.Utf8Validation.NONE); + assertThat(features.getMessageEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.MessageEncoding.LENGTH_PREFIXED); + assertThat(features.getJsonFormat()) + .isEqualTo(DescriptorProtos.FeatureSet.JsonFormat.LEGACY_BEST_EFFORT); + + assertThat(features.getExtension(JavaFeaturesProto.java).getLegacyClosedEnum()).isTrue(); + assertThat(features.getExtension(JavaFeaturesProto.java).getUtf8Validation()) + .isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.DEFAULT); + } + + @Test + public void testProto3Defaults() { + FieldDescriptor proto3Field = + UnittestProto3.TestAllTypes.getDescriptor().findFieldByName("optional_int32"); + DescriptorProtos.FeatureSet features = proto3Field.features; + assertThat(features.getFieldPresence()) + .isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.IMPLICIT); + assertThat(features.getEnumType()).isEqualTo(DescriptorProtos.FeatureSet.EnumType.OPEN); + assertThat(features.getRepeatedFieldEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.PACKED); + assertThat(features.getUtf8Validation()) + .isEqualTo(DescriptorProtos.FeatureSet.Utf8Validation.VERIFY); + assertThat(features.getMessageEncoding()) + .isEqualTo(DescriptorProtos.FeatureSet.MessageEncoding.LENGTH_PREFIXED); + + assertThat(features.getExtension(JavaFeaturesProto.java).getLegacyClosedEnum()).isFalse(); + assertThat(features.getExtension(JavaFeaturesProto.java).getUtf8Validation()) + .isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.DEFAULT); + } + + @RunWith(JUnit4.class) + public static class FeatureInheritanceTest { + FileDescriptorProto.Builder fileProto; + FieldDescriptorProto.Builder topExtensionProto; + EnumDescriptorProto.Builder topEnumProto; + EnumValueDescriptorProto.Builder enumValueProto; + DescriptorProto.Builder topMessageProto; + FieldDescriptorProto.Builder fieldProto; + FieldDescriptorProto.Builder nestedExtensionProto; + DescriptorProto.Builder nestedMessageProto; + EnumDescriptorProto.Builder nestedEnumProto; + OneofDescriptorProto.Builder oneofProto; + FieldDescriptorProto.Builder oneofFieldProto; + ServiceDescriptorProto.Builder serviceProto; + MethodDescriptorProto.Builder methodProto; + + @Before + public void setUp() { + this.fileProto = + DescriptorProtos.FileDescriptorProto.newBuilder() + .setName("some/filename/some.proto") + .setPackage("protobuf_unittest") + .setEdition(DescriptorProtos.Edition.EDITION_2023) + .setSyntax("editions"); + + this.topExtensionProto = + FieldDescriptorProto.newBuilder() + .setName("top_extension") + .setNumber(10) + .setType(FieldDescriptorProto.Type.TYPE_INT32) + .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) + .setExtendee(".protobuf_unittest.TopMessage"); + this.fileProto.addExtension(topExtensionProto); + + this.topEnumProto = EnumDescriptorProto.newBuilder().setName("TopEnum"); + this.enumValueProto = EnumValueDescriptorProto.newBuilder().setName("TOP_VALUE").setNumber(0); + this.topEnumProto.addValue(enumValueProto); + this.fileProto.addEnumType(topEnumProto); + + this.topMessageProto = DescriptorProto.newBuilder().setName("TopMessage"); + this.fieldProto = + FieldDescriptorProto.newBuilder() + .setName("field") + .setNumber(1) + .setType(FieldDescriptorProto.Type.TYPE_INT32) + .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL); + this.topMessageProto.addField(fieldProto); + this.nestedExtensionProto = + FieldDescriptorProto.newBuilder() + .setName("nested_extension") + .setNumber(11) + .setType(FieldDescriptorProto.Type.TYPE_INT32) + .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) + .setExtendee(".protobuf_unittest.TopMessage"); + this.topMessageProto.addExtension(nestedExtensionProto); + this.nestedMessageProto = DescriptorProto.newBuilder().setName("NestedMessage"); + this.topMessageProto.addNestedType(nestedMessageProto); + this.nestedEnumProto = EnumDescriptorProto.newBuilder().setName("NestedEnum"); + this.nestedEnumProto.addValue( + EnumValueDescriptorProto.newBuilder().setName("NESTED_VALUE").setNumber(0)); + this.topMessageProto.addEnumType(nestedEnumProto); + this.topMessageProto.addOneofDecl(OneofDescriptorProto.newBuilder().setName("Oneof")); + this.topMessageProto.addField( + FieldDescriptorProto.newBuilder() + .setName("oneof_field") + .setNumber(2) + .setType(FieldDescriptorProto.Type.TYPE_INT32) + .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) + .setOneofIndex(0)); + this.fileProto.addMessageType(topMessageProto); + + this.serviceProto = ServiceDescriptorProto.newBuilder().setName("TestService"); + this.methodProto = + MethodDescriptorProto.newBuilder() + .setName("CallMethod") + .setInputType(".protobuf_unittest.TopMessage") + .setOutputType(".protobuf_unittest.TopMessage"); + this.serviceProto.addMethod(methodProto); + this.fileProto.addService(serviceProto); + } + + void setTestFeature(DescriptorProtos.FeatureSet.Builder features, int value) { + features.setExtension( + UnittestFeatures.test, + features.getExtension(UnittestFeatures.test).toBuilder() + .setIntMultipleFeature(value) + .build()); + } + + int getTestFeature(DescriptorProtos.FeatureSet features) { + return features.getExtension(UnittestFeatures.test).getIntMultipleFeature(); + } + + FileDescriptor buildFrom(FileDescriptorProto fileProto) throws Exception { + return FileDescriptor.buildFrom(fileProto, new FileDescriptor[0]); + } + + @Test + public void testFileDefaults() throws Exception { + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.features)).isEqualTo(1); + } + + @Test + public void testFileOverrides() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.features)).isEqualTo(3); + } + + @Test + public void testFileMessageInherit() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).features)).isEqualTo(3); + } + + @Test + public void testFileMessageOverride() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).features)).isEqualTo(5); + } + + @Test + public void testFileEnumInherit() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + + assertThat(getTestFeature(descriptor.getEnumTypes().get(0).features)).isEqualTo(3); + } + + @Test + public void testFileEnumOverride() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(topEnumProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getEnumTypes().get(0).features)).isEqualTo(5); + } + + @Test + public void testFileExtensionInherit() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getExtensions().get(0).features)).isEqualTo(3); + } + + @Test + public void testFileExtensionOverride() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(topExtensionProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getExtensions().get(0).features)).isEqualTo(5); + } + + @Test + public void testFileServiceInherit() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getServices().get(0).features)).isEqualTo(3); + } + + @Test + public void testFileServiceOverride() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(serviceProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getServices().get(0).features)).isEqualTo(5); + } + + @Test + public void testMessageFieldInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getFields().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testMessageFieldOverride() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(fieldProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getFields().get(0).features)) + .isEqualTo(5); + } + + @Test + public void testMessageEnumInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getEnumTypes().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testMessageEnumOverride() throws Exception { + setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(nestedEnumProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getEnumTypes().get(0).features)) + .isEqualTo(5); + } + + @Test + public void testMessageMessageInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat( + getTestFeature(descriptor.getMessageTypes().get(0).getNestedTypes().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testMessageMessageOverride() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(nestedMessageProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat( + getTestFeature(descriptor.getMessageTypes().get(0).getNestedTypes().get(0).features)) + .isEqualTo(5); + } + + @Test + public void testMessageExtensionInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat( + getTestFeature(descriptor.getMessageTypes().get(0).getExtensions().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testMessageExtensionOverride() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(nestedExtensionProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat( + getTestFeature(descriptor.getMessageTypes().get(0).getExtensions().get(0).features)) + .isEqualTo(5); + } + + @Test + public void testMessageOneofInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getOneofs().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testMessageOneofOverride() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(oneofProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getOneofs().get(0).features)) + .isEqualTo(5); + } + + @Test + public void testOneofFieldInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat( + getTestFeature( + descriptor + .getMessageTypes() + .get(0) + .getOneofs() + .get(0) + .getFields() + .get(0) + .features)) + .isEqualTo(3); + } + + @Test + public void testOneofFieldOverride() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(oneofProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat( + getTestFeature( + descriptor + .getMessageTypes() + .get(0) + .getOneofs() + .get(0) + .getFields() + .get(0) + .features)) + .isEqualTo(5); + } + + @Test + public void testEnumValueInherit() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getEnumTypes().get(0).getValues().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testEnumValueOverride() throws Exception { + setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(enumValueProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getEnumTypes().get(0).getValues().get(0).features)) + .isEqualTo(5); + } + + @Test + public void testServiceMethodInherit() throws Exception { + setTestFeature(serviceProto.getOptionsBuilder().getFeaturesBuilder(), 3); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getServices().get(0).getMethods().get(0).features)) + .isEqualTo(3); + } + + @Test + public void testServiceMethodOverride() throws Exception { + setTestFeature(serviceProto.getOptionsBuilder().getFeaturesBuilder(), 3); + setTestFeature(methodProto.getOptionsBuilder().getFeaturesBuilder(), 5); + FileDescriptor descriptor = buildFrom(fileProto.build()); + assertThat(getTestFeature(descriptor.getServices().get(0).getMethods().get(0).features)) + .isEqualTo(5); + } + } } diff --git a/java/pom.xml b/java/pom.xml index 9e0a6992c01fa..89b155d989a00 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -33,6 +33,7 @@ ${project.basedir}/../.. ${protobuf.basedir}/src + ${protobuf.basedir}/java/core/src ${protobuf.basedir}/protoc src/test/proto ${project.build.directory}/generated-sources diff --git a/python/build_targets.bzl b/python/build_targets.bzl index 5f4ae756b342f..a1f1767fefb79 100644 --- a/python/build_targets.bzl +++ b/python/build_targets.bzl @@ -221,6 +221,7 @@ def build_targets(name): testonly = 1, srcs = [ "//:test_proto_srcs", + "//:test_proto_editions_srcs", "//src/google/protobuf/util:test_proto_srcs", ], strip_prefix = "src", diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index c561a0411c2ac..56677d34a39e4 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -18,11 +18,11 @@ from google.protobuf import symbol_database from google.protobuf import text_format from google.protobuf.internal import api_implementation -from google.protobuf.internal import legacy_features_pb2 from google.protobuf.internal import test_util from google.protobuf.internal import testing_refleaks from google.protobuf.internal import _parameterized +from google.protobuf import unittest_legacy_features_pb2 from google.protobuf import unittest_custom_options_pb2 from google.protobuf import unittest_features_pb2 from google.protobuf import unittest_import_pb2 @@ -1268,20 +1268,20 @@ def testDescriptorProtoOverrideFeatures(self): ) def testFeaturesStripped(self): - desc = legacy_features_pb2.TestEditionsMessage.DESCRIPTOR.fields_by_name[ + desc = unittest_legacy_features_pb2.TestEditionsMessage.DESCRIPTOR.fields_by_name[ 'required_field' ] self.assertFalse(desc.GetOptions().HasField('features')) def testLegacyRequiredTransform(self): - desc = legacy_features_pb2.TestEditionsMessage.DESCRIPTOR + desc = unittest_legacy_features_pb2.TestEditionsMessage.DESCRIPTOR self.assertEqual( desc.fields_by_name['required_field'].label, descriptor.FieldDescriptor.LABEL_REQUIRED, ) def testLegacyGroupTransform(self): - desc = legacy_features_pb2.TestEditionsMessage.DESCRIPTOR + desc = unittest_legacy_features_pb2.TestEditionsMessage.DESCRIPTOR self.assertEqual( desc.fields_by_name['delimited_field'].type, descriptor.FieldDescriptor.TYPE_GROUP, diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index ef167bb66b9c0..6a25c878dc375 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -772,6 +772,7 @@ filegroup( "unittest_lazy_dependencies.proto", "unittest_lazy_dependencies_custom_option.proto", "unittest_lazy_dependencies_enum.proto", + "unittest_legacy_features.proto", "unittest_no_field_presence.proto", "unittest_preserve_unknown_enum.proto", "unittest_preserve_unknown_enum2.proto", @@ -844,6 +845,27 @@ proto_library( ], ) +proto_library( + name = "generic_test_editions_protos", + srcs = [":test_proto_editions_srcs"], + strip_import_prefix = "/src", + visibility = ["//:__subpackages__"], + deps = [ + ":any_proto", + ":api_proto", + ":descriptor_proto", + ":duration_proto", + ":empty_proto", + ":field_mask_proto", + ":generic_test_protos", + ":source_context_proto", + ":struct_proto", + ":timestamp_proto", + ":type_proto", + ":wrappers_proto", + ], +) + exports_files( [ "test_messages_proto2.proto", diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index d5a125fcc9668..8a60501010ea9 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -2661,6 +2661,14 @@ bool CommandLineInterface::GenerateOutput( return false; } + // TODO: Remove once Java lite supports editions. + if (output_directive.name == "--java_out" && experimental_editions_) { + if (!parameters.empty()) { + parameters.append(","); + } + parameters.append("experimental_editions"); + } + if (!output_directive.generator->GenerateAll(parsed_files, parameters, generator_context, &error)) { // Generator returned an error. diff --git a/src/google/protobuf/compiler/java/generator.cc b/src/google/protobuf/compiler/java/generator.cc index 5435bb9e13911..e4fbd6531c556 100644 --- a/src/google/protobuf/compiler/java/generator.cc +++ b/src/google/protobuf/compiler/java/generator.cc @@ -38,7 +38,8 @@ JavaGenerator::JavaGenerator() {} JavaGenerator::~JavaGenerator() {} uint64_t JavaGenerator::GetSupportedFeatures() const { - return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL; + return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL | + CodeGenerator::Feature::FEATURE_SUPPORTS_EDITIONS; } bool JavaGenerator::Generate(const FileDescriptor* file, @@ -54,6 +55,7 @@ bool JavaGenerator::Generate(const FileDescriptor* file, file_options.opensource_runtime = opensource_runtime_; + bool enforce_editions = true; for (auto& option : options) { if (option.first == "output_list_file") { file_options.output_list_file = option.second; @@ -73,6 +75,8 @@ bool JavaGenerator::Generate(const FileDescriptor* file, file_options.annotation_list_file = option.second; } else if (option.first == "experimental_strip_nonfunctional_codegen") { file_options.strip_nonfunctional_codegen = true; + } else if (option.first == "experimental_editions") { + enforce_editions = false; } else { *error = absl::StrCat("Unknown generator option: ", option.first); return false; @@ -84,6 +88,13 @@ bool JavaGenerator::Generate(const FileDescriptor* file, return false; } + // TODO: Remove once Java lite supports editions + if (enforce_editions && file_options.enforce_lite && + GetEdition(*file) > google::protobuf::Edition::EDITION_PROTO3) { + *error = "lite runtime generator option cannot be used with editions yet."; + return false; + } + // By default we generate immutable code and shared code for immutable API. if (!file_options.generate_immutable_code && !file_options.generate_mutable_code && diff --git a/src/google/protobuf/compiler/java/kotlin_generator.cc b/src/google/protobuf/compiler/java/kotlin_generator.cc index 5cf3482edc42a..c9848b390d2f6 100644 --- a/src/google/protobuf/compiler/java/kotlin_generator.cc +++ b/src/google/protobuf/compiler/java/kotlin_generator.cc @@ -22,7 +22,8 @@ KotlinGenerator::KotlinGenerator() {} KotlinGenerator::~KotlinGenerator() {} uint64_t KotlinGenerator::GetSupportedFeatures() const { - return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL; + return CodeGenerator::Feature::FEATURE_PROTO3_OPTIONAL | + CodeGenerator::Feature::FEATURE_SUPPORTS_EDITIONS; } bool KotlinGenerator::Generate(const FileDescriptor* file, diff --git a/src/google/protobuf/editions/BUILD b/src/google/protobuf/editions/BUILD index 2cd0092fd8d07..1003ae21983c9 100644 --- a/src/google/protobuf/editions/BUILD +++ b/src/google/protobuf/editions/BUILD @@ -113,6 +113,13 @@ internal_objc_proto_library( visibility = ["//conformance:__pkg__"], ) +java_proto_library( + name = "test_messages_proto2_editions_java_proto", + testonly = True, + visibility = ["//conformance:__pkg__"], + deps = [":test_messages_proto2_editions_proto"], +) + py_proto_library( name = "test_messages_proto2_editions_py_pb2", testonly = True, @@ -182,6 +189,13 @@ internal_objc_proto_library( visibility = ["//conformance:__pkg__"], ) +java_proto_library( + name = "test_messages_proto3_editions_java_proto", + testonly = True, + visibility = ["//conformance:__pkg__"], + deps = [":test_messages_proto3_editions_proto"], +) + py_proto_library( name = "test_messages_proto3_editions_py_pb2", testonly = True, diff --git a/python/google/protobuf/internal/legacy_features.proto b/src/google/protobuf/unittest_legacy_features.proto similarity index 94% rename from python/google/protobuf/internal/legacy_features.proto rename to src/google/protobuf/unittest_legacy_features.proto index ef803ddbade8e..77a72da873431 100644 --- a/python/google/protobuf/internal/legacy_features.proto +++ b/src/google/protobuf/unittest_legacy_features.proto @@ -9,7 +9,7 @@ edition = "2023"; -package google.protobuf.internal; +package legacy_features_unittest; message TestEditionsMessage { int32 required_field = 1 [features.field_presence = LEGACY_REQUIRED];