From bca8fb6117cf24785edb238d96bffe2c3a6d7164 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 15 Mar 2024 15:15:45 -0700 Subject: [PATCH] Implement edition 2023 support in all Ruby runtimes. Three of these runtimes are based on upb, and the fourth is based on the Java runtime. Both of these already have editions support, so this was mostly just a matter of: - Advertising support to allow editions codegen - Stripping features from the runtime options - Hooking up conformance tests - Adding some lightweight editions tests There are also a few minor orthogonal fixes included here: - Ruby's upb hack for treating all enums as open enums needed tweaking - The `enable_editions` flag is no longer needed in our internal proto rules PiperOrigin-RevId: 616256211 --- bazel/amalgamate.py | 2 +- conformance/BUILD.bazel | 4 +- conformance/conformance_ruby.rb | 16 ++-- conformance/failure_list_jruby.txt | 71 ++++++++++++++ .../text_format_failure_list_jruby.txt | 8 -- conformance/text_format_failure_list_ruby.txt | 8 -- ruby/BUILD.bazel | 10 +- ruby/Rakefile | 2 + ruby/ext/google/protobuf_c/defs.c | 15 ++- ruby/lib/google/protobuf/ffi/descriptor.rb | 4 +- .../google/protobuf/ffi/enum_descriptor.rb | 4 +- ruby/lib/google/protobuf/ffi/ffi.rb | 5 - .../google/protobuf/ffi/field_descriptor.rb | 4 +- .../google/protobuf/ffi/file_descriptor.rb | 4 +- .../google/protobuf/ffi/oneof_descriptor.rb | 4 +- .../protobuf/jruby/RubyFieldDescriptor.java | 2 + ruby/tests/basic.rb | 80 ++++++++++++++++ ruby/tests/basic_proto2.rb | 31 ++++++ ruby/tests/basic_test.proto | 4 + ruby/tests/basic_test_features.proto | 21 ++++ ruby/tests/basic_test_proto2.proto | 14 +++ ruby/tests/generated_code_editions.proto | 96 +++++++++++++++++++ ruby/tests/generated_code_test.rb | 3 + .../protobuf/compiler/ruby/ruby_generator.h | 6 +- src/google/protobuf/editions/BUILD | 1 + upb/port/def.inc | 6 +- upb/port/undef.inc | 2 +- upb/reflection/enum_def.c | 6 +- upb/reflection/enum_def.h | 1 + upb/reflection/enum_value_def.c | 11 +-- 30 files changed, 388 insertions(+), 57 deletions(-) delete mode 100644 conformance/text_format_failure_list_jruby.txt delete mode 100644 conformance/text_format_failure_list_ruby.txt create mode 100644 ruby/tests/basic_test_features.proto create mode 100644 ruby/tests/generated_code_editions.proto diff --git a/bazel/amalgamate.py b/bazel/amalgamate.py index 8dc14cbb52..f4e83fc79a 100755 --- a/bazel/amalgamate.py +++ b/bazel/amalgamate.py @@ -54,7 +54,7 @@ class Amalgamator: self.output_c.write('#include "%s"\n' % (self.h_out)) if self.h_out == "ruby-upb.h": self.output_h.write("// Ruby is still using proto3 enum semantics for proto2\n") - self.output_h.write("#define UPB_DISABLE_PROTO2_ENUM_CHECKING\n") + self.output_h.write("#define UPB_DISABLE_CLOSED_ENUM_CHECKING\n") self.output_h.write("/* Amalgamated source file */\n") diff --git a/conformance/BUILD.bazel b/conformance/BUILD.bazel index f146052d3d..248b8ce4eb 100644 --- a/conformance/BUILD.bazel +++ b/conformance/BUILD.bazel @@ -36,9 +36,6 @@ exports_files([ "text_format_failure_list_python.txt", "text_format_failure_list_python_cpp.txt", "text_format_failure_list_python_upb.txt", - "text_format_failure_list_ruby.txt", - "text_format_failure_list_jruby.txt", - "text_format_failure_list_jruby_ffi.txt", ]) cc_proto_library( @@ -377,6 +374,7 @@ ruby_binary( deps = [ ":conformance_ruby_proto", "//ruby:conformance_test_ruby_proto", + "//ruby:protobuf", ], ) diff --git a/conformance/conformance_ruby.rb b/conformance/conformance_ruby.rb index 228872d84a..d7202e6b91 100755 --- a/conformance/conformance_ruby.rb +++ b/conformance/conformance_ruby.rb @@ -8,8 +8,11 @@ # https://developers.google.com/open-source/licenses/bsd require 'conformance/conformance_pb' +require 'google/protobuf' require 'google/protobuf/test_messages_proto3_pb' require 'google/protobuf/test_messages_proto2_pb' +require 'google/protobuf/editions/golden/test_messages_proto2_editions_pb' +require 'google/protobuf/editions/golden/test_messages_proto3_editions_pb' $test_count = 0 $verbose = false @@ -20,7 +23,12 @@ def do_test(request) descriptor = Google::Protobuf::DescriptorPool.generated_pool.lookup(request.message_type) unless descriptor - response.skipped = "Unknown message type: " + request.message_type + response.runtime_error = "Unknown message type: " + request.message_type + end + + if request.test_category == :TEXT_FORMAT_TEST + response.skipped = "Ruby doesn't support text format" + return response end begin @@ -45,12 +53,6 @@ def do_test(request) return response end - when :text_payload - begin - response.skipped = "Ruby doesn't support text format" - return response - end - when nil fail "Request didn't have payload" end diff --git a/conformance/failure_list_jruby.txt b/conformance/failure_list_jruby.txt index 9c5e06d622..3825b77083 100644 --- a/conformance/failure_list_jruby.txt +++ b/conformance/failure_list_jruby.txt @@ -69,3 +69,74 @@ Required.Proto3.JsonInput.Int32FieldPlusSign Required.Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotBool Required.Proto3.JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt Required.Proto3.JsonInput.StringFieldNotAString +Recommended.Editions_Proto3.FieldMaskNumbersDontRoundTrip.JsonOutput +Recommended.Editions_Proto3.FieldMaskPathsDontRoundTrip.JsonOutput +Recommended.Editions_Proto3.FieldMaskTooManyUnderscore.JsonOutput +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.FieldNameExtension.Validator +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 +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 +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 +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 diff --git a/conformance/text_format_failure_list_jruby.txt b/conformance/text_format_failure_list_jruby.txt deleted file mode 100644 index 404b64a584..0000000000 --- a/conformance/text_format_failure_list_jruby.txt +++ /dev/null @@ -1,8 +0,0 @@ -Recommended.Proto3.ProtobufInput.GroupUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.GroupUnknownFields_Print.TextFormatOutput -Recommended.Proto3.ProtobufInput.MessageUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.MessageUnknownFields_Print.TextFormatOutput -Recommended.Proto3.ProtobufInput.RepeatedUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.RepeatedUnknownFields_Print.TextFormatOutput -Recommended.Proto3.ProtobufInput.ScalarUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.ScalarUnknownFields_Print.TextFormatOutput diff --git a/conformance/text_format_failure_list_ruby.txt b/conformance/text_format_failure_list_ruby.txt deleted file mode 100644 index 404b64a584..0000000000 --- a/conformance/text_format_failure_list_ruby.txt +++ /dev/null @@ -1,8 +0,0 @@ -Recommended.Proto3.ProtobufInput.GroupUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.GroupUnknownFields_Print.TextFormatOutput -Recommended.Proto3.ProtobufInput.MessageUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.MessageUnknownFields_Print.TextFormatOutput -Recommended.Proto3.ProtobufInput.RepeatedUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.RepeatedUnknownFields_Print.TextFormatOutput -Recommended.Proto3.ProtobufInput.ScalarUnknownFields_Drop.TextFormatOutput -Recommended.Proto3.ProtobufInput.ScalarUnknownFields_Print.TextFormatOutput diff --git a/ruby/BUILD.bazel b/ruby/BUILD.bazel index 087a9c5f9d..9f85d45b46 100755 --- a/ruby/BUILD.bazel +++ b/ruby/BUILD.bazel @@ -113,6 +113,8 @@ internal_copy_files( srcs = [ "//src/google/protobuf:test_messages_proto2.proto", "//src/google/protobuf:test_messages_proto3.proto", + "//src/google/protobuf/editions:golden/test_messages_proto2_editions.proto", + "//src/google/protobuf/editions:golden/test_messages_proto3_editions.proto", ], strip_prefix = "src", ) @@ -232,12 +234,12 @@ filegroup( conformance_test( name = "conformance_test", failure_list = "//conformance:failure_list_ruby.txt", + maximum_edition = "2023", target_compatible_with = select({ ":ruby_native": [], "//conditions:default": ["@platforms//:incompatible"], }), testee = "//conformance:conformance_ruby", - text_format_failure_list = "//conformance:text_format_failure_list_ruby.txt", ) conformance_test( @@ -246,23 +248,23 @@ conformance_test( "PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION": "ffi", }, failure_list = "//conformance:failure_list_ruby.txt", + maximum_edition = "2023", target_compatible_with = select({ ":ruby_ffi": [], "//conditions:default": ["@platforms//:incompatible"], }), testee = "//conformance:conformance_ruby", - text_format_failure_list = "//conformance:text_format_failure_list_ruby.txt", ) conformance_test( name = "conformance_test_jruby", failure_list = "//conformance:failure_list_jruby.txt", + maximum_edition = "2023", target_compatible_with = select({ ":jruby_native": [], "//conditions:default": ["@platforms//:incompatible"], }), testee = "//conformance:conformance_ruby", - text_format_failure_list = "//conformance:text_format_failure_list_jruby.txt", ) conformance_test( @@ -271,12 +273,12 @@ conformance_test( "PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION": "ffi", }, failure_list = "//conformance:failure_list_jruby_ffi.txt", + maximum_edition = "2023", target_compatible_with = select({ ":jruby_ffi": [], "//conditions:default": ["@platforms//:incompatible"], }), testee = "//conformance:conformance_ruby", - text_format_failure_list = "//conformance:text_format_failure_list_jruby.txt", ) ################################################################################ diff --git a/ruby/Rakefile b/ruby/Rakefile index 6d842917c3..efcb17c82f 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -22,9 +22,11 @@ well_known_protos = %w[ test_protos = %w[ tests/basic_test.proto + tests/basic_test_features.proto tests/basic_test_proto2.proto tests/generated_code.proto tests/generated_code_proto2.proto + tests/generated_code_editions.proto tests/multi_level_nesting_test.proto tests/repeated_field_test.proto tests/stress.proto diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index 9141c7bd08..4f8b6bee16 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -257,7 +257,20 @@ static VALUE decode_options(VALUE self, const char* option_type, int size, VALUE desc_rb = get_msgdef_obj(descriptor_pool, msgdef); const Descriptor* desc = ruby_to_Descriptor(desc_rb); - options_rb = Message_decode_bytes(size, bytes, 0, desc->klass, true); + options_rb = Message_decode_bytes(size, bytes, 0, desc->klass, false); + + // Strip features from the options proto to keep it internal. + const upb_MessageDef* decoded_desc = NULL; + upb_Message* options = Message_GetMutable(options_rb, &decoded_desc); + PBRUBY_ASSERT(options != NULL); + PBRUBY_ASSERT(decoded_desc == msgdef); + const upb_FieldDef* field = + upb_MessageDef_FindFieldByName(decoded_desc, "features"); + PBRUBY_ASSERT(field != NULL); + upb_Message_ClearFieldByDef(options, field); + + Message_freeze(options_rb); + rb_ivar_set(self, options_instancevar_interned, options_rb); return options_rb; } diff --git a/ruby/lib/google/protobuf/ffi/descriptor.rb b/ruby/lib/google/protobuf/ffi/descriptor.rb index 3175d41355..1ccaf77a90 100644 --- a/ruby/lib/google/protobuf/ffi/descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/descriptor.rb @@ -100,7 +100,9 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.message_options(self, size_ptr, temporary_arena) - Google::Protobuf::MessageOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze + opts = Google::Protobuf::MessageOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze) + opts.clear_features() + opts.freeze end end diff --git a/ruby/lib/google/protobuf/ffi/enum_descriptor.rb b/ruby/lib/google/protobuf/ffi/enum_descriptor.rb index e280e393a1..e7ce04dab0 100644 --- a/ruby/lib/google/protobuf/ffi/enum_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/enum_descriptor.rb @@ -84,7 +84,9 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.enum_options(self, size_ptr, temporary_arena) - Google::Protobuf::EnumOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze + opts = Google::Protobuf::EnumOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze) + opts.clear_features() + opts.freeze end end diff --git a/ruby/lib/google/protobuf/ffi/ffi.rb b/ruby/lib/google/protobuf/ffi/ffi.rb index 765f04c2ba..2549fcb0a6 100644 --- a/ruby/lib/google/protobuf/ffi/ffi.rb +++ b/ruby/lib/google/protobuf/ffi/ffi.rb @@ -95,11 +95,6 @@ module Google :repeated ) - Syntax = enum( - :Proto2, 2, - :Proto3 - ) - # All the different kind of well known type messages. For simplicity of check, # number wrappers and string wrappers are grouped together. Make sure the # order and merber of these groups are not changed. diff --git a/ruby/lib/google/protobuf/ffi/field_descriptor.rb b/ruby/lib/google/protobuf/ffi/field_descriptor.rb index 82fd2a1812..632692b75d 100644 --- a/ruby/lib/google/protobuf/ffi/field_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/field_descriptor.rb @@ -219,7 +219,9 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.field_options(self, size_ptr, temporary_arena) - Google::Protobuf::FieldOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze + opts = Google::Protobuf::FieldOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze) + opts.clear_features() + opts.freeze end end diff --git a/ruby/lib/google/protobuf/ffi/file_descriptor.rb b/ruby/lib/google/protobuf/ffi/file_descriptor.rb index 2035b98efc..2a7bf87e6d 100644 --- a/ruby/lib/google/protobuf/ffi/file_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/file_descriptor.rb @@ -39,7 +39,9 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.file_options(@file_def, size_ptr, temporary_arena) - Google::Protobuf::FileOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze + opts = Google::Protobuf::FileOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze) + opts.clear_features() + opts.freeze end end end diff --git a/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb b/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb index 934f8f0ac4..65050ef0ac 100644 --- a/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb @@ -58,7 +58,9 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.oneof_options(self, size_ptr, temporary_arena) - Google::Protobuf::OneofOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze + opts = Google::Protobuf::OneofOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze) + opts.clear_features() + opts.freeze end end diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java index 6f52e10067..189d881fbc 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java @@ -287,6 +287,8 @@ public class RubyFieldDescriptor extends RubyObject { private void calculateLabel(ThreadContext context) { if (descriptor.isRepeated()) { this.label = context.runtime.newSymbol("repeated"); + } else if (descriptor.isRequired()) { + this.label = context.runtime.newSymbol("required"); } else if (descriptor.isOptional()) { this.label = context.runtime.newSymbol("optional"); } else { diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index b296a85f80..a45e6006af 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -3,6 +3,7 @@ # basic_test_pb.rb is in the same directory as this test. $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) +require 'basic_test_features_pb' require 'basic_test_pb' require 'common_tests' require 'google/protobuf' @@ -748,5 +749,84 @@ module BasicTest assert_equal str, m.optional_string end + + def test_proto3_explicit_presence + descriptor = TestMessage.descriptor.lookup("optional_int32") + assert_true descriptor.has_presence? + assert_false descriptor.options.has_features? + end + + def test_proto3_implicit_presence + descriptor = TestSingularFields.descriptor.lookup("singular_int32") + assert_false descriptor.has_presence? + assert_false descriptor.options.has_features? + end + + def test_proto3_packed_encoding + descriptor = TestMessage.descriptor.lookup("repeated_int32") + assert_true descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_proto3_expanded_encoding + descriptor = TestUnpackedMessage.descriptor.lookup("repeated_int32") + assert_false descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_proto3_expanded_encoding_unpackable + descriptor = TestMessage.descriptor.lookup("optional_msg") + assert_false descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_editions_explicit_presence + descriptor = TestFeaturesMessage.descriptor.lookup("explicit") + assert_true descriptor.has_presence? + assert_false descriptor.options.has_features? + end + + def test_editions_implicit_presence + descriptor = TestFeaturesMessage.descriptor.lookup("implicit") + assert_false descriptor.has_presence? + assert_false descriptor.options.has_features? + end + + def test_editions_required_presence + descriptor = TestFeaturesMessage.descriptor.lookup("legacy_required") + assert_equal :required, descriptor.label + assert_false descriptor.options.has_features? + end + + def test_editions_packed_encoding + descriptor = TestFeaturesMessage.descriptor.lookup("packed") + assert_true descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_editions_expanded_encoding + descriptor = TestFeaturesMessage.descriptor.lookup("expanded") + assert_false descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_editions_expanded_encoding_unpackable + descriptor = TestFeaturesMessage.descriptor.lookup("unpackable") + assert_false descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_field_delimited_encoding + descriptor = TestFeaturesMessage.descriptor.lookup("delimited") + assert_equal :group, descriptor.type + assert_false descriptor.options.has_features? + end + + def test_field_length_prefixed_encoding + descriptor = TestFeaturesMessage.descriptor.lookup("length_prefixed") + assert_equal :message, descriptor.type + assert_false descriptor.options.has_features? + end + end end diff --git a/ruby/tests/basic_proto2.rb b/ruby/tests/basic_proto2.rb index e9fce5751d..2112732664 100755 --- a/ruby/tests/basic_proto2.rb +++ b/ruby/tests/basic_proto2.rb @@ -6,6 +6,7 @@ $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) require 'basic_test_proto2_pb' require 'common_tests' require 'google/protobuf' +require 'google/protobuf/descriptor_pb' require 'json' require 'test/unit' @@ -347,5 +348,35 @@ module BasicTestProto2 decoded_message = TestMessageSet.decode encoded_message assert_equal message, decoded_message end + + def test_field_explicit_presence + descriptor = TestMessage.descriptor.lookup("optional_int32") + assert_true descriptor.has_presence? + assert_false descriptor.options.has_features? + end + + def test_field_expanded_encoding + descriptor = TestMessage.descriptor.lookup("repeated_int32") + assert_false descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_field_packed_encoding + descriptor = TestPackedMessage.descriptor.lookup("repeated_int32") + assert_true descriptor.is_packed? + assert_false descriptor.options.has_features? + end + + def test_field_group_type + descriptor = TestGroupMessage.descriptor.lookup("groupfield") + assert_equal :group, descriptor.type + assert_false descriptor.options.has_features? + end + + def test_field_required + descriptor = TestRequiredMessage.descriptor.lookup("required_int32") + assert_equal :required, descriptor.label + assert_false descriptor.options.has_features? + end end end diff --git a/ruby/tests/basic_test.proto b/ruby/tests/basic_test.proto index 5c9bb6012e..2479fa5a8a 100644 --- a/ruby/tests/basic_test.proto +++ b/ruby/tests/basic_test.proto @@ -53,6 +53,10 @@ message TestMessage { optional TestSingularFields optional_msg2 = 23; } +message TestUnpackedMessage { + repeated int32 repeated_int32 = 25 [packed = false]; +} + message TestSingularFields { int32 singular_int32 = 1; int64 singular_int64 = 2; diff --git a/ruby/tests/basic_test_features.proto b/ruby/tests/basic_test_features.proto new file mode 100644 index 0000000000..70e9a5bc3e --- /dev/null +++ b/ruby/tests/basic_test_features.proto @@ -0,0 +1,21 @@ +edition = "2023"; + +package basic_test; + +import "test_import_proto2.proto"; + +option features.enum_type = CLOSED; // Ignored by ruby. +option features.field_presence = IMPLICIT; + +message TestFeaturesMessage { + int32 implicit = 1; + int32 explicit = 2 [features.field_presence = EXPLICIT]; + int32 legacy_required = 3 [features.field_presence = LEGACY_REQUIRED]; + + repeated int32 packed = 50; + repeated int32 expanded = 51 [features.repeated_field_encoding = EXPANDED]; + repeated foo_bar.proto2.TestImportedMessage unpackable = 52; + + TestFeaturesMessage delimited = 100 [features.message_encoding = DELIMITED]; + TestFeaturesMessage length_prefixed = 101; +} diff --git a/ruby/tests/basic_test_proto2.proto b/ruby/tests/basic_test_proto2.proto index 85d61cf6b1..bc20418ba8 100644 --- a/ruby/tests/basic_test_proto2.proto +++ b/ruby/tests/basic_test_proto2.proto @@ -46,6 +46,20 @@ message TestMessage { repeated TestEnum repeated_enum = 22; } +message TestPackedMessage { + repeated int32 repeated_int32 = 12 [packed = true]; +} + +message TestGroupMessage { + optional group GroupField = 1 { + optional int32 a = 1; + } +} + +message TestRequiredMessage { + required int32 required_int32 = 1; +} + message TestMessage2 { optional int32 foo = 1; } diff --git a/ruby/tests/generated_code_editions.proto b/ruby/tests/generated_code_editions.proto new file mode 100644 index 0000000000..fc5eb04cca --- /dev/null +++ b/ruby/tests/generated_code_editions.proto @@ -0,0 +1,96 @@ +edition = "2023"; + +package a.b.editions; + +option features.repeated_field_encoding = EXPANDED; +option features.utf8_validation = NONE; + +message TestMessage { + int32 optional_int32 = 1; + int64 optional_int64 = 2; + uint32 optional_uint32 = 3; + uint64 optional_uint64 = 4; + bool optional_bool = 5; + double optional_double = 6; + float optional_float = 7; + string optional_string = 8; + bytes optional_bytes = 9; + TestEnum optional_enum = 10; + TestMessage optional_msg = 11; + repeated int32 repeated_int32 = 21; + repeated int64 repeated_int64 = 22; + repeated uint32 repeated_uint32 = 23; + repeated uint64 repeated_uint64 = 24; + repeated bool repeated_bool = 25; + repeated double repeated_double = 26; + repeated float repeated_float = 27; + repeated string repeated_string = 28; + repeated bytes repeated_bytes = 29; + repeated TestEnum repeated_enum = 30; + repeated TestMessage repeated_msg = 31; + int32 required_int32 = 41 [features.field_presence = LEGACY_REQUIRED]; + + int64 required_int64 = 42 [features.field_presence = LEGACY_REQUIRED]; + + uint32 required_uint32 = 43 [features.field_presence = LEGACY_REQUIRED]; + + uint64 required_uint64 = 44 [features.field_presence = LEGACY_REQUIRED]; + + bool required_bool = 45 [features.field_presence = LEGACY_REQUIRED]; + + double required_double = 46 [features.field_presence = LEGACY_REQUIRED]; + + float required_float = 47 [features.field_presence = LEGACY_REQUIRED]; + + string required_string = 48 [features.field_presence = LEGACY_REQUIRED]; + + bytes required_bytes = 49 [features.field_presence = LEGACY_REQUIRED]; + + TestEnum required_enum = 50 [features.field_presence = LEGACY_REQUIRED]; + + TestMessage required_msg = 51 [features.field_presence = LEGACY_REQUIRED]; + + oneof my_oneof { + int32 oneof_int32 = 61; + int64 oneof_int64 = 62; + uint32 oneof_uint32 = 63; + uint64 oneof_uint64 = 64; + bool oneof_bool = 65; + double oneof_double = 66; + float oneof_float = 67; + string oneof_string = 68; + bytes oneof_bytes = 69; + TestEnum oneof_enum = 70; + TestMessage oneof_msg = 71; + } + + message NestedMessage { + int32 foo = 1; + } + + NestedMessage nested_message = 80; + + // Reserved for non-existing field test. + // int32 non_exist = 89; +} + +enum TestEnum { + option features.enum_type = CLOSED; + + Default = 0; + A = 1; + B = 2; + C = 3; + v0 = 4; +} + +message TestUnknown { + TestUnknown optional_unknown = 11; + repeated TestUnknown repeated_unknown = 31; + + oneof my_oneof { + TestUnknown oneof_unknown = 51; + } + + int32 unknown_field = 89; +} diff --git a/ruby/tests/generated_code_test.rb b/ruby/tests/generated_code_test.rb index aed1cf8ebc..b2092c6e1a 100755 --- a/ruby/tests/generated_code_test.rb +++ b/ruby/tests/generated_code_test.rb @@ -3,6 +3,7 @@ # generated_code.rb is in the same directory as this test. $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) +require 'generated_code_editions_pb' require 'generated_code_pb' require 'test_import_pb' require 'test_ruby_package_pb' @@ -19,5 +20,7 @@ class GeneratedCodeTest < Test::Unit::TestCase A::B::C::TestLowercaseNested::Lowercase.new FooBar::TestImportedMessage.new A::B::TestRubyPackageMessage.new + A::B::Editions::TestMessage.new + A::B::Editions::TestMessage::NestedMessage.new end end diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.h b/src/google/protobuf/compiler/ruby/ruby_generator.h index b1164ecd37..55d33a30df 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.h +++ b/src/google/protobuf/compiler/ruby/ruby_generator.h @@ -13,7 +13,6 @@ #include #include "google/protobuf/compiler/code_generator.h" - #include "google/protobuf/port_def.inc" namespace google { @@ -30,8 +29,11 @@ class PROTOC_EXPORT Generator : public CodeGenerator { GeneratorContext* generator_context, std::string* error) const override; uint64_t GetSupportedFeatures() const override { - return FEATURE_PROTO3_OPTIONAL; + return Feature::FEATURE_PROTO3_OPTIONAL | + Feature::FEATURE_SUPPORTS_EDITIONS; } + Edition GetMinimumEdition() const override { return Edition::EDITION_PROTO2; } + Edition GetMaximumEdition() const override { return Edition::EDITION_2023; } }; } // namespace ruby diff --git a/src/google/protobuf/editions/BUILD b/src/google/protobuf/editions/BUILD index f7fc387ad3..8cb12e277d 100644 --- a/src/google/protobuf/editions/BUILD +++ b/src/google/protobuf/editions/BUILD @@ -221,6 +221,7 @@ exports_files( ], visibility = [ "//python:__pkg__", + "//ruby:__pkg__", ], ) diff --git a/upb/port/def.inc b/upb/port/def.inc index 1a41d45828..7d23d5c8b3 100644 --- a/upb/port/def.inc +++ b/upb/port/def.inc @@ -309,10 +309,10 @@ void __asan_unpoison_memory_region(void const volatile *addr, size_t size); /* Disable proto2 arena behavior (TEMPORARY) **********************************/ -#ifdef UPB_DISABLE_PROTO2_ENUM_CHECKING -#define UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 1 +#ifdef UPB_DISABLE_CLOSED_ENUM_CHECKING +#define UPB_TREAT_CLOSED_ENUMS_LIKE_OPEN 1 #else -#define UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 0 +#define UPB_TREAT_CLOSED_ENUMS_LIKE_OPEN 0 #endif #if defined(__cplusplus) diff --git a/upb/port/undef.inc b/upb/port/undef.inc index e4aec84b50..c715441fd1 100644 --- a/upb/port/undef.inc +++ b/upb/port/undef.inc @@ -45,7 +45,7 @@ #undef UPB_ASAN #undef UPB_ASAN_GUARD_SIZE #undef UPB_CLANG_ASAN -#undef UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 +#undef UPB_TREAT_CLOSED_ENUMS_LIKE_OPEN #undef UPB_DEPRECATED #undef UPB_GNUC_MIN #undef UPB_DESCRIPTOR_UPB_H_FILENAME diff --git a/upb/reflection/enum_def.c b/upb/reflection/enum_def.c index de85cbd9b1..4a6b2408ae 100644 --- a/upb/reflection/enum_def.c +++ b/upb/reflection/enum_def.c @@ -160,7 +160,11 @@ const upb_EnumValueDef* upb_EnumDef_Value(const upb_EnumDef* e, int i) { } bool upb_EnumDef_IsClosed(const upb_EnumDef* e) { - if (UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3) return false; + if (UPB_TREAT_CLOSED_ENUMS_LIKE_OPEN) return false; + return upb_EnumDef_IsSpecifiedAsClosed(e); +} + +bool upb_EnumDef_IsSpecifiedAsClosed(const upb_EnumDef* e) { return UPB_DESC(FeatureSet_enum_type)(e->resolved_features) == UPB_DESC(FeatureSet_CLOSED); } diff --git a/upb/reflection/enum_def.h b/upb/reflection/enum_def.h index a81ce5fd11..197fb45144 100644 --- a/upb/reflection/enum_def.h +++ b/upb/reflection/enum_def.h @@ -33,6 +33,7 @@ UPB_API const upb_EnumValueDef* upb_EnumDef_FindValueByNumber( UPB_API const char* upb_EnumDef_FullName(const upb_EnumDef* e); bool upb_EnumDef_HasOptions(const upb_EnumDef* e); bool upb_EnumDef_IsClosed(const upb_EnumDef* e); +bool upb_EnumDef_IsSpecifiedAsClosed(const upb_EnumDef* e); // Creates a mini descriptor string for an enum, returns true on success. bool upb_EnumDef_MiniDescriptorEncode(const upb_EnumDef* e, upb_Arena* a, diff --git a/upb/reflection/enum_value_def.c b/upb/reflection/enum_value_def.c index b9b94c9dc9..0ede8463d3 100644 --- a/upb/reflection/enum_value_def.c +++ b/upb/reflection/enum_value_def.c @@ -113,15 +113,10 @@ static void create_enumvaldef(upb_DefBuilder* ctx, const char* prefix, static void _upb_EnumValueDef_CheckZeroValue(upb_DefBuilder* ctx, const upb_EnumDef* e, const upb_EnumValueDef* v, int n) { - if (upb_EnumDef_IsClosed(e) || n == 0 || v[0].number == 0) return; - - // When the special UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 is enabled, we have to - // exempt proto2 enums from this check, even when we are treating them as + // When the special UPB_TREAT_CLOSED_ENUMS_LIKE_OPEN is enabled, we have to + // exempt closed enums from this check, even when we are treating them as // open. - if (UPB_TREAT_PROTO2_ENUMS_LIKE_PROTO3 && - upb_FileDef_Syntax(upb_EnumDef_File(e)) == kUpb_Syntax_Proto2) { - return; - } + if (upb_EnumDef_IsSpecifiedAsClosed(e) || n == 0 || v[0].number == 0) return; _upb_DefBuilder_Errf(ctx, "for open enums, the first value must be zero (%s)", upb_EnumDef_FullName(e));