diff --git a/serde_with/CHANGELOG.md b/serde_with/CHANGELOG.md index 23a7f446..76346372 100644 --- a/serde_with/CHANGELOG.md +++ b/serde_with/CHANGELOG.md @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Changed + +* Improve the error message when deserializing `OneOrMany` or `PickFirst` fails. + It now includes the original error message for each of the individual variants. + This is possible by dropping untagged enums as the internal implementations, since they will likely never support this, as these old PRs show [serde#2376](https://github.com/serde-rs/serde/pull/2376) and [serde#1544](https://github.com/serde-rs/serde/pull/1544). + + The new errors look like: + + ```text + OneOrMany could not deserialize any variant: + One: invalid type: map, expected u32 + Many: invalid type: map, expected a sequence + ``` + + ```text + PickFirst could not deserialize any variant: + First: invalid type: string "Abc", expected u32 + Second: invalid digit found in string + ``` + ## [2.3.1] - 2023-03-10 ### Fixed diff --git a/serde_with/Cargo.toml b/serde_with/Cargo.toml index 969dd049..895aaa20 100644 --- a/serde_with/Cargo.toml +++ b/serde_with/Cargo.toml @@ -65,6 +65,8 @@ chrono_0_4 = {package = "chrono", version = "0.4.20", optional = true, default-f doc-comment = {version = "0.3.3", optional = true} hex = {version = "0.4.3", optional = true, default-features = false} indexmap_1 = {package = "indexmap", version = "1.8", optional = true, default-features = false, features = ["serde-1"]} +# The derive feature is needed for the flattened_maybe macro. +# https://github.com/jonasbb/serde_with/blob/eb1965a74a3be073ecd13ec05f02a01bc1c44309/serde_with/src/flatten_maybe.rs#L67 serde = {version = "1.0.122", default-features = false, features = ["derive"]} serde_json = {version = "1.0.45", optional = true, default-features = false} serde_with_macros = {path = "../serde_with_macros", version = "=2.3.1", optional = true} diff --git a/serde_with/src/de/impls.rs b/serde_with/src/de/impls.rs index 92fbf839..fb6409f4 100644 --- a/serde_with/src/de/impls.rs +++ b/serde_with/src/de/impls.rs @@ -1,4 +1,4 @@ -use crate::{formats::*, prelude::*}; +use crate::{formats::*, prelude::*, utils::SeqIter}; #[cfg(feature = "indexmap_1")] use indexmap_1::{IndexMap, IndexSet}; @@ -842,22 +842,32 @@ where where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde( - untagged, - bound(deserialize = "DeserializeAsWrap: Deserialize<'de>") - )] - enum GoodOrError<'a, T, TAs> + enum GoodOrError { + Good(T), + // Only here to consume the TAs generic + Error(PhantomData), + } + + impl<'de, T, TAs> Deserialize<'de> for GoodOrError where - TAs: DeserializeAs<'a, T>, + TAs: DeserializeAs<'de, T>, { - Good(DeserializeAsWrap), - // This consumes one "item" when `T` errors while deserializing. - // This is necessary to make this work, when instead of having a direct value - // like integer or string, the deserializer sees a list or map. - Error(IgnoredAny), - #[serde(skip)] - _JustAMarkerForTheLifetime(PhantomData<&'a u32>), + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let is_hr = deserializer.is_human_readable(); + let content: content::de::Content<'de> = Deserialize::deserialize(deserializer)?; + + Ok( + match >::deserialize( + content::de::ContentDeserializer::::new(content, is_hr), + ) { + Ok(elem) => GoodOrError::Good(elem.into_inner()), + Err(_) => GoodOrError::Error(PhantomData), + }, + ) + } } struct SeqVisitor { @@ -865,9 +875,9 @@ where marker2: PhantomData, } - impl<'de, T, U> Visitor<'de> for SeqVisitor + impl<'de, T, TAs> Visitor<'de> for SeqVisitor where - U: DeserializeAs<'de, T>, + TAs: DeserializeAs<'de, T>, { type Value = Vec; @@ -875,18 +885,17 @@ where formatter.write_str("a sequence") } - fn visit_seq(self, mut seq: A) -> Result + fn visit_seq(self, seq: A) -> Result where A: SeqAccess<'de>, { - let mut values = Vec::with_capacity(seq.size_hint().unwrap_or_default()); - - while let Some(value) = seq.next_element()? { - if let GoodOrError::::Good(value) = value { - values.push(value.into_inner()); - } - } - Ok(values) + SeqIter::new(seq) + .filter_map(|res: Result, A::Error>| match res { + Ok(GoodOrError::Good(value)) => Some(Ok(value)), + Ok(GoodOrError::Error(_)) => None, + Err(err) => Some(Err(err)), + }) + .collect() } } @@ -952,28 +961,21 @@ where where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde( - untagged, - bound(deserialize = "DeserializeAsWrap: Deserialize<'de>") - )] - enum GoodOrError<'a, T, TAs> - where - TAs: DeserializeAs<'a, T>, - { - Good(DeserializeAsWrap), - // This consumes one "item" when `T` errors while deserializing. - // This is necessary to make this work, when instead of having a direct value - // like integer or string, the deserializer sees a list or map. - Error(IgnoredAny), - #[serde(skip)] - _JustAMarkerForTheLifetime(PhantomData<&'a u32>), - } + let is_hr = deserializer.is_human_readable(); + let content: content::de::Content<'de> = match Deserialize::deserialize(deserializer) { + Ok(content) => content, + Err(_) => return Ok(Default::default()), + }; - Ok(match Deserialize::deserialize(deserializer) { - Ok(GoodOrError::::Good(res)) => res.into_inner(), - _ => Default::default(), - }) + Ok( + match >::deserialize(content::de::ContentDeserializer::< + D::Error, + >::new(content, is_hr)) + { + Ok(elem) => elem.into_inner(), + Err(_) => Default::default(), + }, + ) } } @@ -1522,38 +1524,34 @@ impl<'de, const N: usize> DeserializeAs<'de, Box<[u8; N]>> for Bytes { } #[cfg(feature = "alloc")] -impl<'de, T, U, FORMAT> DeserializeAs<'de, Vec> for OneOrMany +impl<'de, T, TAs, FORMAT> DeserializeAs<'de, Vec> for OneOrMany where - U: DeserializeAs<'de, T>, + TAs: DeserializeAs<'de, T>, FORMAT: Format, { fn deserialize_as(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde( - untagged, - bound(deserialize = r#"DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap, Vec>: Deserialize<'de>"#), - expecting = "a list or single element" - )] - enum Helper<'a, T, U> - where - U: DeserializeAs<'a, T>, - { - One(DeserializeAsWrap), - Many(DeserializeAsWrap, Vec>), - #[serde(skip)] - _JustAMarkerForTheLifetime(PhantomData<&'a u32>), - } - - let h: Helper<'de, T, U> = Deserialize::deserialize(deserializer)?; - match h { - Helper::One(one) => Ok(alloc::vec![one.into_inner()]), - Helper::Many(many) => Ok(many.into_inner()), - Helper::_JustAMarkerForTheLifetime(_) => unreachable!(), - } + let is_hr = deserializer.is_human_readable(); + let content: content::de::Content<'de> = Deserialize::deserialize(deserializer)?; + + let one_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(one) => return Ok(alloc::vec![one.into_inner()]), + Err(err) => err, + }; + let many_err: D::Error = match , Vec>>::deserialize( + content::de::ContentDeserializer::new(content, is_hr), + ) { + Ok(many) => return Ok(many.into_inner()), + Err(err) => err, + }; + Err(DeError::custom(format_args!( + "OneOrMany could not deserialize any variant:\n One: {}\n Many: {}", + one_err, many_err + ))) } } @@ -1580,32 +1578,25 @@ where where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde( - untagged, - bound(deserialize = r#" - DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap: Deserialize<'de>, - "#), - expecting = "PickFirst could not deserialize data" - )] - enum Helper<'a, T, TAs1, TAs2> - where - TAs1: DeserializeAs<'a, T>, - TAs2: DeserializeAs<'a, T>, - { - First(DeserializeAsWrap), - Second(DeserializeAsWrap), - #[serde(skip)] - _JustAMarkerForTheLifetime(PhantomData<&'a u32>), - } - - let h: Helper<'de, T, TAs1, TAs2> = Deserialize::deserialize(deserializer)?; - match h { - Helper::First(first) => Ok(first.into_inner()), - Helper::Second(second) => Ok(second.into_inner()), - Helper::_JustAMarkerForTheLifetime(_) => unreachable!(), - } + let is_hr = deserializer.is_human_readable(); + let content: content::de::Content<'de> = Deserialize::deserialize(deserializer)?; + + let first_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(first) => return Ok(first.into_inner()), + Err(err) => err, + }; + let second_err: D::Error = match >::deserialize( + content::de::ContentDeserializer::new(content, is_hr), + ) { + Ok(second) => return Ok(second.into_inner()), + Err(err) => err, + }; + Err(DeError::custom(format_args!( + "PickFirst could not deserialize any variant:\n First: {}\n Second: {}", + first_err, second_err + ))) } } @@ -1620,36 +1611,31 @@ where where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde( - untagged, - bound(deserialize = r#" - DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap: Deserialize<'de>, - "#), - expecting = "PickFirst could not deserialize data" - )] - enum Helper<'a, T, TAs1, TAs2, TAs3> - where - TAs1: DeserializeAs<'a, T>, - TAs2: DeserializeAs<'a, T>, - TAs3: DeserializeAs<'a, T>, - { - First(DeserializeAsWrap), - Second(DeserializeAsWrap), - Third(DeserializeAsWrap), - #[serde(skip)] - _JustAMarkerForTheLifetime(PhantomData<&'a u32>), - } - - let h: Helper<'de, T, TAs1, TAs2, TAs3> = Deserialize::deserialize(deserializer)?; - match h { - Helper::First(first) => Ok(first.into_inner()), - Helper::Second(second) => Ok(second.into_inner()), - Helper::Third(third) => Ok(third.into_inner()), - Helper::_JustAMarkerForTheLifetime(_) => unreachable!(), - } + let is_hr = deserializer.is_human_readable(); + let content: content::de::Content<'de> = Deserialize::deserialize(deserializer)?; + + let first_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(first) => return Ok(first.into_inner()), + Err(err) => err, + }; + let second_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(second) => return Ok(second.into_inner()), + Err(err) => err, + }; + let third_err: D::Error = match >::deserialize( + content::de::ContentDeserializer::new(content, is_hr), + ) { + Ok(third) => return Ok(third.into_inner()), + Err(err) => err, + }; + Err(DeError::custom(format_args!( + "PickFirst could not deserialize any variant:\n First: {}\n Second: {}\n Third: {}", + first_err, second_err, third_err, + ))) } } @@ -1665,40 +1651,37 @@ where where D: Deserializer<'de>, { - #[derive(serde::Deserialize)] - #[serde( - untagged, - bound(deserialize = r#" - DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap: Deserialize<'de>, - DeserializeAsWrap: Deserialize<'de>, - "#), - expecting = "PickFirst could not deserialize data" - )] - enum Helper<'a, T, TAs1, TAs2, TAs3, TAs4> - where - TAs1: DeserializeAs<'a, T>, - TAs2: DeserializeAs<'a, T>, - TAs3: DeserializeAs<'a, T>, - TAs4: DeserializeAs<'a, T>, - { - First(DeserializeAsWrap), - Second(DeserializeAsWrap), - Third(DeserializeAsWrap), - Forth(DeserializeAsWrap), - #[serde(skip)] - _JustAMarkerForTheLifetime(PhantomData<&'a u32>), - } - - let h: Helper<'de, T, TAs1, TAs2, TAs3, TAs4> = Deserialize::deserialize(deserializer)?; - match h { - Helper::First(first) => Ok(first.into_inner()), - Helper::Second(second) => Ok(second.into_inner()), - Helper::Third(third) => Ok(third.into_inner()), - Helper::Forth(forth) => Ok(forth.into_inner()), - Helper::_JustAMarkerForTheLifetime(_) => unreachable!(), - } + let is_hr = deserializer.is_human_readable(); + let content: content::de::Content<'de> = Deserialize::deserialize(deserializer)?; + + let first_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(first) => return Ok(first.into_inner()), + Err(err) => err, + }; + let second_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(second) => return Ok(second.into_inner()), + Err(err) => err, + }; + let third_err: D::Error = match >::deserialize( + content::de::ContentRefDeserializer::new(&content, is_hr), + ) { + Ok(third) => return Ok(third.into_inner()), + Err(err) => err, + }; + let fourth_err: D::Error = match >::deserialize( + content::de::ContentDeserializer::new(content, is_hr), + ) { + Ok(fourth) => return Ok(fourth.into_inner()), + Err(err) => err, + }; + Err(DeError::custom(format_args!( + "PickFirst could not deserialize any variant:\n First: {}\n Second: {}\n Third: {}\n Fourth: {}", + first_err, second_err, third_err, fourth_err, + ))) } } diff --git a/serde_with/tests/serde_as/lib.rs b/serde_with/tests/serde_as/lib.rs index 6209312d..122c02fd 100644 --- a/serde_with/tests/serde_as/lib.rs +++ b/serde_with/tests/serde_as/lib.rs @@ -844,8 +844,20 @@ fn test_one_or_many_prefer_one() { ); check_deserialization(S1Vec(vec![1]), r#"1"#); check_deserialization(S1Vec(vec![1]), r#"[1]"#); - check_error_deserialization::(r#"{}"#, expect![[r#"a list or single element"#]]); - check_error_deserialization::(r#""xx""#, expect![[r#"a list or single element"#]]); + check_error_deserialization::( + r#"{}"#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid type: map, expected u32 + Many: invalid type: map, expected a sequence"#]], + ); + check_error_deserialization::( + r#""xx""#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid type: string "xx", expected u32 + Many: invalid type: string "xx", expected a sequence"#]], + ); #[serde_as] #[derive(Debug, Serialize, Deserialize, PartialEq)] @@ -865,8 +877,20 @@ fn test_one_or_many_prefer_one() { ); check_deserialization(S2Vec(vec![1]), r#""1""#); check_deserialization(S2Vec(vec![1]), r#"["1"]"#); - check_error_deserialization::(r#"{}"#, expect![[r#"a list or single element"#]]); - check_error_deserialization::(r#""xx""#, expect![[r#"a list or single element"#]]); + check_error_deserialization::( + r#"{}"#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid type: map, expected a string + Many: invalid type: map, expected a sequence"#]], + ); + check_error_deserialization::( + r#""xx""#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid digit found in string + Many: invalid type: string "xx", expected a sequence"#]], + ); } #[test] @@ -897,8 +921,20 @@ fn test_one_or_many_prefer_many() { ); check_deserialization(S1Vec(vec![1]), r#"1"#); check_deserialization(S1Vec(vec![1]), r#"[1]"#); - check_error_deserialization::(r#"{}"#, expect![[r#"a list or single element"#]]); - check_error_deserialization::(r#""xx""#, expect![[r#"a list or single element"#]]); + check_error_deserialization::( + r#"{}"#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid type: map, expected u32 + Many: invalid type: map, expected a sequence"#]], + ); + check_error_deserialization::( + r#""xx""#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid type: string "xx", expected u32 + Many: invalid type: string "xx", expected a sequence"#]], + ); #[serde_as] #[derive(Debug, Serialize, Deserialize, PartialEq)] @@ -924,8 +960,20 @@ fn test_one_or_many_prefer_many() { ); check_deserialization(S2Vec(vec![1]), r#""1""#); check_deserialization(S2Vec(vec![1]), r#"["1"]"#); - check_error_deserialization::(r#"{}"#, expect![[r#"a list or single element"#]]); - check_error_deserialization::(r#""xx""#, expect![[r#"a list or single element"#]]); + check_error_deserialization::( + r#"{}"#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid type: map, expected a string + Many: invalid type: map, expected a sequence"#]], + ); + check_error_deserialization::( + r#""xx""#, + expect![[r#" + OneOrMany could not deserialize any variant: + One: invalid digit found in string + Many: invalid type: string "xx", expected a sequence"#]], + ); } /// Test that Cow borrows from the input diff --git a/serde_with/tests/serde_as/pickfirst.rs b/serde_with/tests/serde_as/pickfirst.rs index 6443526f..8c221df8 100644 --- a/serde_with/tests/serde_as/pickfirst.rs +++ b/serde_with/tests/serde_as/pickfirst.rs @@ -14,7 +14,10 @@ fn test_pick_first_two() { check_deserialization(S(123), r#""123""#); check_error_deserialization::( r#""Abc""#, - expect![[r#"PickFirst could not deserialize data"#]], + expect![[r#" + PickFirst could not deserialize any variant: + First: invalid type: string "Abc", expected u32 + Second: invalid digit found in string"#]], ); #[serde_as] @@ -91,7 +94,11 @@ fn test_pick_first_three() { check_deserialization(S(vec![1, 2, 3]), r#""1,2,3""#); check_error_deserialization::( r#""Abc""#, - expect![[r#"PickFirst could not deserialize data"#]], + expect![[r#" + PickFirst could not deserialize any variant: + First: invalid type: string "Abc", expected a sequence + Second: invalid type: string "Abc", expected a sequence + Third: invalid digit found in string"#]], ); #[serde_as] @@ -132,6 +139,11 @@ fn test_pick_first_four() { is_equal(S(123), expect![[r#"123"#]]); check_error_deserialization::( r#""Abc""#, - expect![[r#"PickFirst could not deserialize data"#]], + expect![[r#" + PickFirst could not deserialize any variant: + First: invalid type: string "Abc", expected u32 + Second: invalid type: string "Abc", expected u32 + Third: invalid type: string "Abc", expected u32 + Fourth: invalid type: string "Abc", expected u32"#]], ); }