Skip to content

Commit

Permalink
Auto capitalize enums name in Ruby (#10454) (#10763)
Browse files Browse the repository at this point in the history
This closes #1965.
  • Loading branch information
tisonkun authored Oct 13, 2022
1 parent 5d90ef2 commit fa5a9e1
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 14 deletions.
4 changes: 2 additions & 2 deletions ruby/compatibility_tests/v3.0.0/tests/basic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions ruby/ext/google/protobuf_c/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -1290,15 +1290,20 @@ 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 = strdup(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));
free(name);
}

rb_define_singleton_method(mod, "lookup", enum_lookup, 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,10 @@ private RubyModule buildModuleFromDescriptor(ThreadContext context) {
boolean defaultValueRequiredButNotFound =
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))) {
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)) {
enumModule.defineConstant(name, runtime.newFixnum(value.getNumber()));
} else {
runtime
Expand All @@ -189,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;
Expand Down
1 change: 1 addition & 0 deletions ruby/tests/basic_test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ enum TestEnum {
A = 1;
B = 2;
C = 3;
v0 = 4;
}

message TestEmbeddedMessageParent {
Expand Down
1 change: 1 addition & 0 deletions ruby/tests/basic_test_proto2.proto
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ enum TestEnum {
A = 1;
B = 2;
C = 3;
v0 = 4;
}

enum TestNonZeroEnum {
Expand Down
19 changes: 12 additions & 7 deletions ruby/tests/common_tests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -542,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
Expand Down Expand Up @@ -712,14 +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
Expand Down Expand Up @@ -788,7 +793,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

Expand Down Expand Up @@ -1762,7 +1767,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'])
Expand Down
2 changes: 2 additions & 0 deletions ruby/tests/generated_code.proto
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ enum TestEnum {
A = 1;
B = 2;
C = 3;

v0 = 4;
}

message testLowercaseNested {
Expand Down
2 changes: 2 additions & 0 deletions ruby/tests/generated_code_proto2.proto
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ enum TestEnum {
A = 1;
B = 2;
C = 3;

v0 = 4;
}

message TestUnknown {
Expand Down
1 change: 1 addition & 0 deletions ruby/tests/repeated_field_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ def fill_test_msg(test_msg)
value :A, 1
value :B, 2
value :C, 3
value :v0, 4
end
end

Expand Down

0 comments on commit fa5a9e1

Please sign in to comment.