Skip to content

Commit d1a3c6d

Browse files
shaldengekicopybara-github
authored andcommitted
For Ruby oneof fields, generate hazzers for members (#11655)
When generating Ruby clients for proto3 messages that have a oneof, we generate a hazzer for members of the oneof, not just a hazzer for the oneof itself. In other words, for a proto like this: ``` syntax = "proto3"; message Foo { oneof bar { string baz = 1; } } ``` The generated `Foo` will now have a method called `has_baz?`, in addition to the (pre-existing) method `has_bar?`. I updated the unit tests, and verified that all the tests under `//ruby/...` pass. Fixes #9561. Closes #11655 COPYBARA_INTEGRATE_REVIEW=#11655 from shaldengeki:test-ruby-oneof-hazzer a15e474 PiperOrigin-RevId: 506090930
1 parent 96b1fce commit d1a3c6d

File tree

3 files changed

+17
-43
lines changed

3 files changed

+17
-43
lines changed

ruby/ext/google/protobuf_c/message.c

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,6 @@ static int extract_method_call(VALUE method_name, Message* self,
243243
if (Match(m, name, f, o, "clear_", "")) return METHOD_CLEAR;
244244
if (Match(m, name, f, o, "has_", "?") &&
245245
(*o || (*f && upb_FieldDef_HasPresence(*f)))) {
246-
// Disallow oneof hazzers for proto3.
247-
// TODO(haberman): remove this test when we are enabling oneof hazzers for
248-
// proto3.
249-
if (*f && !upb_FieldDef_IsSubMessage(*f) &&
250-
upb_FieldDef_RealContainingOneof(*f) &&
251-
upb_MessageDef_Syntax(upb_FieldDef_ContainingType(*f)) !=
252-
kUpb_Syntax_Proto2) {
253-
return METHOD_UNKNOWN;
254-
}
255246
return METHOD_PRESENCE;
256247
}
257248
if (Match(m, name, f, o, "", "_as_value") && *f &&
@@ -1287,12 +1278,12 @@ VALUE build_module_from_enumdesc(VALUE _enumdesc) {
12871278
int32_t value = upb_EnumValueDef_Number(ev);
12881279
if (name[0] < 'A' || name[0] > 'Z') {
12891280
if (name[0] >= 'a' && name[0] <= 'z') {
1290-
name[0] -= 32; // auto capitalize
1281+
name[0] -= 32; // auto capitalize
12911282
} else {
12921283
rb_warn(
1293-
"Enum value '%s' does not start with an uppercase letter "
1294-
"as is required for Ruby constants.",
1295-
name);
1284+
"Enum value '%s' does not start with an uppercase letter "
1285+
"as is required for Ruby constants.",
1286+
name);
12961287
}
12971288
}
12981289
rb_define_const(mod, name, INT2NUM(value));

ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -306,11 +306,7 @@ public IRubyObject respondTo(ThreadContext context, IRubyObject[] args) {
306306
if (methodName.startsWith(HAS_PREFIX) && methodName.endsWith(QUESTION_MARK)) {
307307
String strippedMethodName = methodName.substring(4, methodName.length() - 1);
308308
FieldDescriptor fieldDescriptor = descriptor.findFieldByName(strippedMethodName);
309-
if (fieldDescriptor != null
310-
&& (!proto3
311-
|| fieldDescriptor.getContainingOneof() == null
312-
|| fieldDescriptor.getContainingOneof().isSynthetic())
313-
&& fieldDescriptor.hasPresence()) {
309+
if (fieldDescriptor != null && fieldDescriptor.hasPresence()) {
314310
return context.runtime.getTrue();
315311
}
316312
oneofDescriptor =
@@ -459,11 +455,7 @@ public IRubyObject methodMissing(ThreadContext context, IRubyObject[] args) {
459455

460456
fieldDescriptor = descriptor.findFieldByName(methodName);
461457

462-
if (fieldDescriptor != null
463-
&& (!proto3
464-
|| fieldDescriptor.getContainingOneof() == null
465-
|| fieldDescriptor.getContainingOneof().isSynthetic())
466-
&& fieldDescriptor.hasPresence()) {
458+
if (fieldDescriptor != null && fieldDescriptor.hasPresence()) {
467459
return fields.containsKey(fieldDescriptor) ? runtime.getTrue() : runtime.getFalse();
468460
}
469461

ruby/tests/basic.rb

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,10 @@ def test_has_field
119119

120120
m = OneofMessage.new
121121
assert !m.has_my_oneof?
122+
assert !m.has_a?
122123
m.a = "foo"
123124
assert m.has_my_oneof?
124-
assert_raise NoMethodError do
125-
m.has_a?
126-
end
125+
assert m.has_a?
127126
assert_true OneofMessage.descriptor.lookup('a').has?(m)
128127

129128
m = TestSingularFields.new
@@ -716,24 +715,16 @@ def test_map_fields_respond_to? # regression test for issue 9202
716715

717716
def test_oneof_fields_respond_to? # regression test for issue 9202
718717
msg = proto_module::OneofMessage.new
719-
# `has_` prefix + "?" suffix actions should only work for oneofs fields.
718+
# `has_` prefix + "?" suffix actions should work for oneofs fields and members.
720719
assert msg.has_my_oneof?
721720
assert msg.respond_to? :has_my_oneof?
722-
assert !msg.respond_to?( :has_a? )
723-
assert_raise NoMethodError do
724-
msg.has_a?
725-
end
726-
assert !msg.respond_to?( :has_b? )
727-
assert_raise NoMethodError do
728-
msg.has_b?
729-
end
730-
assert !msg.respond_to?( :has_c? )
731-
assert_raise NoMethodError do
732-
msg.has_c?
733-
end
734-
assert !msg.respond_to?( :has_d? )
735-
assert_raise NoMethodError do
736-
msg.has_d?
737-
end
721+
assert msg.respond_to?( :has_a? )
722+
assert !msg.has_a?
723+
assert msg.respond_to?( :has_b? )
724+
assert !msg.has_b?
725+
assert msg.respond_to?( :has_c? )
726+
assert !msg.has_c?
727+
assert msg.respond_to?( :has_d? )
728+
assert !msg.has_d?
738729
end
739730
end

0 commit comments

Comments
 (0)