Merge pull request #394 from MomoLangenstein/393-better-serde-errors

ron-rs · Aug 15, 2022 · 172bdd6 · 172bdd6
2 parents f7e8417 + 021eb4c
commit 172bdd6
Show file tree

Hide file tree

Showing 8 changed files with 583 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix issue [#301](https://github.com/ron-rs/ron/issues/301) with better error messages ([#354](https://github.com/ron-rs/ron/pull/354))
 - Fix issue [#337](https://github.com/ron-rs/ron/issues/337) by removing `decimal_floats` PrettyConfig option and unconditional decimals in floats ([#363](https://github.com/ron-rs/ron/pull/363))
 - Fix issue [#203](https://github.com/ron-rs/ron/issues/203) with full de error positioning ([#356](https://github.com/ron-rs/ron/pull/356))
+- Expand the `ron::Error` enum to distinguish `serde` errors like `NoSuchEnumVariant` and `MissingStructField` with error positioning ([#394](https://github.com/ron-rs/ron/pull/394))
 - Bump MSRV to 1.56.0 ([#396](https://github.com/ron-rs/ron/pull/396))
 
 ## [0.7.1] - 2022-06-15

diff --git a/src/de/mod.rs b/src/de/mod.rs
@@ -25,6 +25,7 @@ mod value;
 pub struct Deserializer<'de> {
     bytes: Bytes<'de>,
     newtype_variant: bool,
+    last_identifier: Option<&'de str>,
 }
 
 impl<'de> Deserializer<'de> {
@@ -46,6 +47,7 @@ impl<'de> Deserializer<'de> {
         let mut deserializer = Deserializer {
             bytes: Bytes::new(input)?,
             newtype_variant: false,
+            last_identifier: None,
         };
 
         deserializer.bytes.exts |= options.default_extensions;
@@ -528,7 +530,19 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
             let old_newtype_variant = self.newtype_variant;
             self.newtype_variant = false;
 
-            let value = visitor.visit_map(CommaSeparated::new(b')', self))?;
+            let value = visitor
+                .visit_map(CommaSeparated::new(b')', self))
+                .map_err(|err| {
+                    struct_error_name(
+                        err,
+                        if !old_newtype_variant && !name.is_empty() {
+                            Some(name)
+                        } else {
+                            None
+                        },
+                    )
+                })?;
+
             self.bytes.comma()?;
 
             if old_newtype_variant || self.bytes.consume(")") {
@@ -545,7 +559,7 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
 
     fn deserialize_enum<V>(
         self,
-        _name: &'static str,
+        name: &'static str,
         _variants: &'static [&'static str],
         visitor: V,
     ) -> Result<V::Value>
@@ -554,14 +568,30 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
     {
         self.newtype_variant = false;
 
-        visitor.visit_enum(Enum::new(self))
+        match visitor.visit_enum(Enum::new(self)) {
+            Ok(value) => Ok(value),
+            Err(Error::NoSuchEnumVariant {
+                expected,
+                found,
+                outer: None,
+            }) if !name.is_empty() => Err(Error::NoSuchEnumVariant {
+                expected,
+                found,
+                outer: Some(String::from(name)),
+            }),
+            Err(e) => Err(e),
+        }
     }
 
     fn deserialize_identifier<V>(self, visitor: V) -> Result<V::Value>
     where
         V: Visitor<'de>,
     {
-        visitor.visit_str(str::from_utf8(self.bytes.identifier()?).map_err(Error::from)?)
+        let identifier = str::from_utf8(self.bytes.identifier()?).map_err(Error::from)?;
+
+        self.last_identifier = Some(identifier);
+
+        visitor.visit_str(identifier)
     }
 
     fn deserialize_ignored_any<V>(self, visitor: V) -> Result<V::Value>
@@ -699,6 +729,8 @@ impl<'de, 'a> de::VariantAccess<'de> for Enum<'a, 'de> {
     where
         T: DeserializeSeed<'de>,
     {
+        let newtype_variant = self.de.last_identifier;
+
         self.de.bytes.skip_ws()?;
 
         if self.de.bytes.consume("(") {
@@ -710,7 +742,9 @@ impl<'de, 'a> de::VariantAccess<'de> for Enum<'a, 'de> {
                 .exts
                 .contains(Extensions::UNWRAP_VARIANT_NEWTYPES);
 
-            let val = seed.deserialize(&mut *self.de)?;
+            let val = seed
+                .deserialize(&mut *self.de)
+                .map_err(|err| struct_error_name(err, newtype_variant))?;
 
             self.de.newtype_variant = false;
 
@@ -739,8 +773,35 @@ impl<'de, 'a> de::VariantAccess<'de> for Enum<'a, 'de> {
     where
         V: Visitor<'de>,
     {
+        let struct_variant = self.de.last_identifier;
+
         self.de.bytes.skip_ws()?;
 
-        self.de.deserialize_struct("", fields, visitor)
+        self.de
+            .deserialize_struct("", fields, visitor)
+            .map_err(|err| struct_error_name(err, struct_variant))
+    }
+}
+
+fn struct_error_name(error: Error, name: Option<&str>) -> Error {
+    match error {
+        Error::NoSuchStructField {
+            expected,
+            found,
+            outer: None,
+        } => Error::NoSuchStructField {
+            expected,
+            found,
+            outer: name.map(ToOwned::to_owned),
+        },
+        Error::MissingStructField { field, outer: None } => Error::MissingStructField {
+            field,
+            outer: name.map(ToOwned::to_owned),
+        },
+        Error::DuplicateStructField { field, outer: None } => Error::DuplicateStructField {
+            field,
+            outer: name.map(ToOwned::to_owned),
+        },
+        e => e,
     }
 }