Skip to content

Commit

Permalink
Add tests for one-tuple-variants inside untagged* + start with tests …
Browse files Browse the repository at this point in the history 
…for implicit Some inside untagged*
  • Loading branch information
@juntyr
juntyr committed Oct 2, 2023
1 parent 85ef8ab commit f97f9da
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 95 deletions.
9 changes: 5 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
90 changes: 50 additions & 40 deletions fuzz/fuzz_targets/bench/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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() {
Expand All @@ -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
Expand All @@ -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: _,
Expand All @@ -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))
})
}
Expand All @@ -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
Expand All @@ -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() {
Expand All @@ -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))
})
}
Expand Down
Loading

0 comments on commit f97f9da

Please sign in to comment.