Skip to content

Commit

Permalink
Better handle serde_as on enum variants #499
Browse files Browse the repository at this point in the history
Change the serde_as macro to warn on serde_as attributes on enum variant
fields. They are unsupported (in contrast to serde_derive), so showing
an error is better and avoids confusion when migrating.
Also update the main examples to include an enum.
  • Loading branch information
jonasbb committed Aug 14, 2022
1 parent 2519bef commit 7fa8f0b
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 28 deletions.
6 changes: 6 additions & 0 deletions serde_with/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
* `time` added support for the well-known `Iso8601` format.
This extends the existing support of `Rfc2822` and `Rfc3339`.

### Changed

* Warn if `serde_as` is used on an enum variant.
Attributes on enum variants were never supported.
But `#[serde(with = "...")]` can be added on variants, such that some confusion can occur when migration ([#499](https://github.com/jonasbb/serde_with/issues/499)).

## [2.0.0] - 2022-07-17

### Added
Expand Down
56 changes: 36 additions & 20 deletions serde_with/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@
//! The `serde_as` attribute allows circumventing this restriction, even for nested types and nested arrays.
//!
//! ```rust
//! # #[cfg(FALSE)] {
//! # #[cfg(feature = "macros")]
//! # use serde::{Deserialize, Serialize};
//! # #[cfg(feature = "macros")]
Expand Down Expand Up @@ -150,7 +149,6 @@
//! };
//! assert!(serde_json::to_string(&arrays).is_ok());
//! # }
//! # }
//! ```
//!
//! ## `skip_serializing_none`
Expand Down Expand Up @@ -196,7 +194,7 @@
//! ## Advanced `serde_as` usage
//!
//! This example is mainly supposed to highlight the flexibility of the `serde_as`-annotation compared to [serde's with-annotation][with-annotation].
//! More details about `serde_as` can be found in the [user guide][].
//! More details about `serde_as` can be found in the [user guide].
//!
//! ```rust
//! # #[cfg(all(feature = "macros", feature = "hex"))]
Expand All @@ -210,38 +208,56 @@
//! #[serde_as]
//! # #[derive(Debug, Eq, PartialEq)]
//! #[derive(Deserialize, Serialize)]
//! struct Foo {
//! // Serialize them into a list of number as seconds
//! #[serde_as(as = "Vec<DurationSeconds>")]
//! durations: Vec<Duration>,
//! // We can treat a Vec like a map with duplicates.
//! // JSON only allows string keys, so convert i32 to strings
//! // The bytes will be hex encoded
//! #[serde_as(as = "BTreeMap<DisplayFromStr, Hex>")]
//! bytes: Vec<(i32, Vec<u8>)>,
//! enum Foo {
//! Durations(
//! // Serialize them into a list of number as seconds
//! #[serde_as(as = "Vec<DurationSeconds>")]
//! Vec<Duration>,
//! ),
//! Bytes {
//! // We can treat a Vec like a map with duplicates.
//! // JSON only allows string keys, so convert i32 to strings
//! // The bytes will be hex encoded
//! #[serde_as(as = "BTreeMap<DisplayFromStr, Hex>")]
//! bytes: Vec<(i32, Vec<u8>)>,
//! }
//! }
//!
//! # #[cfg(all(feature = "macros", feature = "json", feature = "hex"))] {
//! // This will serialize
//! # let foo =
//! Foo {
//! durations: vec![Duration::new(5, 0), Duration::new(3600, 0), Duration::new(0, 0)],
//! Foo::Durations(
//! vec![Duration::new(5, 0), Duration::new(3600, 0), Duration::new(0, 0)]
//! )
//! # ;
//! // into this JSON
//! # let json = r#"
//! {
//! "Durations": [5, 3600, 0]
//! }
//! # "#;
//! # assert_eq!(json.replace(" ", "").replace("\n", ""), serde_json::to_string(&foo).unwrap());
//! # assert_eq!(foo, serde_json::from_str(&json).unwrap());
//!
//! // and serializes
//! # let foo =
//! Foo::Bytes {
//! bytes: vec![
//! (1, vec![0, 1, 2]),
//! (-100, vec![100, 200, 255]),
//! (1, vec![0, 111, 222]),
//! ],
//! }
//! # ;
//!
//! // into this JSON
//! # let json = r#"
//! {
//! "durations": [5, 3600, 0],
//! "bytes": {
//! "1": "000102",
//! "-100": "64c8ff",
//! "1": "006fde"
//! "Bytes": {
//! "bytes": {
//! "1": "000102",
//! "-100": "64c8ff",
//! "1": "006fde"
//! }
//! }
//! }
//! # "#;
Expand Down
6 changes: 6 additions & 0 deletions serde_with_macros/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

### Changed

* Warn if `serde_as` is used on an enum variant.
Attributes on enum variants were never supported.
But `#[serde(with = "...")]` can be added on variants, such that some confusion can occur when migration ([#499](https://github.com/jonasbb/serde_with/issues/499)).

## [2.0.0] - 2022-07-17

No changes compared to v2.0.0-rc.0.
Expand Down
41 changes: 33 additions & 8 deletions serde_with_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,16 +175,38 @@ where
input.attrs.push(consume_serde_as_attribute);
Ok(quote!(#input))
} else if let Ok(mut input) = syn::parse::<ItemEnum>(input) {
let errors: Vec<DarlingError> = input
// Prevent serde_as on enum variants
let mut errors: Vec<DarlingError> = input
.variants
.iter_mut()
.map(|variant| apply_on_fields(&mut variant.fields, function))
// turn the Err variant into the Some, such that we only collect errors
.filter_map(|res| match res {
Err(e) => Some(e),
Ok(()) => None,
.iter()
.flat_map(|variant| {
variant.attrs.iter().filter_map(|attr| {
if attr.path.is_ident("serde_as") {
Some(
DarlingError::custom(
"serde_as attribute is not allowed on enum variants",
)
.with_span(&attr),
)
} else {
None
}
})
})
.collect();
// Process serde_as on all fields
errors.extend(
input
.variants
.iter_mut()
.map(|variant| apply_on_fields(&mut variant.fields, function))
// turn the Err variant into the Some, such that we only collect errors
.filter_map(|res| match res {
Err(e) => Some(e),
Ok(()) => None,
}),
);

if errors.is_empty() {
input.attrs.push(consume_serde_as_attribute);
Ok(quote!(#input))
Expand Down Expand Up @@ -417,7 +439,10 @@ fn field_has_attribute(field: &Field, namespace: &str, name: &str) -> bool {
/// Convenience macro to use the [`serde_as`] system.
///
/// The [`serde_as`] system is designed as a more flexible alternative to serde's with-annotation.
/// The `#[serde_as]` attribute must be placed *before* the `#[derive]` attribute
/// The `#[serde_as]` attribute must be placed *before* the `#[derive]` attribute.
/// Each field of a struct or enum can be annotated with `#[serde_as(...)]` to specify which transformations should be applied.
/// `serde_as` is *not* supported on enum variants.
/// This is in contrast to `#[serde(with = "...")]`.
///
/// # Example
///
Expand Down
16 changes: 16 additions & 0 deletions serde_with_macros/tests/compile-fail/serde_as-enum-variant-499.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use serde::{Deserialize, Serialize};
use serde_with_macros::serde_as;

#[serde_as]
#[derive(Serialize, Deserialize)]
#[serde(tag = "messageType", content = "content")]
/// The contents of a message combined with the `MessageType`
pub enum MessageContentJsonStringEnum {
/// A normal message
Text(String),
/// Fancy object message
#[serde_as(as = "serde_with::json::JsonString")]
Object(String),
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: serde_as attribute is not allowed on enum variants
--> tests/compile-fail/serde_as-enum-variant-499.rs:12:5
|
12 | #[serde_as(as = "serde_with::json::JsonString")]
| ^

0 comments on commit 7fa8f0b

Please sign in to comment.