diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index ebe2f1ab49..283939c9df 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -166,7 +166,7 @@ VALUE Message_method_missing(int argc, VALUE* argv, VALUE _self) { name, name_len); if (f == NULL) { - rb_raise(rb_eArgError, "Unknown field"); + return rb_call_super(argc, argv); } if (setter) { @@ -197,7 +197,7 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { f = upb_msgdef_ntofz(self->descriptor->msgdef, name); if (f == NULL) { rb_raise(rb_eArgError, - "Unknown field name in initialization map entry."); + "Unknown field name '%s' in initialization map entry.", name); } if (is_map_field(f)) { @@ -205,7 +205,7 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { if (TYPE(val) != T_HASH) { rb_raise(rb_eArgError, - "Expected Hash object as initializer value for map field."); + "Expected Hash object as initializer value for map field '%s'.", name); } map = layout_get(self->descriptor->layout, Message_data(self), f); Map_merge_into_self(map, val); @@ -214,7 +214,7 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { if (TYPE(val) != T_ARRAY) { rb_raise(rb_eArgError, - "Expected array as initializer value for repeated field."); + "Expected array as initializer value for repeated field '%s'.", name); } ary = layout_get(self->descriptor->layout, Message_data(self), f); for (int i = 0; i < RARRAY_LEN(val); i++) { 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 547ab22cb2..39213c4d16 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -86,14 +86,14 @@ public class RubyMessage extends RubyObject { if (Utils.isMapEntry(fieldDescriptor)) { if (!(value instanceof RubyHash)) - throw runtime.newArgumentError("Expected Hash object as initializer value for map field."); + throw runtime.newArgumentError("Expected Hash object as initializer value for map field '" + key.asJavaString() + "'."); final RubyMap map = newMapForField(context, fieldDescriptor); map.mergeIntoSelf(context, value); maps.put(fieldDescriptor, map); } else if (fieldDescriptor.isRepeated()) { if (!(value instanceof RubyArray)) - throw runtime.newTypeError("Expected array as initializer var for repeated field."); + throw runtime.newArgumentError("Expected array as initializer value for repeated field '" + key.asJavaString() + "'."); RubyRepeatedField repeatedField = rubyToRepeatedField(context, fieldDescriptor, value); addRepeatedField(fieldDescriptor, repeatedField); } else { @@ -217,6 +217,9 @@ public class RubyMessage extends RubyObject { RubyDescriptor rubyDescriptor = (RubyDescriptor) getDescriptor(context, metaClass); IRubyObject oneofDescriptor = rubyDescriptor.lookupOneof(context, args[0]); if (oneofDescriptor.isNil()) { + if (!hasField(args[0])) { + return Helpers.invokeSuper(context, this, metaClass, "method_missing", args, Block.NULL_BLOCK); + } return index(context, args[0]); } RubyOneofDescriptor rubyOneofDescriptor = (RubyOneofDescriptor) oneofDescriptor; @@ -233,6 +236,10 @@ public class RubyMessage extends RubyObject { if (field.end_with_p(context, equalSign).isTrue()) { field.chomp_bang(context, equalSign); } + + if (!hasField(field)) { + return Helpers.invokeSuper(context, this, metaClass, "method_missing", args, Block.NULL_BLOCK); + } return indexSet(context, field, args[1]); } } @@ -435,6 +442,11 @@ public class RubyMessage extends RubyObject { return ret; } + private boolean hasField(IRubyObject fieldName) { + String nameStr = fieldName.asJavaString(); + return this.descriptor.findFieldByName(Utils.escapeIdentifier(nameStr)) != null; + } + private void checkRepeatedFieldType(ThreadContext context, IRubyObject value, Descriptors.FieldDescriptor fieldDescriptor) { Ruby runtime = context.runtime; diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 40c200786b..da85520fde 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -191,6 +191,35 @@ module BasicTest assert m1.hash != m2.hash end + def test_unknown_field_errors + e = assert_raise NoMethodError do + TestMessage.new.hello + end + assert_match(/hello/, e.message) + + e = assert_raise NoMethodError do + TestMessage.new.hello = "world" + end + assert_match(/hello/, e.message) + end + + def test_initialization_map_errors + e = assert_raise ArgumentError do + TestMessage.new(:hello => "world") + end + assert_match(/hello/, e.message) + + e = assert_raise ArgumentError do + MapMessage.new(:map_string_int32 => "hello") + end + assert_equal e.message, "Expected Hash object as initializer value for map field 'map_string_int32'." + + e = assert_raise ArgumentError do + TestMessage.new(:repeated_uint32 => "hello") + end + assert_equal e.message, "Expected array as initializer value for repeated field 'repeated_uint32'." + end + def test_type_errors m = TestMessage.new assert_raise TypeError do