Unify deser impl #338
Unify deser impl #338
Conversation
This tests, in particular, that the error messdage is as expected including the right key and filename.
This had open-coded copies of the impl for Value. Replace them with calls to the impl for Value. This reduces duplication. It would allow us to change the impl for Value. Use a macro for the many very-similar functions.
There was a problem hiding this comment.
Hi. Sorry for not getting back to this for such a long time... I had other things on my mind and then I was on vacation. 🏖️
I hope you don't mind. 😸
If you are not interested anymore in continuing work on this PR, I am happy to make the adaptions requested in my review myself!
| fn deserialize_bool<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| visitor.visit_bool(self.cache.into_bool()?) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_i8<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type | ||
| visitor.visit_i8(self.cache.into_int()? as i8) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_i16<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type | ||
| visitor.visit_i16(self.cache.into_int()? as i16) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_i32<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type | ||
| visitor.visit_i32(self.cache.into_int()? as i32) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_i64<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| visitor.visit_i64(self.cache.into_int()?) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_u8<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type | ||
| visitor.visit_u8(self.cache.into_int()? as u8) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_u16<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type | ||
| visitor.visit_u16(self.cache.into_int()? as u16) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_u32<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type | ||
| visitor.visit_u32(self.cache.into_int()? as u32) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn deserialize_u64<V: de::Visitor<'de>>(self, visitor: V) -> Result<V::Value> { | ||
| // FIXME: This should *fail* if the value does not fit in the requets integer type |
There was a problem hiding this comment.
There are FIXME nontations here that get lost in your patch. Can you please port them to your improvements?
There was a problem hiding this comment.
In the meantime those FIXMEs have gone and much of the code has been reorganised. I'm going to fix #464 first and then maybe revisit this.
| let expect_err = |res: Result<(), ConfigError>| { | ||
| assert!(res.is_err()); | ||
| assert_eq!( | ||
| res.unwrap_err().to_string(), | ||
| format!( | ||
| "invalid type: string \"Torre di Pisa\", expected an integer for key `place.name` in tests/Settings.toml", | ||
| ) | ||
| ); | ||
| }; | ||
|
|
||
| let c = make(); | ||
| let res = c.try_deserialize().map(|_: Output| ()); | ||
| expect_err(res); |
There was a problem hiding this comment.
Why does this have to be so complicated?
There was a problem hiding this comment.
I was expecting there might be more tests with errors, so wanted to provide a helper to check an error was expected. I can inline this again if you prefer, which would probably make it a bit simpler looking.
|
Closing in favour of #488 |
As part of my efforts to try to make
#[deser(flatten)]work correctly (#336) I did some refactoring. Although the deser work is unfinished, I think these two commits are worth submitting separately.