From 7e250daccecd16ef8dd1401fd1c61dc5c9685509 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 1 Aug 2017 19:36:26 +0200 Subject: [PATCH] Enhance error messages --- src/conversion.rs | 85 +++++++++++++++++++++++++++++++---------------- src/error.rs | 46 ++++++++++++++++++++----- src/lua.rs | 82 ++++++++++++++++++++++++++++++++++----------- src/util.rs | 5 +-- 4 files changed, 157 insertions(+), 61 deletions(-) diff --git a/src/conversion.rs b/src/conversion.rs index bd5cb76..0c9574b 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -39,9 +39,12 @@ impl<'lua> FromLua<'lua> for Table<'lua> { fn from_lua(value: Value<'lua>, _: &'lua Lua) -> Result> { match value { Value::Table(table) => Ok(table), - _ => Err(Error::FromLuaConversionError( - "cannot convert lua value to table".to_owned(), - )), + _ => Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "table", + expected: None, + message: None, + }), } } } @@ -56,9 +59,12 @@ impl<'lua> FromLua<'lua> for Function<'lua> { fn from_lua(value: Value<'lua>, _: &'lua Lua) -> Result> { match value { Value::Function(table) => Ok(table), - _ => Err(Error::FromLuaConversionError( - "cannot convert lua value to function".to_owned(), - )), + _ => Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "function", + expected: None, + message: None, + }), } } } @@ -73,9 +79,12 @@ impl<'lua> FromLua<'lua> for Thread<'lua> { fn from_lua(value: Value<'lua>, _: &'lua Lua) -> Result> { match value { Value::Thread(t) => Ok(t), - _ => Err(Error::FromLuaConversionError( - "cannot convert lua value to thread".to_owned(), - )), + _ => Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "thread", + expected: None, + message: None, + }), } } } @@ -90,9 +99,12 @@ impl<'lua> FromLua<'lua> for AnyUserData<'lua> { fn from_lua(value: Value<'lua>, _: &'lua Lua) -> Result> { match value { Value::UserData(ud) => Ok(ud), - _ => Err(Error::FromLuaConversionError( - "cannot convert lua value to userdata".to_owned(), - )), + _ => Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "userdata", + expected: None, + message: None, + }), } } } @@ -107,9 +119,12 @@ impl<'lua, T: UserData + Clone> FromLua<'lua> for T { fn from_lua(value: Value<'lua>, _: &'lua Lua) -> Result { match value { Value::UserData(ud) => Ok(ud.borrow::()?.clone()), - _ => Err(Error::FromLuaConversionError( - "cannot convert lua value to userdata".to_owned(), - )), + _ => Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "userdata", + expected: None, + message: None, + }), } } } @@ -156,12 +171,15 @@ impl<'lua> ToLua<'lua> for LightUserData { } impl<'lua> FromLua<'lua> for LightUserData { - fn from_lua(v: Value, _: &'lua Lua) -> Result { - match v { + fn from_lua(value: Value, _: &'lua Lua) -> Result { + match value { Value::LightUserData(ud) => Ok(ud), - _ => Err(Error::FromLuaConversionError( - "cannot convert lua value to lightuserdata".to_owned(), - )), + _ => Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "light userdata", + expected: None, + message: None, + }), } } } @@ -241,9 +259,12 @@ impl<'lua, T: FromLua<'lua>> FromLua<'lua> for Vec { if let Value::Table(table) = value { table.sequence_values().collect() } else { - Err(Error::FromLuaConversionError( - "cannot convert lua value to table for Vec".to_owned(), - )) + Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "Vec", + expected: Some("table"), + message: None, + }) } } } @@ -259,9 +280,12 @@ impl<'lua, K: Eq + Hash + FromLua<'lua>, V: FromLua<'lua>> FromLua<'lua> for Has if let Value::Table(table) = value { table.pairs().collect() } else { - Err(Error::FromLuaConversionError( - "cannot convert lua value to table for HashMap".to_owned(), - )) + Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "HashMap", + expected: Some("table"), + message: None, + }) } } } @@ -277,9 +301,12 @@ impl<'lua, K: Ord + FromLua<'lua>, V: FromLua<'lua>> FromLua<'lua> for BTreeMap< if let Value::Table(table) = value { table.pairs().collect() } else { - Err(Error::FromLuaConversionError( - "cannot convert lua value to table for BTreeMap".to_owned(), - )) + Err(Error::FromLuaConversionError { + from: value.type_name(), + to: "BTreeMap", + expected: Some("table"), + message: None, + }) } } } diff --git a/src/error.rs b/src/error.rs index 7e3161b..c8c5c63 100644 --- a/src/error.rs +++ b/src/error.rs @@ -22,9 +22,28 @@ pub enum Error { /// instead of invoking the error handler. ErrorError(String), /// A Rust value could not be converted to a Lua value. - ToLuaConversionError(String), + ToLuaConversionError { + /// Name of the Rust type that could not be converted. + from: &'static str, + /// Name of the Lua type that could not be created. + to: &'static str, + /// A message indicating why the conversion failed in more detail. + message: Option, + }, /// A Lua value could not be converted to the expected Rust type. - FromLuaConversionError(String), + FromLuaConversionError { + /// Name of the Lua type that could not be converted. + from: &'static str, + /// Name of the Rust type that could not be created. + to: &'static str, + /// A string indicating the possible Lua values/types for this conversion. + /// + /// To avoid redundancy, this should only be set to `Some` when there are nontrivial rules + /// about valid conversions, since the `to` string should already hint at the problem. + expected: Option<&'static str>, + /// A string containing more detailed error information. + message: Option, + }, /// [`Thread::resume`] was called on an inactive coroutine. /// /// A coroutine is inactive if its main function has returned or if an error has occured inside @@ -89,11 +108,22 @@ impl fmt::Display for Error { } Error::RuntimeError(ref msg) => write!(fmt, "Lua runtime error: {}", msg), Error::ErrorError(ref msg) => write!(fmt, "Lua error in error handler: {}", msg), - Error::ToLuaConversionError(ref msg) => { - write!(fmt, "Error converting rust type to lua: {}", msg) + Error::ToLuaConversionError { from, to, ref message } => { + write!(fmt, "error converting {} to Lua {}", from, to)?; + match *message { + None => Ok(()), + Some(ref message) => write!(fmt, " ({})", message), + } } - Error::FromLuaConversionError(ref msg) => { - write!(fmt, "Error converting lua type to rust: {}", msg) + Error::FromLuaConversionError { from, to, ref expected, ref message } => { + write!(fmt, "error converting Lua {} to {}", from, to)?; + match (expected.as_ref(), message.as_ref()) { + (None, None) => Ok(()), + (None, Some(ref message)) => write!(fmt, " ({})", message), + (Some(ref expected), None) => write!(fmt, " (expected {})", expected), + (Some(ref expected), Some(ref message)) => + write!(fmt, " ({}; expected {})", message, expected), + } } Error::CoroutineInactive => write!(fmt, "Cannot resume inactive coroutine"), Error::UserDataTypeMismatch => write!(fmt, "Userdata not expected type"), @@ -112,8 +142,8 @@ impl StdError for Error { Error::IncompleteStatement(_) => "lua incomplete statement", Error::RuntimeError(_) => "lua runtime error", Error::ErrorError(_) => "lua error handling error", - Error::ToLuaConversionError(_) => "conversion error to lua", - Error::FromLuaConversionError(_) => "conversion error from lua", + Error::ToLuaConversionError { .. } => "conversion error to lua", + Error::FromLuaConversionError { .. } => "conversion error from lua", Error::CoroutineInactive => "lua coroutine inactive", Error::UserDataTypeMismatch => "lua userdata type mismatch", Error::UserDataBorrowError => "lua userdata already mutably borrowed", diff --git a/src/lua.rs b/src/lua.rs index a121854..359babc 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -47,6 +47,24 @@ pub enum Value<'lua> { } pub use self::Value::Nil; +impl<'lua> Value<'lua> { + pub(crate) fn type_name(&self) -> &'static str { + match *self { + Value::Nil => "nil", + Value::Boolean(_) => "boolean", + Value::LightUserData(_) => "light userdata", + Value::Integer(_) => "integer", + Value::Number(_) => "number", + Value::String(_) => "string", + Value::Table(_) => "table", + Value::Function(_) => "function", + Value::Thread(_) => "thread", + Value::UserData(_) => "userdata", + Value::Error(_) => "userdata", + } + } +} + /// Trait for types convertible to `Value`. pub trait ToLua<'a> { /// Performs the conversion. @@ -190,7 +208,12 @@ impl<'lua> String<'lua> { /// # } /// ``` pub fn to_str(&self) -> Result<&str> { - str::from_utf8(self.as_bytes()).map_err(|e| Error::FromLuaConversionError(e.to_string())) + str::from_utf8(self.as_bytes()).map_err(|e| Error::FromLuaConversionError { + from: "string", + to: "&str", + expected: Some("utf-8 string"), + message: Some(e.to_string()), + }) } /// Get the bytes that make up this string. @@ -1084,10 +1107,12 @@ impl<'lua, T: UserData> UserDataMethods<'lua, T> { method(lua, &userdata, A::from_lua_multi(args, lua)?)? .to_lua_multi(lua) } else { - Err(Error::FromLuaConversionError( - "No userdata supplied as first argument to method" - .to_owned(), - )) + Err(Error::FromLuaConversionError { + from: "missing argument", + to: "UserData", + expected: Some("userdata"), + message: None, + }) }) } @@ -1103,11 +1128,12 @@ impl<'lua, T: UserData> UserDataMethods<'lua, T> { method(lua, &mut userdata, A::from_lua_multi(args, lua)?)? .to_lua_multi(lua) } else { - Err( - Error::FromLuaConversionError( - "No userdata supplied as first argument to method".to_owned(), - ).into(), - ) + Err(Error::FromLuaConversionError { + from: "missing argument", + to: "UserData", + expected: Some("userdata"), + message: None, + }) }) } } @@ -1385,7 +1411,11 @@ impl Lua { self.state, if let Some(name) = name { let name = CString::new(name.to_owned()).map_err(|e| { - Error::ToLuaConversionError(e.to_string()) + Error::ToLuaConversionError { + from: "&str", + to: "string", + message: Some(e.to_string()), + } })?; ffi::luaL_loadbuffer( self.state, @@ -1606,12 +1636,16 @@ impl Lua { v => unsafe { stack_guard(self.state, 0, || { check_stack(self.state, 1); + let ty = v.type_name(); self.push_value(self.state, v); if ffi::lua_tostring(self.state, -1).is_null() { ffi::lua_pop(self.state, 1); - Err(Error::FromLuaConversionError( - "cannot convert lua value to string".to_owned(), - )) + Err(Error::FromLuaConversionError { + from: ty, + to: "String", + expected: Some("string or number"), + message: None, + }) } else { Ok(String(self.pop_ref(self.state))) } @@ -1630,14 +1664,18 @@ impl Lua { v => unsafe { stack_guard(self.state, 0, || { check_stack(self.state, 1); + let ty = v.type_name(); self.push_value(self.state, v); let mut isint = 0; let i = ffi::lua_tointegerx(self.state, -1, &mut isint); ffi::lua_pop(self.state, 1); if isint == 0 { - Err(Error::FromLuaConversionError( - "cannot convert lua value to integer".to_owned(), - )) + Err(Error::FromLuaConversionError { + from: ty, + to: "integer", + expected: None, + message: None, + }) } else { Ok(i) } @@ -1656,14 +1694,18 @@ impl Lua { v => unsafe { stack_guard(self.state, 0, || { check_stack(self.state, 1); + let ty = v.type_name(); self.push_value(self.state, v); let mut isnum = 0; let n = ffi::lua_tonumberx(self.state, -1, &mut isnum); ffi::lua_pop(self.state, 1); if isnum == 0 { - Err(Error::FromLuaConversionError( - "cannot convert lua value to number".to_owned(), - )) + Err(Error::FromLuaConversionError { + from: ty, + to: "number", + expected: Some("number or string coercible to number"), + message: None, + }) } else { Ok(n) } diff --git a/src/util.rs b/src/util.rs index 2cc56ed..d731df6 100644 --- a/src/util.rs +++ b/src/util.rs @@ -501,10 +501,7 @@ pub unsafe fn push_wrapped_error(state: *mut ffi::lua_State, err: Error) { Ok(1) } else { - Err(Error::FromLuaConversionError( - "internal error: userdata mismatch in Error metamethod" - .to_owned(), - )) + panic!("internal error: userdata mismatch in Error metamethod"); }) }