Pārlūkot izejas kodu

Adds a base class for all explicitly raised TypeErrors (#4255)

* This allows for ruby code to catch and handle Protobuf
    TypeErrors separately from the standard Ruby TypeError

  * Maintains backwards compatibility by having the new
    Google::Protobuf::TypeError inherit from the base
    TypeError. Any code that was catching TypeError should
    continue to work.
Erik Benoist 7 gadi atpakaļ
vecāks
revīzija
74f8e24232

+ 5 - 5
ruby/ext/google/protobuf_c/defs.c

@@ -812,7 +812,7 @@ VALUE FieldDescriptor_submsg_name_set(VALUE _self, VALUE value) {
   upb_fielddef* mut_def = check_field_notfrozen(self->fielddef);
   const char* str = get_str(value);
   if (!upb_fielddef_hassubdef(self->fielddef)) {
-    rb_raise(rb_eTypeError, "FieldDescriptor does not have subdef.");
+    rb_raise(cTypeError, "FieldDescriptor does not have subdef.");
   }
   CHECK_UPB(upb_fielddef_setsubdefname(mut_def, str, &status),
             "Error setting submessage name");
@@ -854,7 +854,7 @@ VALUE FieldDescriptor_get(VALUE _self, VALUE msg_rb) {
   MessageHeader* msg;
   TypedData_Get_Struct(msg_rb, MessageHeader, &Message_type, msg);
   if (msg->descriptor->msgdef != upb_fielddef_containingtype(self->fielddef)) {
-    rb_raise(rb_eTypeError, "get method called on wrong message type");
+    rb_raise(cTypeError, "get method called on wrong message type");
   }
   return layout_get(msg->descriptor->layout, Message_data(msg), self->fielddef);
 }
@@ -872,7 +872,7 @@ VALUE FieldDescriptor_set(VALUE _self, VALUE msg_rb, VALUE value) {
   MessageHeader* msg;
   TypedData_Get_Struct(msg_rb, MessageHeader, &Message_type, msg);
   if (msg->descriptor->msgdef != upb_fielddef_containingtype(self->fielddef)) {
-    rb_raise(rb_eTypeError, "set method called on wrong message type");
+    rb_raise(cTypeError, "set method called on wrong message type");
   }
   layout_set(msg->descriptor->layout, Message_data(msg), self->fielddef, value);
   return Qnil;
@@ -1713,7 +1713,7 @@ static void validate_msgdef(const upb_msgdef* msgdef) {
        upb_msg_field_next(&it)) {
     const upb_fielddef* field = upb_msg_iter_field(&it);
     if (upb_fielddef_label(field) == UPB_LABEL_REQUIRED) {
-      rb_raise(rb_eTypeError, "Required fields are unsupported in proto3.");
+      rb_raise(cTypeError, "Required fields are unsupported in proto3.");
     }
   }
 }
@@ -1723,7 +1723,7 @@ static void validate_enumdef(const upb_enumdef* enumdef) {
   // value.)
   const char* lookup = upb_enumdef_iton(enumdef, 0);
   if (lookup == NULL) {
-    rb_raise(rb_eTypeError,
+    rb_raise(cTypeError,
              "Enum definition does not contain a value for '0'.");
   }
 }

+ 1 - 1
ruby/ext/google/protobuf_c/message.c

@@ -631,7 +631,7 @@ VALUE build_module_from_enumdesc(EnumDescriptor* enumdesc) {
     const char* name = upb_enum_iter_name(&it);
     int32_t value = upb_enum_iter_number(&it);
     if (name[0] < 'A' || name[0] > 'Z') {
-      rb_raise(rb_eTypeError,
+      rb_raise(cTypeError,
                "Enum value '%s' does not start with an uppercase letter "
                "as is required for Ruby constants.",
                name);

+ 2 - 0
ruby/ext/google/protobuf_c/protobuf.c

@@ -41,6 +41,7 @@ VALUE upb_def_to_ruby_obj_map;
 
 VALUE cError;
 VALUE cParseError;
+VALUE cTypeError;
 
 void add_def_obj(const void* def, VALUE value) {
   rb_hash_aset(upb_def_to_ruby_obj_map, ULL2NUM((intptr_t)def), value);
@@ -102,6 +103,7 @@ void Init_protobuf_c() {
 
   cError = rb_const_get(protobuf, rb_intern("Error"));
   cParseError = rb_const_get(protobuf, rb_intern("ParseError"));
+  cTypeError = rb_const_get(protobuf, rb_intern("TypeError"));
 
   rb_define_singleton_method(protobuf, "discard_unknown",
                              Google_Protobuf_discard_unknown, 1);

+ 1 - 0
ruby/ext/google/protobuf_c/protobuf.h

@@ -161,6 +161,7 @@ extern VALUE cBuilder;
 
 extern VALUE cError;
 extern VALUE cParseError;
+extern VALUE cTypeError;
 
 // We forward-declare all of the Ruby method implementations here because we
 // sometimes call the methods directly across .c files, rather than going

+ 17 - 17
ruby/ext/google/protobuf_c/storage.c

@@ -96,7 +96,7 @@ static bool is_ruby_num(VALUE value) {
 
 void native_slot_check_int_range_precision(upb_fieldtype_t type, VALUE val) {
   if (!is_ruby_num(val)) {
-    rb_raise(rb_eTypeError, "Expected number type for integral field.");
+    rb_raise(cTypeError, "Expected number type for integral field.");
   }
 
   // NUM2{INT,UINT,LL,ULL} macros do the appropriate range checks on upper
@@ -153,13 +153,13 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class,
   switch (type) {
     case UPB_TYPE_FLOAT:
       if (!is_ruby_num(value)) {
-        rb_raise(rb_eTypeError, "Expected number type for float field.");
+        rb_raise(cTypeError, "Expected number type for float field.");
       }
       DEREF(memory, float) = NUM2DBL(value);
       break;
     case UPB_TYPE_DOUBLE:
       if (!is_ruby_num(value)) {
-        rb_raise(rb_eTypeError, "Expected number type for double field.");
+        rb_raise(cTypeError, "Expected number type for double field.");
       }
       DEREF(memory, double) = NUM2DBL(value);
       break;
@@ -170,7 +170,7 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class,
       } else if (value == Qfalse) {
         val = 0;
       } else {
-        rb_raise(rb_eTypeError, "Invalid argument for boolean field.");
+        rb_raise(cTypeError, "Invalid argument for boolean field.");
       }
       DEREF(memory, int8_t) = val;
       break;
@@ -179,7 +179,7 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class,
       if (CLASS_OF(value) == rb_cSymbol) {
         value = rb_funcall(value, rb_intern("to_s"), 0, NULL);
       } else if (CLASS_OF(value) != rb_cString) {
-        rb_raise(rb_eTypeError, "Invalid argument for string field.");
+        rb_raise(cTypeError, "Invalid argument for string field.");
       }
 
       DEREF(memory, VALUE) = native_slot_encode_and_freeze_string(type, value);
@@ -187,7 +187,7 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class,
 
     case UPB_TYPE_BYTES: {
       if (CLASS_OF(value) != rb_cString) {
-        rb_raise(rb_eTypeError, "Invalid argument for string field.");
+        rb_raise(cTypeError, "Invalid argument for string field.");
       }
 
       DEREF(memory, VALUE) = native_slot_encode_and_freeze_string(type, value);
@@ -197,7 +197,7 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class,
       if (CLASS_OF(value) == CLASS_OF(Qnil)) {
         value = Qnil;
       } else if (CLASS_OF(value) != type_class) {
-        rb_raise(rb_eTypeError,
+        rb_raise(cTypeError,
                  "Invalid type %s to assign to submessage field.",
                  rb_class2name(CLASS_OF(value)));
       }
@@ -209,7 +209,7 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class,
       if (TYPE(value) == T_STRING) {
         value = rb_funcall(value, rb_intern("to_sym"), 0, NULL);
       } else if (!is_ruby_num(value) && TYPE(value) != T_SYMBOL) {
-        rb_raise(rb_eTypeError,
+        rb_raise(cTypeError,
                  "Expected number or symbol type for enum field.");
       }
       if (TYPE(value) == T_SYMBOL) {
@@ -598,18 +598,18 @@ static void check_repeated_field_type(VALUE val, const upb_fielddef* field) {
 
   if (!RB_TYPE_P(val, T_DATA) || !RTYPEDDATA_P(val) ||
       RTYPEDDATA_TYPE(val) != &RepeatedField_type) {
-    rb_raise(rb_eTypeError, "Expected repeated field array");
+    rb_raise(cTypeError, "Expected repeated field array");
   }
 
   self = ruby_to_RepeatedField(val);
   if (self->field_type != upb_fielddef_type(field)) {
-    rb_raise(rb_eTypeError, "Repeated field array has wrong element type");
+    rb_raise(cTypeError, "Repeated field array has wrong element type");
   }
 
-  if (self->field_type == UPB_TYPE_MESSAGE) { 
+  if (self->field_type == UPB_TYPE_MESSAGE) {
     if (self->field_type_class !=
         Descriptor_msgclass(get_def_obj(upb_fielddef_subdef(field)))) {
-      rb_raise(rb_eTypeError,
+      rb_raise(cTypeError,
                "Repeated field array has wrong message class");
     }
   }
@@ -618,7 +618,7 @@ static void check_repeated_field_type(VALUE val, const upb_fielddef* field) {
   if (self->field_type == UPB_TYPE_ENUM) {
     if (self->field_type_class !=
         EnumDescriptor_enummodule(get_def_obj(upb_fielddef_subdef(field)))) {
-      rb_raise(rb_eTypeError,
+      rb_raise(cTypeError,
                "Repeated field array has wrong enum class");
     }
   }
@@ -631,21 +631,21 @@ static void check_map_field_type(VALUE val, const upb_fielddef* field) {
 
   if (!RB_TYPE_P(val, T_DATA) || !RTYPEDDATA_P(val) ||
       RTYPEDDATA_TYPE(val) != &Map_type) {
-    rb_raise(rb_eTypeError, "Expected Map instance");
+    rb_raise(cTypeError, "Expected Map instance");
   }
 
   self = ruby_to_Map(val);
   if (self->key_type != upb_fielddef_type(key_field)) {
-    rb_raise(rb_eTypeError, "Map key type does not match field's key type");
+    rb_raise(cTypeError, "Map key type does not match field's key type");
   }
   if (self->value_type != upb_fielddef_type(value_field)) {
-    rb_raise(rb_eTypeError, "Map value type does not match field's value type");
+    rb_raise(cTypeError, "Map value type does not match field's value type");
   }
   if (upb_fielddef_type(value_field) == UPB_TYPE_MESSAGE ||
       upb_fielddef_type(value_field) == UPB_TYPE_ENUM) {
     if (self->value_type_class !=
         get_def_obj(upb_fielddef_subdef(value_field))) {
-      rb_raise(rb_eTypeError,
+      rb_raise(cTypeError,
                "Map value type has wrong message/enum class");
     }
   }

+ 1 - 0
ruby/lib/google/protobuf.rb

@@ -37,6 +37,7 @@ module Google
   module Protobuf
     class Error < StandardError; end
     class ParseError < Error; end
+    class TypeError < ::TypeError; end
   end
 end
 

+ 26 - 21
ruby/tests/basic.rb

@@ -283,31 +283,36 @@ module BasicTest
 
     def test_type_errors
       m = TestMessage.new
-      assert_raise TypeError do
+      e = assert_raise Google::Protobuf::TypeError do
         m.optional_int32 = "hello"
       end
-      assert_raise TypeError do
+
+      # Google::Protobuf::TypeError should inherit from TypeError for backwards compatibility
+      # TODO: This can be removed when we can safely migrate to Google::Protobuf::TypeError
+      assert_true e.is_a?(::TypeError)
+
+      assert_raise Google::Protobuf::TypeError do
         m.optional_string = 42
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.optional_string = nil
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.optional_bool = 42
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.optional_msg = TestMessage.new  # expects TestMessage2
       end
 
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.repeated_int32 = []  # needs RepeatedField
       end
 
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.repeated_int32.push "hello"
       end
 
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.repeated_msg.push TestMessage.new
       end
     end
@@ -373,7 +378,7 @@ module BasicTest
       assert l.pop == 9
       assert l == [5, 2, 3, 4, 7, 8]
 
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m = TestMessage.new
         l.push m
       end
@@ -423,10 +428,10 @@ module BasicTest
       l = Google::Protobuf::RepeatedField.new(:message, TestMessage)
       l.push TestMessage.new
       assert l.count == 1
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         l.push TestMessage2.new
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         l.push 42
       end
 
@@ -575,7 +580,7 @@ module BasicTest
       assert_raise RangeError do
         m[0x8000_0000] = 1
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m["asdf"] = 1
       end
 
@@ -584,7 +589,7 @@ module BasicTest
       assert_raise RangeError do
         m[0x1_0000_0000_0000_0000] = 1
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m["asdf"] = 1
       end
 
@@ -609,10 +614,10 @@ module BasicTest
       m = Google::Protobuf::Map.new(:bool, :int32)
       m[true] = 1
       m[false] = 2
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m[1] = 1
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m["asdf"] = 1
       end
 
@@ -639,7 +644,7 @@ module BasicTest
     def test_map_msg_enum_valuetypes
       m = Google::Protobuf::Map.new(:string, :message, TestMessage)
       m["asdf"] = TestMessage.new
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m["jkl;"] = TestMessage2.new
       end
 
@@ -706,17 +711,17 @@ module BasicTest
       m.map_string_msg.delete("c")
       assert m.map_string_msg == { "a" => TestMessage2.new(:foo => 1) }
 
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.map_string_msg["e"] = TestMessage.new # wrong value type
       end
       # ensure nothing was added by the above
       assert m.map_string_msg == { "a" => TestMessage2.new(:foo => 1) }
 
       m.map_string_int32 = Google::Protobuf::Map.new(:string, :int32)
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.map_string_int32 = Google::Protobuf::Map.new(:string, :int64)
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         m.map_string_int32 = {}
       end
 
@@ -1015,7 +1020,7 @@ module BasicTest
 
     def test_def_errors
       s = Google::Protobuf::DescriptorPool.new
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         s.build do
           # enum with no default (integer value 0)
           add_enum "MyEnum" do
@@ -1023,7 +1028,7 @@ module BasicTest
           end
         end
       end
-      assert_raise TypeError do
+      assert_raise Google::Protobuf::TypeError do
         s.build do
           # message with required field (unsupported in proto3)
           add_message "MyMessage" do