diff --git a/README.md b/README.md index 073b918e0..710e80b28 100644 --- a/README.md +++ b/README.md @@ -190,13 +190,14 @@ While data structures with any of these attributes should generally roundtrip th - internally (or adjacently) tagged or untagged enum or a `#[serde(flatten)]`ed fields do not support: - `i128` or `u128` values - struct names, e.g. by enabling the `PrettyConfig::struct_names` setting - - `Option`s when `#![enable(implicit_some)]` is enabled - - newtypes and zero-length arrays / tuples / tuple structs / structs / tuple variants / struct variants - - externally tagged tuple enum variants with just one field (that are not newtype variants) + - newtypes + - zero-length arrays / tuples / tuple structs / structs / tuple variants / struct variants + - `Option`s with `#[enable(implicit_some)]` must nog contain any of these or a unit, unit struct, or an untagged unit variant + - externally tagged tuple variants with just one field (that are not newtype variants) + - tuples or arrays with just one element are not supported inside newtype variants with `#[enable(unwrap_variant_newtypes)]` - untagged tuple / struct variants with no fields are not supported - untagged tuple variants with just one field (that are not newtype variants) are not supported when the `#![enable(unwrap_variant_newtypes)]` extension is enabled - internally tagged newtype variants must not contain a unit / unit struct inside an untagged newtype variant, or an untagged unit variant -- tuples or arrays with just one element are not supported inside newtype variants with `#[enable(unwrap_variant_newtypes)]` - flattened structs with conflicting keys (e.g. an earlier inner-struct key matches a later outer-struct key or two flattened maps in the same struct share a key) are not supported by serde Please file a [new issue](https://github.com/ron-rs/ron/issues/new) if you come across a use case which is not listed among the above restrictions but still breaks. diff --git a/fuzz/fuzz_targets/bench/lib.rs b/fuzz/fuzz_targets/bench/lib.rs index 6395ea2d6..a7e127669 100644 --- a/fuzz/fuzz_targets/bench/lib.rs +++ b/fuzz/fuzz_targets/bench/lib.rs @@ -4928,14 +4928,10 @@ impl<'a> SerdeDataType<'a> { } let value = SerdeDataValue::Struct { fields: r#struct }; for (ty, flatten) in fields.1.iter().zip(fields.2.iter()) { - if *flatten && !ty.supported_inside_untagged(pretty, false) { + if *flatten && !ty.supported_inside_untagged(pretty, false, false) { // Flattened fields are deserialised through serde's content type return Err(arbitrary::Error::IncorrectFormat); } - // if *flatten && pretty.extensions.contains(Extensions::IMPLICIT_SOME) { - // // BUG: implicit options are not supported inside flattend structs - // return Err(arbitrary::Error::IncorrectFormat); - // } if *flatten && pretty.struct_names { // BUG: struct names inside flattend structs do not roundtrip return Err(arbitrary::Error::IncorrectFormat); @@ -4955,16 +4951,7 @@ impl<'a> SerdeDataType<'a> { representation, SerdeEnumRepresentation::Untagged | SerdeEnumRepresentation::InternallyTagged { tag: _ } - if pretty.struct_names //|| pretty.extensions.contains(Extensions::IMPLICIT_SOME) - ) { - return Err(arbitrary::Error::IncorrectFormat); - } - - // BUG: implicit Some(()) inside untagged do not roundtrip - if matches!( - representation, - SerdeEnumRepresentation::Untagged - if pretty.extensions.contains(Extensions::IMPLICIT_SOME) + if pretty.struct_names ) { return Err(arbitrary::Error::IncorrectFormat); } @@ -5012,7 +4999,7 @@ impl<'a> SerdeDataType<'a> { } SerdeDataVariantType::Newtype { ref mut inner } => { let value = Box::new(inner.arbitrary_value(u, pretty)?); - if matches!(representation, SerdeEnumRepresentation::Untagged | SerdeEnumRepresentation::InternallyTagged { tag: _ } if !inner.supported_inside_untagged(pretty, true)) + if matches!(representation, SerdeEnumRepresentation::Untagged | SerdeEnumRepresentation::InternallyTagged { tag: _ } if !inner.supported_inside_untagged(pretty, true, false)) { return Err(arbitrary::Error::IncorrectFormat); } @@ -5054,7 +5041,7 @@ impl<'a> SerdeDataType<'a> { tuple.push(ty.arbitrary_value(u, pretty)?); } let value = SerdeDataVariantValue::Struct { fields: tuple }; - if matches!(representation, SerdeEnumRepresentation::Untagged if !fields.iter().all(|field| field.supported_inside_untagged(pretty, false))) + if matches!(representation, SerdeEnumRepresentation::Untagged | SerdeEnumRepresentation::InternallyTagged { tag: _ } if !fields.iter().all(|field| field.supported_inside_untagged(pretty, false, false))) { return Err(arbitrary::Error::IncorrectFormat); } @@ -5074,19 +5061,15 @@ impl<'a> SerdeDataType<'a> { r#struct.push(ty.arbitrary_value(u, pretty)?); } let value = SerdeDataVariantValue::Struct { fields: r#struct }; - if matches!(representation, SerdeEnumRepresentation::Untagged | SerdeEnumRepresentation::InternallyTagged { tag: _ } if !fields.1.iter().all(|field| field.supported_inside_untagged(pretty, false))) + if matches!(representation, SerdeEnumRepresentation::Untagged | SerdeEnumRepresentation::InternallyTagged { tag: _ } if !fields.1.iter().all(|field| field.supported_inside_untagged(pretty, false, false))) { return Err(arbitrary::Error::IncorrectFormat); } for (ty, flatten) in fields.1.iter().zip(fields.2.iter()) { - if *flatten && !ty.supported_inside_untagged(pretty, false) { + if *flatten && !ty.supported_inside_untagged(pretty, false, false) { // Flattened fields are deserialised through serde's content type return Err(arbitrary::Error::IncorrectFormat); } - // if *flatten && pretty.extensions.contains(Extensions::IMPLICIT_SOME) { - // // BUG: implicit options are not supported inside flattend structs - // return Err(arbitrary::Error::IncorrectFormat); - // } if *flatten && pretty.struct_names { // BUG: struct names inside flattend structs do not roundtrip return Err(arbitrary::Error::IncorrectFormat); @@ -5118,9 +5101,14 @@ impl<'a> SerdeDataType<'a> { &self, pretty: &PrettyConfig, inside_newtype_variant: bool, + inside_option: bool, ) -> bool { match self { - SerdeDataType::Unit => true, + SerdeDataType::Unit => { + // BUG: implicit `Some(())` is serialized as just `()`, + // which Option's deserializer accepts as `None` + !(inside_option && pretty.extensions.contains(Extensions::IMPLICIT_SOME)) + } SerdeDataType::Bool => true, SerdeDataType::I8 => true, SerdeDataType::I16 => true, @@ -5139,7 +5127,7 @@ impl<'a> SerdeDataType<'a> { SerdeDataType::Char => true, SerdeDataType::String => true, SerdeDataType::ByteBuf => true, - SerdeDataType::Option { inner } => inner.supported_inside_untagged(pretty, true), + SerdeDataType::Option { inner } => inner.supported_inside_untagged(pretty, true, true), SerdeDataType::Array { kind, len } => { if *len == 0 { // BUG: a zero-length array look like a unit to ron @@ -5156,7 +5144,7 @@ impl<'a> SerdeDataType<'a> { return false; } - kind.supported_inside_untagged(pretty, false) + kind.supported_inside_untagged(pretty, false, false) } SerdeDataType::Tuple { elems } => { if elems.is_empty() { @@ -5176,20 +5164,31 @@ impl<'a> SerdeDataType<'a> { elems .iter() - .all(|element| element.supported_inside_untagged(pretty, false)) + .all(|element| element.supported_inside_untagged(pretty, false, false)) } - SerdeDataType::Vec { item } => item.supported_inside_untagged(pretty, false), + SerdeDataType::Vec { item } => item.supported_inside_untagged(pretty, false, false), SerdeDataType::Map { key, value } => { - key.supported_inside_untagged(pretty, false) - && value.supported_inside_untagged(pretty, false) + key.supported_inside_untagged(pretty, false, false) + && value.supported_inside_untagged(pretty, false, false) + } + SerdeDataType::UnitStruct { name: _ } => { + // unit structs always serilize as units here since struct names + // are never allowed inside untagged + + // BUG: implicit `Some(())` is serialized as just `()`, + // which Option's deserializer accepts as `None` + !(inside_option && pretty.extensions.contains(Extensions::IMPLICIT_SOME)) } - SerdeDataType::UnitStruct { name: _ } => true, SerdeDataType::Newtype { name: _, inner: _ } => { // if *name == RAW_VALUE_TOKEN { // return false; // } - // inner.supported_inside_untagged(pretty, false) + // inner.supported_inside_untagged( + // pretty, + // false, + // inside_option && pretty.extensions.contains(Extensions::UNWRAP_NEWTYPES), + // ) // BUG: newtypes inside untagged look like 1-tuples to ron false @@ -5202,7 +5201,7 @@ impl<'a> SerdeDataType<'a> { fields .iter() - .all(|field| field.supported_inside_untagged(pretty, false)) + .all(|field| field.supported_inside_untagged(pretty, false, false)) } SerdeDataType::Struct { name: _, @@ -5219,7 +5218,7 @@ impl<'a> SerdeDataType<'a> { .iter() .zip(fields.2.iter()) .all(|(field, flatten)| { - field.supported_inside_untagged(pretty, false) + field.supported_inside_untagged(pretty, false, false) && (!*flatten || field.supported_inside_flatten(false)) }) } @@ -5228,11 +5227,22 @@ impl<'a> SerdeDataType<'a> { variants, representation, } => variants.1.iter().all(|variant| match variant { - SerdeDataVariantType::Unit => true, - SerdeDataVariantType::TaggedOther => true, - SerdeDataVariantType::Newtype { inner } => { - inner.supported_inside_untagged(pretty, true) + SerdeDataVariantType::Unit => { + // BUG: implicit `Some(())` is serialized as just `()`, + // which Option's deserializer accepts as `None` + !(inside_option + && pretty.extensions.contains(Extensions::IMPLICIT_SOME) + && matches!(representation, SerdeEnumRepresentation::Untagged)) } + SerdeDataVariantType::TaggedOther => true, + SerdeDataVariantType::Newtype { inner } => inner.supported_inside_untagged( + pretty, + true, + inside_option + && pretty + .extensions + .contains(Extensions::UNWRAP_VARIANT_NEWTYPES), + ), SerdeDataVariantType::Tuple { fields } => { if fields.is_empty() { // BUG: an empty tuple struct looks like a unit to ron @@ -5248,7 +5258,7 @@ impl<'a> SerdeDataType<'a> { fields .iter() - .all(|field| field.supported_inside_untagged(pretty, false)) + .all(|field| field.supported_inside_untagged(pretty, false, false)) } SerdeDataVariantType::Struct { fields } => { if fields.0.is_empty() { @@ -5261,7 +5271,7 @@ impl<'a> SerdeDataType<'a> { .iter() .zip(fields.2.iter()) .all(|(field, flatten)| { - field.supported_inside_untagged(pretty, false) + field.supported_inside_untagged(pretty, false, false) && (!*flatten || field.supported_inside_flatten(false)) }) } diff --git a/tests/502_known_bugs.rs b/tests/502_known_bugs.rs index 8f5761d0a..220cc0c8a 100644 --- a/tests/502_known_bugs.rs +++ b/tests/502_known_bugs.rs @@ -236,11 +236,11 @@ fn struct_names_inside_flatten_struct_variant() { ); } -/*#[test] +#[test] fn implicit_some_inside_internally_tagged() { #[derive(PartialEq, Debug, Serialize, Deserialize)] struct A { - hi: Option>, + hi: Option>, } #[derive(PartialEq, Debug, Serialize, Deserialize)] @@ -253,7 +253,7 @@ fn implicit_some_inside_internally_tagged() { check_roundtrip( &InternallyTagged::B { ho: 24, - a: A { hi: Some(Some(42)) } + a: A { hi: Some(Some(())) } }, PrettyConfig::default() ), @@ -263,17 +263,11 @@ fn implicit_some_inside_internally_tagged() { check_roundtrip( &InternallyTagged::B { ho: 24, - a: A { hi: Some(Some(42)) } + a: A { hi: Some(Some(())) } }, PrettyConfig::default().extensions(Extensions::IMPLICIT_SOME) ), - Err(Err(SpannedError { - code: Error::MissingStructField { - field: "hi", - outer: None - }, - position: Position { line: 7, col: 2 } - })), + Err(Ok(Error::Message(String::from("ROUNDTRIP error: B { ho: 24, a: A { hi: Some(Some(())) } } != B { ho: 24, a: A { hi: None } }")))) ); } @@ -281,7 +275,7 @@ fn implicit_some_inside_internally_tagged() { fn implicit_some_inside_adjacently_tagged() { #[derive(PartialEq, Debug, Serialize, Deserialize)] struct A { - hi: Option, + hi: Option>, } #[derive(PartialEq, Debug, Serialize, Deserialize)] @@ -294,7 +288,7 @@ fn implicit_some_inside_adjacently_tagged() { check_roundtrip( &AdjacentlyTagged::B { ho: 24, - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } }, PrettyConfig::default() ), @@ -304,7 +298,7 @@ fn implicit_some_inside_adjacently_tagged() { check_roundtrip( &AdjacentlyTagged::B { ho: 24, - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } }, PrettyConfig::default().extensions(Extensions::IMPLICIT_SOME) ), @@ -312,32 +306,29 @@ fn implicit_some_inside_adjacently_tagged() { ); assert_eq!( ron::from_str::( - "#[enable(implicit_some)] AdjacentlyTagged(tag: B, content: B(ho: 24, a: A(hi: 42)))" + "#![enable(implicit_some)] (tag: B, content: (ho: 24, a: A(hi: ())))" ), Ok(AdjacentlyTagged::B { ho: 24, - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } }), ); assert_eq!( ron::from_str::( - "#[enable(implicit_some)] AdjacentlyTagged(content: B(ho: 24, a: A(hi: 42)), tag: B)" + "#![enable(implicit_some)] (content: (ho: 24, a: A(hi: ())), tag: B)" ), - Err(SpannedError { - code: Error::MissingStructField { - field: "ho", - outer: Some(String::from("AdjacentlyTagged")) - }, - position: Position { line: 1, col: 58 } + Ok(AdjacentlyTagged::B { + ho: 24, + a: A { hi: None } // THIS IS WRONG }), ); -}*/ +} #[test] fn implicit_some_inside_untagged() { #[derive(PartialEq, Debug, Serialize, Deserialize)] struct A { - hi: Option<()>, + hi: Option>, } #[derive(PartialEq, Debug, Serialize, Deserialize)] @@ -350,7 +341,7 @@ fn implicit_some_inside_untagged() { check_roundtrip( &Untagged::B { ho: 24, - a: A { hi: Some(()) } + a: A { hi: Some(Some(())) } }, PrettyConfig::default() ), @@ -360,21 +351,21 @@ fn implicit_some_inside_untagged() { check_roundtrip( &Untagged::B { ho: 24, - a: A { hi: Some(()) } + a: A { hi: Some(Some(())) } }, PrettyConfig::default().extensions(Extensions::IMPLICIT_SOME) ), Err(Ok(Error::Message(String::from( - "ROUNDTRIP error: B { ho: 24, a: A { hi: Some(()) } } != B { ho: 24, a: A { hi: None } }" + "ROUNDTRIP error: B { ho: 24, a: A { hi: Some(Some(())) } } != B { ho: 24, a: A { hi: None } }" )))), ); } -/*#[test] +#[test] fn implicit_some_inside_flatten_struct() { #[derive(PartialEq, Debug, Serialize, Deserialize)] struct A { - hi: Option, + hi: Option>, } #[derive(PartialEq, Debug, Serialize, Deserialize)] @@ -394,7 +385,7 @@ fn implicit_some_inside_flatten_struct() { &FlattenedStruct { ho: 24, a: B { - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } } }, PrettyConfig::default() @@ -406,18 +397,12 @@ fn implicit_some_inside_flatten_struct() { &FlattenedStruct { ho: 24, a: B { - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } } }, PrettyConfig::default().extensions(Extensions::IMPLICIT_SOME) ), - Err(Err(SpannedError { - code: Error::MissingStructField { - field: "hi", - outer: None - }, - position: Position { line: 6, col: 1 } - })), + Err(Ok(Error::Message(String::from("ROUNDTRIP error: FlattenedStruct { ho: 24, a: B { a: A { hi: Some(Some(())) } } } != FlattenedStruct { ho: 24, a: B { a: A { hi: None } } }")))) ); } @@ -425,7 +410,7 @@ fn implicit_some_inside_flatten_struct() { fn implicit_some_inside_flatten_struct_variant() { #[derive(PartialEq, Debug, Serialize, Deserialize)] struct A { - hi: Option, + hi: Option>, } #[derive(PartialEq, Debug, Serialize, Deserialize)] @@ -447,7 +432,7 @@ fn implicit_some_inside_flatten_struct_variant() { &FlattenedStructVariant::C { ho: 24, a: B { - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } } }, PrettyConfig::default() @@ -459,20 +444,14 @@ fn implicit_some_inside_flatten_struct_variant() { &FlattenedStructVariant::C { ho: 24, a: B { - a: A { hi: Some(42) } + a: A { hi: Some(Some(())) } } }, PrettyConfig::default().extensions(Extensions::IMPLICIT_SOME) ), - Err(Err(SpannedError { - code: Error::MissingStructField { - field: "hi", - outer: Some(String::from("C")) - }, - position: Position { line: 6, col: 1 } - })), + Err(Ok(Error::Message(String::from("ROUNDTRIP error: C { ho: 24, a: B { a: A { hi: Some(Some(())) } } } != C { ho: 24, a: B { a: A { hi: None } } }")))) ); -}*/ +} #[test] fn one_tuple_variant_inside_internally_tagged() { @@ -1034,6 +1013,9 @@ fn check_roundtrip