Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for parameterized Rust enums in serde #733

Merged
merged 3 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/serde/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,21 @@ impl<'a, 'de> de::Deserializer<'de> for ValueDeserializer<'a, 'de> {
use IonType::*;
match self.value.ion_type() {
String => visitor.visit_enum(UnitVariantAccess::new(self)),
Struct => visitor.visit_enum(VariantAccess::new(self)),
_ => IonResult::decoding_error("expected a string or struct enum representation"),
// All the parameterized Rust enums uses annotations for representing enum variant
_ => visitor.visit_enum(VariantAccess::new(self)),
}
}

fn deserialize_identifier<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where
V: Visitor<'de>,
{
visitor.visit_str(self.value.read()?.expect_text()?)
// annotations are currently only supported with parameterized Rust enums
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly does this mean? Does the rustdoc for this feature need to be updated as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that a serialized Ion data will only contain annotation in the case of parameterized Rust enums. And yes, I did add more comments in the rustdocs for it updating the table of serde data type to Ion type mappings: https://github.com/amazon-ion/ion-rust/pull/733/files#diff-de73824c75ee6203daf57ccbc5aee2ab3e2b46fbd9219d494c190298a7a8e745R43

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I saw that but didn't make the connection. It might be worth renaming the table to say "Ion representation" instead of "Ion data type" now that we're including more information than just the data type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I will change that.

if let Some(annotation) = self.value.annotations().next() {
visitor.visit_str(annotation?.text().unwrap())
} else {
visitor.visit_str(self.value.read()?.expect_text()?)
}
}

fn deserialize_ignored_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
Expand Down
65 changes: 61 additions & 4 deletions src/serde/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,20 @@
//!| list | vector | seq |
//!| null | None | unit |
//!
//! ## Mapping of serde data types to Ion data types
//! ## Mapping of serde data types to Ion representation
//!
//!| Serde data type | Ion data type |
//!| Serde data type | Ion representation |
//!|--------------------------------------------------------------|---------------------------------------------|
//!| u64, i64, u32, i32, u16, i16, u8, i8 | int |
//!| char, string, unit_variant | string |
//!| byte-array | blob |
//!| option | None - null, Some - based on other mappings |
//!| unit, unit_struct | null |
//!| seq, tuple, tuple_struct, tuple_variant | list |
//!| newtype_struct, newtype_variant, map, struct, struct_variant | struct |
//!| seq, tuple, tuple_struct | list |
//!| newtype_struct, map, struct | struct |
//!| newtype_variant | variant value with annotation |
//!| struct_variant | struct with annotation |
//!| tuple_variant | list with annotation |
//!
//! _Note: Since the serde framework doesn't support [Ion decimal] and [Ion timestamp] types, distinct serialization
//! and deserialization of these types are defined in this module. It uses `newtype_struct` with `$__ion_rs_decimal__`
Expand Down Expand Up @@ -223,6 +226,9 @@ mod tests {
#[serde_as(as = "crate::Timestamp")]
date1: DateTime<FixedOffset>,
nested_struct: NestedTest,
unit_struct: UnitStruct,
newtype_struct: NewTypeStruct,
tuple_struct: TupleStruct,
optional: Option<i64>,
}

Expand All @@ -233,6 +239,18 @@ mod tests {
str: String,
}

#[serde_as]
#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct UnitStruct;

#[serde_as]
#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct NewTypeStruct(i64);

#[serde_as]
#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct TupleStruct(i64, i64);

let datetime: DateTime<FixedOffset> = Utc::now().into();
let my_date0 = Utc::now();
let my_date = Timestamp::from(datetime);
Expand All @@ -250,6 +268,10 @@ mod tests {
boolean: true,
str: "hello".to_string(),
},

unit_struct: UnitStruct,
newtype_struct: NewTypeStruct(5),
tuple_struct: TupleStruct(5, 10),
optional: None,
};

Expand All @@ -269,6 +291,41 @@ mod tests {
assert_eq!(back_result.date1, datetime.clone());
assert!(back_result.nested_struct.boolean);
assert_eq!(&back_result.nested_struct.str, "hello");
assert_eq!(back_result.unit_struct, UnitStruct);
assert_eq!(back_result.newtype_struct, NewTypeStruct(5));
assert_eq!(back_result.tuple_struct, TupleStruct(5, 10));
assert_eq!(back_result.optional, None);
}

#[test]
fn test_enum() {
#[serde_as]
#[derive(Serialize, Deserialize, PartialEq, Debug)]
enum E {
Unit,
Newtype(u32),
Tuple(u32, u32),
Struct { a: u32 },
}

let i = r#""Unit""#;
let expected = E::Unit;
assert_eq!(expected, from_ion(i).unwrap());
assert_eq!(i, to_string(&expected).unwrap());

let i = r#"Newtype::1"#;
let expected = E::Newtype(1);
assert_eq!(expected, from_ion(i).unwrap());
assert_eq!(i, to_string(&expected).unwrap());

let i = r#"Tuple::[1, 2]"#;
let expected = E::Tuple(1, 2);
assert_eq!(expected, from_ion(i).unwrap());
assert_eq!(i, to_string(&expected).unwrap());

let i = r#"Struct::{a: 1}"#;
let expected = E::Struct { a: 1 };
assert_eq!(expected, from_ion(i).unwrap());
assert_eq!(i, to_string(&expected).unwrap());
}
}
12 changes: 6 additions & 6 deletions src/serde/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,8 @@ where
where
T: Serialize,
{
self.writer.step_in(IonType::Struct)?;
self.writer.set_field_name(variant);
value.serialize(&mut *self)?;
self.writer.step_out()
self.writer.set_annotations(vec![variant]);
value.serialize(&mut *self)
}

fn serialize_seq(self, _len: Option<usize>) -> Result<Self::SerializeSeq, Self::Error> {
Expand All @@ -257,9 +255,10 @@ where
self,
_name: &'static str,
_variant_index: u32,
_variant: &'static str,
variant: &'static str,
_len: usize,
) -> Result<Self::SerializeTupleVariant, Self::Error> {
self.writer.set_annotations(vec![variant]);
self.writer.step_in(IonType::List)?;
Ok(self)
}
Expand All @@ -282,9 +281,10 @@ where
self,
_name: &'static str,
_variant_index: u32,
_variant: &'static str,
variant: &'static str,
_len: usize,
) -> Result<Self::SerializeStructVariant, Self::Error> {
self.writer.set_annotations(vec![variant]);
self.writer.step_in(IonType::Struct)?;
Ok(self)
}
Expand Down
Loading