Merge pull request #88 from sval-rs/fix/visit-serde-sval

Fix up how visitor does fallback on sval or serde

src/error.rs

@@ -21,6 +21,22 @@ impl Error {
             inner: Inner::Msg(msg),
         }
     }
+
+    #[cfg(feature = "serde1")]
+    pub(crate) fn try_boxed(msg: &'static str, e: impl fmt::Display) -> Self {
+        #[cfg(feature = "std")]
+        {
+            use crate::std::string::ToString;
+
+            let _ = msg;
+            Error::boxed(e.to_string())
+        }
+        #[cfg(not(feature = "std"))]
+        {
+            let _ = e;
+            Error::msg(msg)
+        }
+    }
 }
 
 impl fmt::Display for Error {

src/fill.rs

@@ -132,18 +132,6 @@ mod tests {
         assert_eq!("1", ValueBag::from_fill(&TestFill).to_string());
     }
 
-    #[test]
-    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
-    fn fill_value_owned() {
-        struct TestFill;
-
-        impl Fill for TestFill {
-            fn fill(&self, slot: Slot) -> Result<(), Error> {
-                slot.fill_any("a string")
-            }
-        }
-    }
-
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn fill_cast() {

src/internal/cast/mod.rs

@@ -275,18 +275,30 @@ impl<'v> Internal<'v> {
             #[cfg(feature = "sval2")]
             #[inline]
             fn sval2(&mut self, v: &dyn super::sval::v2::Value) -> Result<(), Error> {
-                super::sval::v2::internal_visit(v, self)
+                if super::sval::v2::internal_visit(v, self) {
+                    Ok(())
+                } else {
+                    Err(Error::msg("invalid cast"))
+                }
             }
 
             #[cfg(feature = "sval2")]
             fn borrowed_sval2(&mut self, v: &'v dyn super::sval::v2::Value) -> Result<(), Error> {
-                super::sval::v2::borrowed_internal_visit(v, self)
+                if super::sval::v2::borrowed_internal_visit(v, self) {
+                    Ok(())
+                } else {
+                    Err(Error::msg("invalid cast"))
+                }
             }
 
             #[cfg(feature = "serde1")]
             #[inline]
             fn serde1(&mut self, v: &dyn super::serde::v1::Serialize) -> Result<(), Error> {
-                super::serde::v1::internal_visit(v, self)
+                if super::serde::v1::internal_visit(v, self) {
+                    Ok(())
+                } else {
+                    Err(Error::msg("invalid cast"))
+                }
             }
 
             #[cfg(feature = "seq")]

src/internal/serde/v1.rs

@@ -37,6 +37,11 @@ impl<'v> ValueBag<'v> {
     }
 
     // NOTE: no `from_dyn_serde1` until `erased-serde` stabilizes
+    pub(crate) const fn from_dyn_serde1(value: &'v dyn Serialize) -> Self {
+        ValueBag {
+            inner: Internal::AnonSerde1(value),
+        }
+    }
 }
 
 pub(crate) trait DowncastSerialize {
@@ -86,17 +91,23 @@ impl<'v> value_bag_serde1::lib::Serialize for ValueBag<'v> {
             fn result(&self) -> Result<(), Error> {
                 match self.result {
                     Some(Ok(_)) => Ok(()),
-                    Some(Err(_)) | None => Err(Error::serde()),
+                    Some(Err(ref e)) => Err(Error::serde(e)),
+                    None => Err(Error::msg("`serde` serialization didn't produce a result")),
                 }
             }
 
             fn serializer(&mut self) -> Result<S, Error> {
-                self.inner.take().ok_or_else(|| Error::serde())
+                self.inner
+                    .take()
+                    .ok_or_else(|| Error::msg("`serde` serializer is in an invalid state"))
             }
 
             fn into_result(self) -> Result<S::Ok, S::Error> {
-                self.result
-                    .unwrap_or_else(|| Err(S::Error::custom("`serde` serialization failed")))
+                self.result.unwrap_or_else(|| {
+                    Err(S::Error::custom(
+                        "`serde` serialization didn't produce a result",
+                    ))
+                })
             }
         }
 
@@ -268,10 +279,7 @@ fn serialize_seq<S: value_bag_serde1::lib::Serializer>(
     s.serializer.end()
 }
 
-pub(crate) fn internal_visit<'v>(
-    v: &dyn Serialize,
-    visitor: &mut dyn InternalVisitor<'v>,
-) -> Result<(), Error> {
+pub(crate) fn internal_visit<'v>(v: &dyn Serialize, visitor: &mut dyn InternalVisitor<'v>) -> bool {
     struct VisitorSerializer<'a, 'v>(&'a mut dyn InternalVisitor<'v>);
 
     impl<'a, 'v> value_bag_serde1::lib::Serializer for VisitorSerializer<'a, 'v> {
@@ -457,12 +465,12 @@ pub(crate) fn internal_visit<'v>(
         }
     }
 
-    value_bag_serde1::erased::serialize(v, VisitorSerializer(visitor)).map_err(|_| Error::serde())
+    value_bag_serde1::erased::serialize(v, VisitorSerializer(visitor)).is_ok()
 }
 
 impl Error {
-    fn serde() -> Self {
-        Error::msg("`serde` serialization failed")
+    fn serde(e: impl fmt::Display) -> Self {
+        Error::try_boxed("`serde` serialization failed", e)
     }
 }
 

src/internal/sval/v2.rs

@@ -226,32 +226,25 @@ where
     value_bag_sval2::serde1::serialize(s, v)
 }
 
-pub(crate) fn internal_visit<'v>(
-    v: &dyn Value,
-    visitor: &mut dyn InternalVisitor<'v>,
-) -> Result<(), Error> {
+pub(crate) fn internal_visit<'v>(v: &dyn Value, visitor: &mut dyn InternalVisitor<'v>) -> bool {
     let mut visitor = VisitorStream {
         visitor,
         text_buf: Default::default(),
     };
 
-    value_bag_sval2::lib::stream_computed(&mut visitor, v).map_err(Error::from_sval2)?;
-
-    Ok(())
+    value_bag_sval2::lib::stream_computed(&mut visitor, v).is_ok()
 }
 
 pub(crate) fn borrowed_internal_visit<'v>(
     v: &'v dyn Value,
     visitor: &mut dyn InternalVisitor<'v>,
