From 4cf99ea2a2a84b8a25822a26404af61f29ba87b5 Mon Sep 17 00:00:00 2001 From: Guillem Nieto Date: Sun, 5 May 2019 23:53:30 +0200 Subject: Remove ValueWithKey struct While using this library, I end up having lifetime issues with `Config::get`. I've seen that current implementation forces the calleer to match `key` lifetime to the output of the function. My use case is, under some circumstances, return a suffixed version of the config key. Something similar to: ``` if some_condition == true { let key_name = format!("{}_suffix", key); self.config.get(&key_name) } else { self.config.get(key) } ``` This code is noy compiling for me due to conflicting lifetimes. To avoid this, I've started looking to the code and I've found that `key` needed this lifetime because of `ValueWithKey`. The purpouse of this struct seems to be add more information to the errors that are returned to the user. To mitigate this lifetime coupling I've: - Mapped the error on `Config::get` to include the originating key of the current error - Remove all the code related with `ValueWithKey` --- src/config.rs | 7 ++-- src/de.rs | 128 +--------------------------------------------------------- src/ser.rs | 2 +- src/value.rs | 55 ------------------------- 4 files changed, 6 insertions(+), 186 deletions(-) diff --git a/src/config.rs b/src/config.rs index 2ce73f1..7e031d0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -10,7 +10,7 @@ use ser::ConfigSerializer; use source::Source; use path; -use value::{Table, Value, ValueKind, ValueWithKey}; +use value::{Table, Value, ValueKind}; #[derive(Clone, Debug)] enum ConfigKind { @@ -151,7 +151,7 @@ impl Config { self.refresh() } - pub fn get<'de, T: Deserialize<'de>>(&self, key: &'de str) -> Result { + pub fn get<'de, T: Deserialize<'de>>(&self, key: &str) -> Result { // Parse the key into a path expression let expr: path::Expression = key.to_lowercase().parse()?; @@ -161,7 +161,8 @@ impl Config { match value { Some(value) => { // Deserialize the received value into the requested type - T::deserialize(ValueWithKey::new(value, key)) + T::deserialize(value) + .map_err(|e| e.extend_with_key(key)) } None => Err(ConfigError::NotFound(key.into())), diff --git a/src/de.rs b/src/de.rs index 9267338..9611a21 100644 --- a/src/de.rs +++ b/src/de.rs @@ -3,133 +3,7 @@ use error::*; use serde::de; use std::collections::{HashMap, VecDeque}; use std::iter::Enumerate; -use value::{Value, ValueKind, ValueWithKey, Table}; - -// TODO: Use a macro or some other magic to reduce the code duplication here - -impl<'de> de::Deserializer<'de> for ValueWithKey<'de> { - type Error = ConfigError; - - #[inline] - fn deserialize_any(self, visitor: V) -> Result - where - V: de::Visitor<'de>, - { - // Deserialize based on the underlying type - match self.0.kind { - ValueKind::Nil => visitor.visit_unit(), - ValueKind::Integer(i) => visitor.visit_i64(i), - ValueKind::Boolean(b) => visitor.visit_bool(b), - ValueKind::Float(f) => visitor.visit_f64(f), - ValueKind::String(s) => visitor.visit_string(s), - ValueKind::Array(values) => visitor.visit_seq(SeqAccess::new(values)), - ValueKind::Table(map) => visitor.visit_map(MapAccess::new(map)), - } - } - - #[inline] - fn deserialize_bool>(self, visitor: V) -> Result { - visitor.visit_bool(self.into_bool()?) - } - - #[inline] - fn deserialize_i8>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_i8(self.into_int()? as i8) - } - - #[inline] - fn deserialize_i16>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_i16(self.into_int()? as i16) - } - - #[inline] - fn deserialize_i32>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_i32(self.into_int()? as i32) - } - - #[inline] - fn deserialize_i64>(self, visitor: V) -> Result { - visitor.visit_i64(self.into_int()?) - } - - #[inline] - fn deserialize_u8>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_u8(self.into_int()? as u8) - } - - #[inline] - fn deserialize_u16>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_u16(self.into_int()? as u16) - } - - #[inline] - fn deserialize_u32>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_u32(self.into_int()? as u32) - } - - #[inline] - fn deserialize_u64>(self, visitor: V) -> Result { - // FIXME: This should *fail* if the value does not fit in the requets integer type - visitor.visit_u64(self.into_int()? as u64) - } - - #[inline] - fn deserialize_f32>(self, visitor: V) -> Result { - visitor.visit_f32(self.into_float()? as f32) - } - - #[inline] - fn deserialize_f64>(self, visitor: V) -> Result { - visitor.visit_f64(self.into_float()?) - } - - #[inline] - fn deserialize_str>(self, visitor: V) -> Result { - visitor.visit_string(self.into_str()?) - } - - #[inline] - fn deserialize_string>(self, visitor: V) -> Result { - visitor.visit_string(self.into_str()?) - } - - #[inline] - fn deserialize_option(self, visitor: V) -> Result - where - V: de::Visitor<'de>, - { - // Match an explicit nil as None and everything else as Some - match self.0.kind { - ValueKind::Nil => visitor.visit_none(), - _ => visitor.visit_some(self), - } - } - - fn deserialize_enum( - self, - name: &'static str, - variants: &'static [&'static str], - visitor: V, - ) -> Result - where - V: de::Visitor<'de>, - { - // FIXME: find a way to extend_with_key - visitor.visit_enum(EnumAccess{ value: self.0, name: name, variants: variants }) - } - - forward_to_deserialize_any! { - char seq - bytes byte_buf map struct unit newtype_struct - identifier ignored_any unit_struct tuple_struct tuple - } -} +use value::{Value, ValueKind, Table}; impl<'de> de::Deserializer<'de> for Value { type Error = ConfigError; diff --git a/src/ser.rs b/src/ser.rs index 32b99ac..b4c1628 100644 --- a/src/ser.rs +++ b/src/ser.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use std::mem; use error::*; -use value::{Value, ValueKind, ValueWithKey}; +use value::{Value, ValueKind}; use Config; #[derive(Default, Debug)] diff --git a/src/value.rs b/src/value.rs index 1bba78c..2db195d 100644 --- a/src/value.rs +++ b/src/value.rs @@ -542,58 +542,3 @@ impl Display for Value { write!(f, "{}", self.kind) } } - -pub struct ValueWithKey<'a>(pub Value, &'a str); - -impl<'a> ValueWithKey<'a> { - pub fn new(value: Value, key: &'a str) -> Self { - ValueWithKey(value, key) - } - - pub fn into_bool(self) -> Result { - match self.0.into_bool() { - Ok(value) => Ok(value), - Err(error) => Err(error.extend_with_key(self.1)), - } - } - - /// Returns `self` into an i64, if possible. - pub fn into_int(self) -> Result { - match self.0.into_int() { - Ok(value) => Ok(value), - Err(error) => Err(error.extend_with_key(self.1)), - } - } - - /// Returns `self` into a f64, if possible. - pub fn into_float(self) -> Result { - match self.0.into_float() { - Ok(value) => Ok(value), - Err(error) => Err(error.extend_with_key(self.1)), - } - } - - /// Returns `self` into a str, if possible. - pub fn into_str(self) -> Result { - match self.0.into_str() { - Ok(value) => Ok(value), - Err(error) => Err(error.extend_with_key(self.1)), - } - } - - /// Returns `self` into an array, if possible - pub fn into_array(self) -> Result> { - match self.0.into_array() { - Ok(value) => Ok(value), - Err(error) => Err(error.extend_with_key(self.1)), - } - } - - /// If the `Value` is a Table, returns the associated Map. - pub fn into_table(self) -> Result> { - match self.0.into_table() { - Ok(value) => Ok(value), - Err(error) => Err(error.extend_with_key(self.1)), - } - } -} -- cgit v1.2.3