From 9407e2eaaaa059e78bd2e8c37c2dfddb30a334f6 Mon Sep 17 00:00:00 2001 From: Juniper Langenstein Date: Sun, 24 Jul 2022 21:08:40 +0000 Subject: [PATCH] Annotate serde struct+enum errors with struct/enum/variant names --- src/de/mod.rs | 73 ++++++++++++++++-- src/error.rs | 62 ++++++++++++--- tests/203_error_positions.rs | 12 ++- tests/393_serde_errors.rs | 146 +++++++++++++++++++++++++++++++++++ 4 files changed, 273 insertions(+), 20 deletions(-) create mode 100644 tests/393_serde_errors.rs diff --git a/src/de/mod.rs b/src/de/mod.rs index 6f4afbdd..0920a67b 100644 --- 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( self, - _name: &'static str, + name: &'static str, _variants: &'static [&'static str], visitor: V, ) -> Result @@ -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(self, visitor: V) -> Result 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(self, visitor: V) -> Result @@ -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, } } diff --git a/src/error.rs b/src/error.rs index d37225a0..81bd462c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -74,13 +74,21 @@ pub enum Error { NoSuchEnumVariant { expected: &'static [&'static str], found: String, + outer: Option, }, NoSuchStructField { expected: &'static [&'static str], found: String, + outer: Option, + }, + MissingStructField { + field: &'static str, + outer: Option, + }, + DuplicateStructField { + field: &'static str, + outer: Option, }, - MissingStructField(&'static str), - DuplicateStructField(&'static str), } impl fmt::Display for SpannedError { @@ -173,11 +181,23 @@ impl fmt::Display for Error { Error::NoSuchEnumVariant { expected, ref found, + ref outer, } => { + f.write_str("Unexpected ")?; + + if outer.is_none() { + f.write_str("enum ")?; + } + + write!(f, "variant named `{}`", found)?; + + if let Some(outer) = outer { + write!(f, "in enum `{}`", outer)?; + } + write!( f, - "Unexpected enum variant named `{}`, {}", - found, + ", {}", OneOf { alts: expected, none: "variants" @@ -187,22 +207,38 @@ impl fmt::Display for Error { Error::NoSuchStructField { expected, ref found, + ref outer, } => { + write!(f, "Unexpected field named `{}`", found)?; + + if let Some(outer) = outer { + write!(f, "in `{}`", outer)?; + } + write!( f, - "Unexpected field named `{}`, {}", - found, + ", {}", OneOf { alts: expected, none: "fields" } ) } - Error::MissingStructField(field) => { - write!(f, "Unexpected missing field `{}`", field) + Error::MissingStructField { field, ref outer } => { + write!(f, "Unexpected missing field `{}`", field)?; + + match outer { + Some(outer) => write!(f, " in `{}`", outer), + None => Ok(()), + } } - Error::DuplicateStructField(field) => { - write!(f, "Unexpected duplicate field `{}`", field) + Error::DuplicateStructField { field, ref outer } => { + write!(f, "Unexpected duplicate field `{}`", field)?; + + match outer { + Some(outer) => write!(f, " in `{}`", outer), + None => Ok(()), + } } } } @@ -262,6 +298,7 @@ impl de::Error for Error { Error::NoSuchEnumVariant { expected, found: variant.to_string(), + outer: None, } } @@ -270,17 +307,18 @@ impl de::Error for Error { Error::NoSuchStructField { expected, found: field.to_string(), + outer: None, } } #[cold] fn missing_field(field: &'static str) -> Self { - Error::MissingStructField(field) + Error::MissingStructField { field, outer: None } } #[cold] fn duplicate_field(field: &'static str) -> Self { - Error::DuplicateStructField(field) + Error::DuplicateStructField { field, outer: None } } } diff --git a/tests/203_error_positions.rs b/tests/203_error_positions.rs index 05a1872c..e7ee7d25 100644 --- a/tests/203_error_positions.rs +++ b/tests/203_error_positions.rs @@ -63,6 +63,7 @@ fn test_error_positions() { code: Error::NoSuchEnumVariant { expected: &["TupleVariant", "StructVariant"], found: String::from("NotAVariant"), + outer: Some(String::from("Test")), }, position: Position { line: 1, col: 12 }, }) @@ -74,6 +75,7 @@ fn test_error_positions() { code: Error::NoSuchStructField { expected: &["a", "b", "c"], found: String::from("d"), + outer: Some(String::from("StructVariant")), }, position: Position { line: 1, col: 39 }, }) @@ -82,7 +84,10 @@ fn test_error_positions() { assert_eq!( ron::from_str::("StructVariant(a: true, c: -42)"), Err(SpannedError { - code: Error::MissingStructField("b"), + code: Error::MissingStructField { + field: "b", + outer: Some(String::from("StructVariant")), + }, position: Position { line: 1, col: 30 }, }) ); @@ -90,7 +95,10 @@ fn test_error_positions() { assert_eq!( ron::from_str::("StructVariant(a: true, b: 1, a: false, c: -42)"), Err(SpannedError { - code: Error::DuplicateStructField("a"), + code: Error::DuplicateStructField { + field: "a", + outer: Some(String::from("StructVariant")), + }, position: Position { line: 1, col: 31 }, }) ); diff --git a/tests/393_serde_errors.rs b/tests/393_serde_errors.rs new file mode 100644 index 00000000..576d4165 --- /dev/null +++ b/tests/393_serde_errors.rs @@ -0,0 +1,146 @@ +use ron::error::{Error, Position, SpannedError}; + +#[derive(Debug, serde::Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +enum TestEnum { + StructVariant { a: bool, b: char, c: i32 }, + NewtypeVariant(TestStruct), +} + +#[derive(Debug, serde::Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] +struct TestStruct { + a: bool, + b: char, + c: i32, +} + +#[test] +fn test_unknown_enum_variant() { + assert_eq!( + ron::from_str::("NotAVariant"), + Err(SpannedError { + code: Error::NoSuchEnumVariant { + expected: &["StructVariant", "NewtypeVariant"], + found: String::from("NotAVariant"), + outer: Some(String::from("TestEnum")), + }, + position: Position { line: 1, col: 12 }, + }) + ); +} + +#[test] +fn test_struct_enum_fields() { + assert_eq!( + ron::from_str::("StructVariant(a: true, b: 'b', c: -42, d: \"gotcha\")"), + Err(SpannedError { + code: Error::NoSuchStructField { + expected: &["a", "b", "c"], + found: String::from("d"), + outer: Some(String::from("StructVariant")), + }, + position: Position { line: 1, col: 41 }, + }) + ); + + assert_eq!( + ron::from_str::("StructVariant(a: true, c: -42)"), + Err(SpannedError { + code: Error::MissingStructField { + field: "b", + outer: Some(String::from("StructVariant")), + }, + position: Position { line: 1, col: 30 }, + }) + ); + + assert_eq!( + ron::from_str::("StructVariant(a: true, b: 'b', a: false, c: -42)"), + Err(SpannedError { + code: Error::DuplicateStructField { + field: "a", + outer: Some(String::from("StructVariant")), + }, + position: Position { line: 1, col: 33 }, + }) + ); +} + +#[test] +fn test_newtype_enum_fields() { + assert_eq!( + ron::from_str::("#![enable(unwrap_variant_newtypes)] NewtypeVariant(a: true, b: 'b', c: -42, d: \"gotcha\")"), + Err(SpannedError { + code: Error::NoSuchStructField { + expected: &["a", "b", "c"], + found: String::from("d"), + outer: Some(String::from("NewtypeVariant")), + }, + position: Position { line: 1, col: 78 }, + }) + ); + + assert_eq!( + ron::from_str::( + "#![enable(unwrap_variant_newtypes)] NewtypeVariant(a: true, c: -42)" + ), + Err(SpannedError { + code: Error::MissingStructField { + field: "b", + outer: Some(String::from("NewtypeVariant")), + }, + position: Position { line: 1, col: 67 }, + }) + ); + + assert_eq!( + ron::from_str::( + "#![enable(unwrap_variant_newtypes)] NewtypeVariant(a: true, b: 'b', a: false, c: -42)" + ), + Err(SpannedError { + code: Error::DuplicateStructField { + field: "a", + outer: Some(String::from("NewtypeVariant")), + }, + position: Position { line: 1, col: 70 }, + }) + ); +} + +#[test] +fn test_struct_fields() { + assert_eq!( + ron::from_str::("TestStruct(a: true, b: 'b', c: -42, d: \"gotcha\")"), + Err(SpannedError { + code: Error::NoSuchStructField { + expected: &["a", "b", "c"], + found: String::from("d"), + outer: Some(String::from("TestStruct")), + }, + position: Position { line: 1, col: 38 }, + }) + ); + + assert_eq!( + ron::from_str::("TestStruct(a: true, c: -42)"), + Err(SpannedError { + code: Error::MissingStructField { + field: "b", + outer: Some(String::from("TestStruct")), + }, + position: Position { line: 1, col: 27 }, + }) + ); + + assert_eq!( + ron::from_str::("TestStruct(a: true, b: 'b', a: false, c: -42)"), + Err(SpannedError { + code: Error::DuplicateStructField { + field: "a", + outer: Some(String::from("TestStruct")), + }, + position: Position { line: 1, col: 30 }, + }) + ); +}