-) -> Result<(), Error> {
+) -> bool {
     let mut visitor = VisitorStream {
         visitor,
         text_buf: Default::default(),
     };
 
-    value_bag_sval2::lib::stream(&mut visitor, v).map_err(Error::from_sval2)?;
-
-    Ok(())
+    value_bag_sval2::lib::stream(&mut visitor, v).is_ok()
 }
 
 struct VisitorStream<'a, 'v> {

src/visit.rs

@@ -307,20 +307,32 @@ impl<'v> ValueBag<'v> {
 
             #[cfg(feature = "sval2")]
             fn sval2(&mut self, v: &dyn internal::sval::v2::Value) -> Result<(), Error> {
-                internal::sval::v2::internal_visit(v, self)
+                if internal::sval::v2::internal_visit(v, self) {
+                    Ok(())
+                } else {
+                    self.0.visit_any(ValueBag::from_dyn_sval2(v))
+                }
             }
 
             #[cfg(feature = "sval2")]
             fn borrowed_sval2(
                 &mut self,
                 v: &'v dyn internal::sval::v2::Value,
             ) -> Result<(), Error> {
-                internal::sval::v2::borrowed_internal_visit(v, self)
+                if internal::sval::v2::borrowed_internal_visit(v, self) {
+                    Ok(())
+                } else {
+                    self.0.visit_any(ValueBag::from_dyn_sval2(v))
+                }
             }
 
             #[cfg(feature = "serde1")]
             fn serde1(&mut self, v: &dyn internal::serde::v1::Serialize) -> Result<(), Error> {
-                internal::serde::v1::internal_visit(v, self)
+                if internal::serde::v1::internal_visit(v, self) {
+                    Ok(())
+                } else {
+                    self.0.visit_any(ValueBag::from_dyn_serde1(v))
+                }
             }
 
             #[cfg(feature = "seq")]
@@ -395,4 +407,102 @@ mod tests {
 
         assert!(visitor.0);
     }
+
+    #[test]
+    #[cfg(feature = "serde1")]
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+    fn visit_serde1() {
+        use crate::std::string::{String, ToString};
+
+        struct Data;
+
+        impl value_bag_serde1::lib::Serialize for Data {
+            fn serialize<S: value_bag_serde1::lib::Serializer>(
+                &self,
+                serializer: S,
+            ) -> Result<S::Ok, S::Error> {
+                use value_bag_serde1::lib::ser::SerializeStruct;
+
+                let mut s = serializer.serialize_struct("Data", 3)?;
+                s.serialize_field("a", &1)?;
+                s.serialize_field("b", &2)?;
+                s.serialize_field("c", &3)?;
+                s.end()
+            }
+        }
+
+        struct Visitor(String);
+
+        impl<'v> Visit<'v> for Visitor {
+            fn visit_any(&mut self, v: ValueBag) -> Result<(), Error> {
+                self.0 = v.to_string();
+
+                Ok(())
+            }
+        }
+
+        let mut visitor = Visitor("".into());
+        ValueBag::from_serde1(&Data).visit(&mut visitor).unwrap();
+
+        assert_eq!("Data { a: 1, b: 2, c: 3 }", visitor.0);
+    }
+
+    #[test]
+    #[cfg(feature = "sval2")]
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
+    fn visit_sval2() {
+        use crate::std::string::{String, ToString};
+
+        struct Data;
+
+        impl value_bag_sval2::lib::Value for Data {
+            fn stream<'sval, S: value_bag_sval2::lib::Stream<'sval> + ?Sized>(
+                &'sval self,
+                stream: &mut S,
+            ) -> value_bag_sval2::lib::Result {
+                stream.map_begin(Some(3))?;
+
+                stream.map_key_begin()?;
+                stream.value("a")?;
+                stream.map_key_end()?;
+
+                stream.map_value_begin()?;
+                stream.value(&1)?;
+                stream.map_value_end()?;
+
+                stream.map_key_begin()?;
+                stream.value("b")?;
+                stream.map_key_end()?;
+
+                stream.map_value_begin()?;
+                stream.value(&2)?;
+                stream.map_value_end()?;
+
+                stream.map_key_begin()?;
+                stream.value("c")?;
+                stream.map_key_end()?;
+
+                stream.map_value_begin()?;
+                stream.value(&3)?;
+                stream.map_value_end()?;
+
+                stream.map_end()
+            }
+        }
+
+        struct Visitor(String);
+
+        impl<'v> Visit<'v> for Visitor {
+            fn visit_any(&mut self, v: ValueBag) -> Result<(), Error> {
+                self.0 = v.to_string();
+
+                Ok(())
+            }
+        }
+
+        let mut visitor = Visitor("".into());
+        ValueBag::from_sval2(&Data).visit(&mut visitor).unwrap();
+
+        assert_eq!("{ \"a\": 1, \"b\": 2, \"c\": 3 }", visitor.0);
+    }
 }