rb: Display field name in type-checking error [THRIFT-78]

Author: Kevin Ballard <kevin@rapleaf.com>

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@678059 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/lib/rb/lib/thrift/struct.rb b/lib/rb/lib/thrift/struct.rb
index d8e8f65..10f2ea0 100644 (file)
--- a/lib/rb/lib/thrift/struct.rb
+++ b/lib/rb/lib/thrift/struct.rb
@@ -6,7 +6,7 @@ module Thrift
     def initialize(d={})
       each_field do |fid, type, name, default|
         value = d.delete(name.to_s) { d.delete(name.to_sym) { default.dup rescue default } }
-        Thrift.check_type(value, type) if Thrift.type_checking
+        Thrift.check_type(value, type, name) if Thrift.type_checking
         instance_variable_set("@#{name}", value)
       end
       raise Exception, "Unknown keys given to #{self.class}.new: #{d.keys.join(", ")}" unless d.empty?
@@ -72,7 +72,7 @@ module Thrift
       fields.each do |field|
         klass.send :attr_reader, field
         klass.send :define_method, "#{field}=" do |value|
-          Thrift.check_type(value, klass::FIELDS.values.find { |f| f[:name].to_s == field.to_s }[:type] ) if Thrift.type_checking
+          Thrift.check_type(value, klass::FIELDS.values.find { |f| f[:name].to_s == field.to_s }[:type], field) if Thrift.type_checking
           instance_variable_set("@#{field}", value)
         end
       end

diff --git a/lib/rb/lib/thrift/types.rb b/lib/rb/lib/thrift/types.rb
index 6666a48..28fdae2 100644 (file)
--- a/lib/rb/lib/thrift/types.rb
+++ b/lib/rb/lib/thrift/types.rb
@@ -25,7 +25,7 @@ module Thrift
   class TypeError < Exception
   end
 
-  def self.check_type(value, type)
+  def self.check_type(value, type, name)
     return if value.nil?
     klasses = case type
               when Types::VOID
@@ -48,7 +48,7 @@ module Thrift
                 Array
               end
     valid = klasses && [*klasses].any? { |klass| klass === value }
-    raise TypeError, "Expected #{type_name(type)}, received #{value.class}" unless valid
+    raise TypeError, "Expected #{type_name(type)}, received #{value.class} for field #{name}" unless valid
   end
 
   def self.type_name(type)

diff --git a/lib/rb/spec/struct_spec.rb b/lib/rb/spec/struct_spec.rb
index eb0542b..2245778 100644 (file)
--- a/lib/rb/spec/struct_spec.rb
+++ b/lib/rb/spec/struct_spec.rb
@@ -177,7 +177,7 @@ class ThriftStructSpec < Spec::ExampleGroup
     it "should support optional type-checking in Thrift::Struct.new" do
       Thrift.type_checking = true
       begin
-        lambda { Hello.new(:greeting => 3) }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum")
+        lambda { Hello.new(:greeting => 3) }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum for field greeting")
       ensure
         Thrift.type_checking = false
       end
@@ -188,7 +188,7 @@ class ThriftStructSpec < Spec::ExampleGroup
       Thrift.type_checking = true
       begin
         hello = Hello.new
-        lambda { hello.greeting = 3 }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum")
+        lambda { hello.greeting = 3 }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum for field greeting")
       ensure
         Thrift.type_checking = false
       end

diff --git a/lib/rb/spec/types_spec.rb b/lib/rb/spec/types_spec.rb
index 564d9a6..4b3d7ea 100644 (file)
--- a/lib/rb/spec/types_spec.rb
+++ b/lib/rb/spec/types_spec.rb
@@ -16,31 +16,40 @@ class ThriftTypesSpec < Spec::ExampleGroup
       Thrift.type_checking = true
       begin
         # lambda { Thrift.check_type(nil, Types::STOP) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::STOP) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(nil, Types::VOID) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::VOID) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(true, Types::BOOL) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::BOOL) }.should raise_error(TypeError)
-        # lambda { Thrift.check_type(nil, Types::BOOL) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::BYTE) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::I16) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::I32) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(42, Types::I64) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3.14, Types::I32) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(3.14, Types::DOUBLE) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::DOUBLE) }.should raise_error(TypeError)
-        lambda { Thrift.check_type("3", Types::STRING) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type(3, Types::STRING) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(3, Types::STOP, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(nil, Types::VOID, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(3, Types::VOID, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(true, Types::BOOL, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(3, Types::BOOL, :foo) }.should raise_error(TypeError)
+        # lambda { Thrift.check_type(nil, Types::BOOL, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(42, Types::BYTE, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(42, Types::I16, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(42, Types::I32, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(42, Types::I64, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(3.14, Types::I32, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(3.14, Types::DOUBLE, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(3, Types::DOUBLE, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type("3", Types::STRING, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type(3, Types::STRING, :foo) }.should raise_error(TypeError)
         hello = SpecNamespace::Hello.new
-        lambda { Thrift.check_type(hello, Types::STRUCT) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type("foo", Types::STRUCT) }.should raise_error(TypeError)
-        lambda { Thrift.check_type({:foo => 1}, Types::MAP) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type([1], Types::MAP) }.should raise_error(TypeError)
-        lambda { Thrift.check_type([1], Types::LIST) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type({:foo => 1}, Types::LIST) }.should raise_error(TypeError)
-        lambda { Thrift.check_type(Set.new([1,2]), Types::SET) }.should_not raise_error(TypeError)
-        lambda { Thrift.check_type([1,2], Types::SET) }.should raise_error(TypeError)
-        lambda { Thrift.check_type({:foo => true}, Types::SET) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(hello, Types::STRUCT, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type("foo", Types::STRUCT, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type({:foo => 1}, Types::MAP, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type([1], Types::MAP, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type([1], Types::LIST, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type({:foo => 1}, Types::LIST, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type(Set.new([1,2]), Types::SET, :foo) }.should_not raise_error(TypeError)
+        lambda { Thrift.check_type([1,2], Types::SET, :foo) }.should raise_error(TypeError)
+        lambda { Thrift.check_type({:foo => true}, Types::SET, :foo) }.should raise_error(TypeError)
+      ensure
+        Thrift.type_checking = false
+      end
+    end
+
+    it "should give the TypeError a readable message" do
+      Thrift.type_checking = true
+      begin
+        lambda { Thrift.check_type(3, Types::STRING, :foo) }.should raise_error(TypeError, "Expected Types::STRING, received Fixnum for field foo")
       ensure
         Thrift.type_checking = false
       end