From ef80643eb22daab37c74137bea265607d572f78f Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 24 Aug 2022 11:14:34 +0800 Subject: [PATCH 01/20] Auto capitalize enums name in Ruby This closes #1965. --- ruby/ext/google/protobuf_c/message.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index b11878545f039..d0481673252c2 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -1263,13 +1263,17 @@ VALUE build_module_from_enumdesc(VALUE _enumdesc) { int n = upb_EnumDef_ValueCount(e); for (int i = 0; i < n; i++) { const upb_EnumValueDef* ev = upb_EnumDef_Value(e, i); - const char* name = upb_EnumValueDef_Name(ev); + char* name = upb_EnumValueDef_Name(ev); int32_t value = upb_EnumValueDef_Number(ev); if (name[0] < 'A' || name[0] > 'Z') { - rb_warn( + if (name[0] >= 'a' && name[0] <= 'z') { + name[0] -= 32; // auto capitalize + } else { + rb_warn( "Enum value '%s' does not start with an uppercase letter " "as is required for Ruby constants.", name); + } } rb_define_const(mod, name, INT2NUM(value)); } From 44895f1977dd0d7dc2bc6338d02a949fb2da73b6 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 24 Aug 2022 13:23:52 +0800 Subject: [PATCH 02/20] dup string for modification Signed-off-by: tison --- ruby/ext/google/protobuf_c/message.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index d0481673252c2..6c7b22d9264ef 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -1263,7 +1263,7 @@ VALUE build_module_from_enumdesc(VALUE _enumdesc) { int n = upb_EnumDef_ValueCount(e); for (int i = 0; i < n; i++) { const upb_EnumValueDef* ev = upb_EnumDef_Value(e, i); - char* name = upb_EnumValueDef_Name(ev); + char* name = strdup(upb_EnumValueDef_Name(ev)); int32_t value = upb_EnumValueDef_Number(ev); if (name[0] < 'A' || name[0] > 'Z') { if (name[0] >= 'a' && name[0] <= 'z') { From 1314fb9747a2da9b6c61f9bd1d3c22aeb05142a7 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 4 Sep 2022 15:30:20 +0800 Subject: [PATCH 03/20] free name string Signed-off-by: tison --- ruby/ext/google/protobuf_c/message.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 6c7b22d9264ef..5acff8096dbb5 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -1276,6 +1276,7 @@ VALUE build_module_from_enumdesc(VALUE _enumdesc) { } } rb_define_const(mod, name, INT2NUM(value)); + free(name); } rb_define_singleton_method(mod, "lookup", enum_lookup, 1); From 3ac6b8716dee2db9b58d1c045e049598a5b9fb77 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 4 Sep 2022 16:13:44 +0800 Subject: [PATCH 04/20] tolerate lowercase first letter for JRuby Signed-off-by: tison --- .../com/google/protobuf/jruby/RubyEnumDescriptor.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java index 65328676e11ea..faa263ce8013b 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java @@ -163,8 +163,15 @@ private RubyModule buildModuleFromDescriptor(ThreadContext context) { descriptor.getFile().getSyntax() == FileDescriptor.Syntax.PROTO3; for (EnumValueDescriptor value : descriptor.getValues()) { String name = value.getName(); - // Make sure its a valid constant name before trying to create it - if (Character.isUpperCase(name.codePointAt(0))) { + // Make sure it's a valid constant name before trying to create it + int ch = name.codePointAt(0); + if (Character.isUpperCase(ch)) { + enumModule.defineConstant(name, runtime.newFixnum(value.getNumber())); + } else if (ch >= 'a' && ch <= 'z') { + // Protobuf enums can start with lowercase letters, while Ruby's symbol should + // always start with uppercase letters. We tolerate this case by capitalizing + // the first character if possible. + name = Character.toUpperCase(ch) + name.substring(1); enumModule.defineConstant(name, runtime.newFixnum(value.getNumber())); } else { runtime From d30f0bb62a8d234c896770eb9d9c51d63fe1b90c Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 4 Sep 2022 16:23:37 +0800 Subject: [PATCH 05/20] add test cases Signed-off-by: tison --- ruby/tests/common_tests.rb | 10 +++++++--- ruby/tests/generated_code.proto | 2 ++ ruby/tests/generated_code_proto2.proto | 2 ++ ruby/tests/repeated_field_test.rb | 1 + 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb index 5918c8a8b136c..5abd7dfb02d15 100644 --- a/ruby/tests/common_tests.rb +++ b/ruby/tests/common_tests.rb @@ -331,14 +331,16 @@ def test_rptfield_enum l.push :A l.push :B l.push :C - assert l.count == 3 + l.push :V0 + assert l.count == 4 assert_raise RangeError do l.push :D end assert l[0] == :A + assert l[3] == :V0 - l.push 4 - assert l[3] == 4 + l.push 5 + assert l[4] == 5 end def test_rptfield_initialize @@ -713,6 +715,8 @@ def test_enum_lookup assert proto_module::TestEnum::B == 2 assert proto_module::TestEnum::C == 3 + assert proto_module::TestEnum::V0 == 4 + assert proto_module::TestEnum::lookup(1) == :A assert proto_module::TestEnum::lookup(2) == :B assert proto_module::TestEnum::lookup(3) == :C diff --git a/ruby/tests/generated_code.proto b/ruby/tests/generated_code.proto index bfdfa5aa78098..5f017ba7ca716 100644 --- a/ruby/tests/generated_code.proto +++ b/ruby/tests/generated_code.proto @@ -67,6 +67,8 @@ enum TestEnum { A = 1; B = 2; C = 3; + + v0 = 4; } message testLowercaseNested { diff --git a/ruby/tests/generated_code_proto2.proto b/ruby/tests/generated_code_proto2.proto index 1e957219fac68..1a50b84be560b 100644 --- a/ruby/tests/generated_code_proto2.proto +++ b/ruby/tests/generated_code_proto2.proto @@ -68,6 +68,8 @@ enum TestEnum { A = 1; B = 2; C = 3; + + v0 = 4; } message TestUnknown { diff --git a/ruby/tests/repeated_field_test.rb b/ruby/tests/repeated_field_test.rb index 881810cdc5e11..bf46aff4fd00d 100755 --- a/ruby/tests/repeated_field_test.rb +++ b/ruby/tests/repeated_field_test.rb @@ -697,6 +697,7 @@ def fill_test_msg(test_msg) value :A, 1 value :B, 2 value :C, 3 + value :V0, 4 end end From b5da5e7263487265b14c697ed05f00da61ee733e Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 10 Sep 2022 01:27:18 +0800 Subject: [PATCH 06/20] update the compiler Signed-off-by: tison --- src/google/protobuf/compiler/ruby/ruby_generator.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc index 1e9e5c59da611..5dc7838329a1f 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc @@ -296,10 +296,17 @@ void GenerateEnum(const EnumDescriptor* en, io::Printer* printer) { for (int i = 0; i < en->value_count(); i++) { const EnumValueDescriptor* value = en->value(i); + char* name = strdup(value->name()); + if (name[0] < 'A' || name[0] > 'Z') { + if (name[0] >= 'a' && name[0] <= 'z') { + name[0] -= 32; // auto capitalize + } + } printer->Print( "value :$name$, $number$\n", - "name", value->name(), + "name", name, "number", NumberToString(value->number())); + free(name); } printer->Outdent(); From 8f604a781a955b93d8fff934b03eb6693f3c320b Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 10 Sep 2022 01:48:22 +0800 Subject: [PATCH 07/20] use cpp style Signed-off-by: tison --- src/google/protobuf/compiler/ruby/ruby_generator.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc index 5dc7838329a1f..8f6071c061937 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc @@ -296,7 +296,7 @@ void GenerateEnum(const EnumDescriptor* en, io::Printer* printer) { for (int i = 0; i < en->value_count(); i++) { const EnumValueDescriptor* value = en->value(i); - char* name = strdup(value->name()); + std::string name = std::string(value->name()); if (name[0] < 'A' || name[0] > 'Z') { if (name[0] >= 'a' && name[0] <= 'z') { name[0] -= 32; // auto capitalize @@ -306,7 +306,6 @@ void GenerateEnum(const EnumDescriptor* en, io::Printer* printer) { "value :$name$, $number$\n", "name", name, "number", NumberToString(value->number())); - free(name); } printer->Outdent(); From 4515ed3e7cdffbb9755e4c73ad0756eeafeedf67 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 10 Sep 2022 02:41:44 +0800 Subject: [PATCH 08/20] fix test Signed-off-by: tison --- ruby/compatibility_tests/v3.0.0/tests/basic.rb | 4 ++-- ruby/tests/basic_test.proto | 1 + ruby/tests/basic_test_proto2.proto | 1 + ruby/tests/common_tests.rb | 8 ++++---- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ruby/compatibility_tests/v3.0.0/tests/basic.rb b/ruby/compatibility_tests/v3.0.0/tests/basic.rb index 7228144cb165b..d45c19678f152 100755 --- a/ruby/compatibility_tests/v3.0.0/tests/basic.rb +++ b/ruby/compatibility_tests/v3.0.0/tests/basic.rb @@ -667,8 +667,8 @@ def test_map_msg_enum_valuetypes assert m["z"] == :C m["z"] = 2 assert m["z"] == :B - m["z"] = 4 - assert m["z"] == 4 + m["z"] = 5 + assert m["z"] == 5 assert_raise RangeError do m["z"] = :Z end diff --git a/ruby/tests/basic_test.proto b/ruby/tests/basic_test.proto index fb70f479db5bf..d480d48e548b7 100644 --- a/ruby/tests/basic_test.proto +++ b/ruby/tests/basic_test.proto @@ -73,6 +73,7 @@ enum TestEnum { A = 1; B = 2; C = 3; + v0 = 4; } message TestEmbeddedMessageParent { diff --git a/ruby/tests/basic_test_proto2.proto b/ruby/tests/basic_test_proto2.proto index 0c1a2b98363f0..ac705ed6305d6 100644 --- a/ruby/tests/basic_test_proto2.proto +++ b/ruby/tests/basic_test_proto2.proto @@ -69,6 +69,7 @@ enum TestEnum { A = 1; B = 2; C = 3; + v0 = 4; } enum TestNonZeroEnum { diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb index 5abd7dfb02d15..0accc7b10633f 100644 --- a/ruby/tests/common_tests.rb +++ b/ruby/tests/common_tests.rb @@ -544,8 +544,8 @@ def test_map_msg_enum_valuetypes assert m["z"] == :C m["z"] = 2 assert m["z"] == :B - m["z"] = 4 - assert m["z"] == 4 + m["z"] = 5 + assert m["z"] == 5 assert_raise RangeError do m["z"] = :Z end @@ -792,7 +792,7 @@ def test_enum_getter_only_enums assert_raise(NoMethodError) { m.a } assert_raise(NoMethodError) { m.a_const_const } end - + def test_repeated_push m = proto_module::TestMessage.new @@ -1766,7 +1766,7 @@ def test_freeze assert_raise(FrozenErrorType) { m.repeated_msg = proto_module::TestMessage2.new } assert_raise(FrozenErrorType) { m.repeated_enum = :A } end - + def test_eq m1 = proto_module::TestMessage.new(:optional_string => 'foo', :repeated_string => ['bar1', 'bar2']) m2 = proto_module::TestMessage.new(:optional_string => 'foo', :repeated_string => ['bar1', 'bar2']) From a701223404f989dcb4d549a2012c3e3770fd7505 Mon Sep 17 00:00:00 2001 From: tison Date: Sat, 10 Sep 2022 02:43:38 +0800 Subject: [PATCH 09/20] add tests for lookup and resolve Signed-off-by: tison --- ruby/tests/common_tests.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb index 0accc7b10633f..6cc4369fec021 100644 --- a/ruby/tests/common_tests.rb +++ b/ruby/tests/common_tests.rb @@ -714,16 +714,17 @@ def test_enum_lookup assert proto_module::TestEnum::A == 1 assert proto_module::TestEnum::B == 2 assert proto_module::TestEnum::C == 3 - assert proto_module::TestEnum::V0 == 4 assert proto_module::TestEnum::lookup(1) == :A assert proto_module::TestEnum::lookup(2) == :B assert proto_module::TestEnum::lookup(3) == :C + assert proto_module::TestEnum::lookup(4) == :V0 assert proto_module::TestEnum::resolve(:A) == 1 assert proto_module::TestEnum::resolve(:B) == 2 assert proto_module::TestEnum::resolve(:C) == 3 + assert proto_module::TestEnum::resolve(:V0) == 4 end def test_enum_const_get_helpers From 775a0e7008599a44727a85ccee94ab5eaf932b29 Mon Sep 17 00:00:00 2001 From: tison Date: Tue, 13 Sep 2022 23:01:17 +0800 Subject: [PATCH 10/20] fix binary_json_conformance_suite Signed-off-by: tison --- conformance/binary_json_conformance_suite.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 3719a527aab7c..73ce15be0fb62 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -2230,11 +2230,11 @@ void BinaryAndJsonConformanceSuite::RunJsonTestsForNonRepeatedTypes() { "optional_aliased_enum: ALIAS_BAZ"); RunValidJsonTest( "EnumFieldWithAliasLowerCase", REQUIRED, - R"({"optionalAliasedEnum": "moo"})", + R"({"optionalAliasedEnum": "Moo"})", "optional_aliased_enum: ALIAS_BAZ"); RunValidJsonTest( "EnumFieldWithAliasDifferentCase", REQUIRED, - R"({"optionalAliasedEnum": "bAz"})", + R"({"optionalAliasedEnum": "BAz"})", "optional_aliased_enum: ALIAS_BAZ"); // Enum values must be represented as strings. ExpectParseFailureForJson( From 86f13da7cc09244f929f27602ec912696b4f9509 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 14 Sep 2022 22:05:45 +0800 Subject: [PATCH 11/20] Revert "fix binary_json_conformance_suite" This reverts commit 775a0e7008599a44727a85ccee94ab5eaf932b29. --- conformance/binary_json_conformance_suite.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conformance/binary_json_conformance_suite.cc b/conformance/binary_json_conformance_suite.cc index 73ce15be0fb62..3719a527aab7c 100644 --- a/conformance/binary_json_conformance_suite.cc +++ b/conformance/binary_json_conformance_suite.cc @@ -2230,11 +2230,11 @@ void BinaryAndJsonConformanceSuite::RunJsonTestsForNonRepeatedTypes() { "optional_aliased_enum: ALIAS_BAZ"); RunValidJsonTest( "EnumFieldWithAliasLowerCase", REQUIRED, - R"({"optionalAliasedEnum": "Moo"})", + R"({"optionalAliasedEnum": "moo"})", "optional_aliased_enum: ALIAS_BAZ"); RunValidJsonTest( "EnumFieldWithAliasDifferentCase", REQUIRED, - R"({"optionalAliasedEnum": "BAz"})", + R"({"optionalAliasedEnum": "bAz"})", "optional_aliased_enum: ALIAS_BAZ"); // Enum values must be represented as strings. ExpectParseFailureForJson( From 29bfb0718bd2a704931c0601dcd70e36dd0d4c22 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 14 Sep 2022 22:39:13 +0800 Subject: [PATCH 12/20] auto capitalize for json decode Signed-off-by: tison --- ruby/ext/google/protobuf_c/ruby-upb.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ruby/ext/google/protobuf_c/ruby-upb.c b/ruby/ext/google/protobuf_c/ruby-upb.c index d50bd99f9c421..0bd6d2a0e0143 100755 --- a/ruby/ext/google/protobuf_c/ruby-upb.c +++ b/ruby/ext/google/protobuf_c/ruby-upb.c @@ -9639,6 +9639,10 @@ static upb_MessageValue jsondec_enum(jsondec* d, const upb_FieldDef* f) { switch (jsondec_peek(d)) { case JD_STRING: { upb_StringView str = jsondec_string(d); + char* name = strdup(str.data); + if (name[0] >= 'a' && name[0] <= 'z') { + name[0] -= 32; // auto capitalize + } const upb_EnumDef* e = upb_FieldDef_EnumSubDef(f); const upb_EnumValueDef* ev = upb_EnumDef_FindValueByNameWithSize(e, str.data, str.size); @@ -9653,6 +9657,7 @@ static upb_MessageValue jsondec_enum(jsondec* d, const upb_FieldDef* f) { UPB_STRINGVIEW_ARGS(str)); } } + free(name); return val; } case JD_NULL: { From bc78de3b3eb6d432a033d5a8d73a7a47622d41dd Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 14 Sep 2022 22:44:41 +0800 Subject: [PATCH 13/20] pass the correct argument Signed-off-by: tison --- ruby/ext/google/protobuf_c/ruby-upb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/ruby-upb.c b/ruby/ext/google/protobuf_c/ruby-upb.c index 0bd6d2a0e0143..9f080154cbf30 100755 --- a/ruby/ext/google/protobuf_c/ruby-upb.c +++ b/ruby/ext/google/protobuf_c/ruby-upb.c @@ -9645,7 +9645,7 @@ static upb_MessageValue jsondec_enum(jsondec* d, const upb_FieldDef* f) { } const upb_EnumDef* e = upb_FieldDef_EnumSubDef(f); const upb_EnumValueDef* ev = - upb_EnumDef_FindValueByNameWithSize(e, str.data, str.size); + upb_EnumDef_FindValueByNameWithSize(e, name, str.size); upb_MessageValue val; if (ev) { val.int32_val = upb_EnumValueDef_Number(ev); From f369bcb4e1d7d3d209f804c4a7e991f22ed48fd3 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 14 Sep 2022 23:14:53 +0800 Subject: [PATCH 14/20] update for JRuby Signed-off-by: tison --- .../main/java/com/google/protobuf/jruby/RubyMessage.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 301b95798215c..792f1f8893c76 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -1064,7 +1064,12 @@ private Object convert( if (Utils.isRubyNum(value)) { val = enumDescriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value)); } else { - val = enumDescriptor.findValueByName(value.asJavaString()); + String name = value.asJavaString(); + int ch = name.codePointAt(0); + if (ch >= 'a' && ch <= 'z') { + name = Character.toUpperCase(ch) + name.substring(1); + } + val = enumDescriptor.findValueByName(name); } break; default: From 4bb02e09d373a21dde2e7522d6c29d16a4810603 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 14 Sep 2022 23:39:21 +0800 Subject: [PATCH 15/20] more jruby Signed-off-by: tison --- .../google/protobuf/jruby/RubyEnumDescriptor.java | 12 +++--------- .../java/com/google/protobuf/jruby/RubyMessage.java | 7 +------ .../main/java/com/google/protobuf/jruby/Utils.java | 13 +++++++++++++ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java index faa263ce8013b..acb43e0ec5712 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java @@ -130,14 +130,14 @@ public boolean isValidValue(ThreadContext context, IRubyObject value) { if (Utils.isRubyNum(value)) { enumValue = descriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value)); } else { - enumValue = descriptor.findValueByName(value.asJavaString()); + enumValue = descriptor.findValueByName(Utils.fixEnumName(value.asJavaString())); } return enumValue != null; } protected IRubyObject nameToNumber(ThreadContext context, IRubyObject name) { - EnumValueDescriptor value = descriptor.findValueByName(name.asJavaString()); + EnumValueDescriptor value = descriptor.findValueByName(Utils.fixEnumName(name.asJavaString())); return value == null ? context.nil : context.runtime.newFixnum(value.getNumber()); } @@ -162,17 +162,11 @@ private RubyModule buildModuleFromDescriptor(ThreadContext context) { boolean defaultValueRequiredButNotFound = descriptor.getFile().getSyntax() == FileDescriptor.Syntax.PROTO3; for (EnumValueDescriptor value : descriptor.getValues()) { - String name = value.getName(); + String name = Utils.fixEnumName(value.getName()); // Make sure it's a valid constant name before trying to create it int ch = name.codePointAt(0); if (Character.isUpperCase(ch)) { enumModule.defineConstant(name, runtime.newFixnum(value.getNumber())); - } else if (ch >= 'a' && ch <= 'z') { - // Protobuf enums can start with lowercase letters, while Ruby's symbol should - // always start with uppercase letters. We tolerate this case by capitalizing - // the first character if possible. - name = Character.toUpperCase(ch) + name.substring(1); - enumModule.defineConstant(name, runtime.newFixnum(value.getNumber())); } else { runtime .getWarnings() diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 792f1f8893c76..b33cd3a9b4f6c 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -1064,12 +1064,7 @@ private Object convert( if (Utils.isRubyNum(value)) { val = enumDescriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value)); } else { - String name = value.asJavaString(); - int ch = name.codePointAt(0); - if (ch >= 'a' && ch <= 'z') { - name = Character.toUpperCase(ch) + name.substring(1); - } - val = enumDescriptor.findValueByName(name); + val = enumDescriptor.findValueByName(Utils.fixEnumName(value.asJavaString())); } break; default: diff --git a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java index 65de683b02262..50a582a65f930 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java @@ -347,6 +347,19 @@ public static RaiseException createInvalidTypeError( String.format(INVALID_TYPE_ERROR_FORMAT, fieldType, fieldName, value.getMetaClass())); } + public static String fixEnumName(String name) { + if (name != null && name.length() > 0) { + int ch = name.codePointAt(0); + if (ch >= 'a' && ch <= 'z') { + // Protobuf enums can start with lowercase letters, while Ruby's symbol should + // always start with uppercase letters. We tolerate this case by capitalizing + // the first character if possible. + return Character.toUpperCase(ch) + name.substring(1); + } + } + return name; + } + protected static boolean isRubyNum(Object value) { return value instanceof RubyFixnum || value instanceof RubyFloat || value instanceof RubyBignum; } From 55a70a9ec9247f2314528f0962f95f547e1a9c3d Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 14 Sep 2022 23:41:26 +0800 Subject: [PATCH 16/20] fix wording Signed-off-by: tison --- ruby/src/main/java/com/google/protobuf/jruby/Utils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java index 50a582a65f930..a8e1354b37254 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java @@ -351,7 +351,7 @@ public static String fixEnumName(String name) { if (name != null && name.length() > 0) { int ch = name.codePointAt(0); if (ch >= 'a' && ch <= 'z') { - // Protobuf enums can start with lowercase letters, while Ruby's symbol should + // Protobuf enums can start with lowercase letters, while Ruby's constant should // always start with uppercase letters. We tolerate this case by capitalizing // the first character if possible. return Character.toUpperCase(ch) + name.substring(1); From d93fa0b9ed919400d6f5beaca5a9f56f57330d54 Mon Sep 17 00:00:00 2001 From: tison Date: Thu, 15 Sep 2022 09:34:06 +0800 Subject: [PATCH 17/20] add failure ignore Signed-off-by: tison --- conformance/failure_list_jruby.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/conformance/failure_list_jruby.txt b/conformance/failure_list_jruby.txt index 516192ec009d6..6c2a2c5202866 100644 --- a/conformance/failure_list_jruby.txt +++ b/conformance/failure_list_jruby.txt @@ -87,6 +87,10 @@ Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.Unpacked Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.UnpackedOutput.ProtobufOutput Required.Proto3.JsonInput.EnumFieldNotQuoted +Required.Proto3.JsonInput.EnumFieldWithAliasDifferentCase.JsonOutput +Required.Proto3.JsonInput.EnumFieldWithAliasDifferentCase.ProtobufOutput +Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.JsonOutput +Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.ProtobufOutput Required.Proto3.JsonInput.Int32FieldLeadingZero Required.Proto3.JsonInput.Int32FieldNegativeWithLeadingZero Required.Proto3.JsonInput.Int32FieldPlusSign From 4bc1091abb35e25f59f35c5cb07abe841f46b82c Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 28 Sep 2022 08:08:47 +0800 Subject: [PATCH 18/20] revert CRuby Enum unrelated changes Signed-off-by: tison --- conformance/failure_list_jruby.txt | 4 ---- ruby/ext/google/protobuf_c/ruby-upb.c | 7 +------ ruby/tests/common_tests.rb | 8 ++++---- ruby/tests/repeated_field_test.rb | 2 +- src/google/protobuf/compiler/ruby/ruby_generator.cc | 8 +------- 5 files changed, 7 insertions(+), 22 deletions(-) diff --git a/conformance/failure_list_jruby.txt b/conformance/failure_list_jruby.txt index 6c2a2c5202866..516192ec009d6 100644 --- a/conformance/failure_list_jruby.txt +++ b/conformance/failure_list_jruby.txt @@ -87,10 +87,6 @@ Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT32.UnpackedInput.Unpacked Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.PackedInput.UnpackedOutput.ProtobufOutput Recommended.Proto3.ProtobufInput.ValidDataRepeated.UINT64.UnpackedInput.UnpackedOutput.ProtobufOutput Required.Proto3.JsonInput.EnumFieldNotQuoted -Required.Proto3.JsonInput.EnumFieldWithAliasDifferentCase.JsonOutput -Required.Proto3.JsonInput.EnumFieldWithAliasDifferentCase.ProtobufOutput -Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.JsonOutput -Required.Proto3.JsonInput.EnumFieldWithAliasLowerCase.ProtobufOutput Required.Proto3.JsonInput.Int32FieldLeadingZero Required.Proto3.JsonInput.Int32FieldNegativeWithLeadingZero Required.Proto3.JsonInput.Int32FieldPlusSign diff --git a/ruby/ext/google/protobuf_c/ruby-upb.c b/ruby/ext/google/protobuf_c/ruby-upb.c index 9f080154cbf30..d50bd99f9c421 100755 --- a/ruby/ext/google/protobuf_c/ruby-upb.c +++ b/ruby/ext/google/protobuf_c/ruby-upb.c @@ -9639,13 +9639,9 @@ static upb_MessageValue jsondec_enum(jsondec* d, const upb_FieldDef* f) { switch (jsondec_peek(d)) { case JD_STRING: { upb_StringView str = jsondec_string(d); - char* name = strdup(str.data); - if (name[0] >= 'a' && name[0] <= 'z') { - name[0] -= 32; // auto capitalize - } const upb_EnumDef* e = upb_FieldDef_EnumSubDef(f); const upb_EnumValueDef* ev = - upb_EnumDef_FindValueByNameWithSize(e, name, str.size); + upb_EnumDef_FindValueByNameWithSize(e, str.data, str.size); upb_MessageValue val; if (ev) { val.int32_val = upb_EnumValueDef_Number(ev); @@ -9657,7 +9653,6 @@ static upb_MessageValue jsondec_enum(jsondec* d, const upb_FieldDef* f) { UPB_STRINGVIEW_ARGS(str)); } } - free(name); return val; } case JD_NULL: { diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb index 6cc4369fec021..928842553f3b4 100644 --- a/ruby/tests/common_tests.rb +++ b/ruby/tests/common_tests.rb @@ -331,13 +331,13 @@ def test_rptfield_enum l.push :A l.push :B l.push :C - l.push :V0 + l.push :v0 assert l.count == 4 assert_raise RangeError do l.push :D end assert l[0] == :A - assert l[3] == :V0 + assert l[3] == :v0 l.push 5 assert l[4] == 5 @@ -719,12 +719,12 @@ def test_enum_lookup assert proto_module::TestEnum::lookup(1) == :A assert proto_module::TestEnum::lookup(2) == :B assert proto_module::TestEnum::lookup(3) == :C - assert proto_module::TestEnum::lookup(4) == :V0 + assert proto_module::TestEnum::lookup(4) == :v0 assert proto_module::TestEnum::resolve(:A) == 1 assert proto_module::TestEnum::resolve(:B) == 2 assert proto_module::TestEnum::resolve(:C) == 3 - assert proto_module::TestEnum::resolve(:V0) == 4 + assert proto_module::TestEnum::resolve(:v0) == 4 end def test_enum_const_get_helpers diff --git a/ruby/tests/repeated_field_test.rb b/ruby/tests/repeated_field_test.rb index bf46aff4fd00d..de968699605c6 100755 --- a/ruby/tests/repeated_field_test.rb +++ b/ruby/tests/repeated_field_test.rb @@ -697,7 +697,7 @@ def fill_test_msg(test_msg) value :A, 1 value :B, 2 value :C, 3 - value :V0, 4 + value :v0, 4 end end diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc index 178921f849c56..be643d88b6e85 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc @@ -296,15 +296,9 @@ void GenerateEnum(const EnumDescriptor* en, io::Printer* printer) { for (int i = 0; i < en->value_count(); i++) { const EnumValueDescriptor* value = en->value(i); - std::string name = std::string(value->name()); - if (name[0] < 'A' || name[0] > 'Z') { - if (name[0] >= 'a' && name[0] <= 'z') { - name[0] -= 32; // auto capitalize - } - } printer->Print( "value :$name$, $number$\n", - "name", name, + "name", value->name(), "number", NumberToString(value->number())); } From c02698adf8c4b73004b9946c056e5655320e512b Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 28 Sep 2022 08:23:27 +0800 Subject: [PATCH 19/20] revert JRuby Enum unrelated changes Signed-off-by: tison --- .../java/com/google/protobuf/jruby/RubyEnumDescriptor.java | 4 ++-- .../src/main/java/com/google/protobuf/jruby/RubyMessage.java | 2 +- ruby/src/main/java/com/google/protobuf/jruby/Utils.java | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java index acb43e0ec5712..02575b914c303 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java @@ -130,14 +130,14 @@ public boolean isValidValue(ThreadContext context, IRubyObject value) { if (Utils.isRubyNum(value)) { enumValue = descriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value)); } else { - enumValue = descriptor.findValueByName(Utils.fixEnumName(value.asJavaString())); + enumValue = descriptor.findValueByName(value.asJavaString()); } return enumValue != null; } protected IRubyObject nameToNumber(ThreadContext context, IRubyObject name) { - EnumValueDescriptor value = descriptor.findValueByName(Utils.fixEnumName(name.asJavaString())); + EnumValueDescriptor value = descriptor.findValueByName(name.asJavaString()); return value == null ? context.nil : context.runtime.newFixnum(value.getNumber()); } diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index b33cd3a9b4f6c..301b95798215c 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -1064,7 +1064,7 @@ private Object convert( if (Utils.isRubyNum(value)) { val = enumDescriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value)); } else { - val = enumDescriptor.findValueByName(Utils.fixEnumName(value.asJavaString())); + val = enumDescriptor.findValueByName(value.asJavaString()); } break; default: diff --git a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java index a8e1354b37254..1e35f4609455f 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java @@ -354,7 +354,10 @@ public static String fixEnumName(String name) { // Protobuf enums can start with lowercase letters, while Ruby's constant should // always start with uppercase letters. We tolerate this case by capitalizing // the first character if possible. - return Character.toUpperCase(ch) + name.substring(1); + return new StringBuilder() + .appendCodePoint(Character.toUpperCase(ch)) + .append(name.substring(1)) + .toString(); } } return name; From 9d7fbf01f57f5825974994e3db1dd7df1a692b91 Mon Sep 17 00:00:00 2001 From: tison Date: Wed, 28 Sep 2022 08:57:57 +0800 Subject: [PATCH 20/20] improve JRuby method Signed-off-by: tison --- .../protobuf/jruby/RubyEnumDescriptor.java | 18 +++++++++++++++++- .../java/com/google/protobuf/jruby/Utils.java | 16 ---------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java index 02575b914c303..0eb7c939cb00e 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java @@ -162,7 +162,7 @@ private RubyModule buildModuleFromDescriptor(ThreadContext context) { boolean defaultValueRequiredButNotFound = descriptor.getFile().getSyntax() == FileDescriptor.Syntax.PROTO3; for (EnumValueDescriptor value : descriptor.getValues()) { - String name = Utils.fixEnumName(value.getName()); + String name = fixEnumConstantName(value.getName()); // Make sure it's a valid constant name before trying to create it int ch = name.codePointAt(0); if (Character.isUpperCase(ch)) { @@ -190,6 +190,22 @@ private RubyModule buildModuleFromDescriptor(ThreadContext context) { return enumModule; } + private static String fixEnumConstantName(String name) { + if (name != null && name.length() > 0) { + int ch = name.codePointAt(0); + if (ch >= 'a' && ch <= 'z') { + // Protobuf enums can start with lowercase letters, while Ruby's constant should + // always start with uppercase letters. We tolerate this case by capitalizing + // the first character if possible. + return new StringBuilder() + .appendCodePoint(Character.toUpperCase(ch)) + .append(name.substring(1)) + .toString(); + } + } + return name; + } + private EnumDescriptor descriptor; private EnumDescriptorProto.Builder builder; private IRubyObject name; diff --git a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java index 1e35f4609455f..65de683b02262 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/Utils.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/Utils.java @@ -347,22 +347,6 @@ public static RaiseException createInvalidTypeError( String.format(INVALID_TYPE_ERROR_FORMAT, fieldType, fieldName, value.getMetaClass())); } - public static String fixEnumName(String name) { - if (name != null && name.length() > 0) { - int ch = name.codePointAt(0); - if (ch >= 'a' && ch <= 'z') { - // Protobuf enums can start with lowercase letters, while Ruby's constant should - // always start with uppercase letters. We tolerate this case by capitalizing - // the first character if possible. - return new StringBuilder() - .appendCodePoint(Character.toUpperCase(ch)) - .append(name.substring(1)) - .toString(); - } - } - return name; - } - protected static boolean isRubyNum(Object value) { return value instanceof RubyFixnum || value instanceof RubyFloat || value instanceof RubyBignum; }