From fb20c9f854e39cddd236f724df85e4da8a809f35 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Mon, 18 Mar 2019 23:47:42 +0100 Subject: [PATCH 1/9] Add a proc_macro component to serde_with A proc_macro is required to implement skip_serializing_null from https://github.com/dtolnay/request-for-implementation/issues/18 Expose the macro crate in serde_with * Add a "macros"-feature to disable compiling proc_macros * Add the feature to the testing matrix --- .travis.yml | 2 +- CHANGELOG.md | 5 +++++ Cargo.toml | 8 ++++++++ serde_with_macros/Cargo.toml | 21 +++++++++++++++++++++ serde_with_macros/LICENSE-APACHE | 1 + serde_with_macros/LICENSE-MIT | 1 + serde_with_macros/README.md | 1 + serde_with_macros/src/lib.rs | 0 src/lib.rs | 7 +++++++ 9 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 serde_with_macros/Cargo.toml create mode 120000 serde_with_macros/LICENSE-APACHE create mode 120000 serde_with_macros/LICENSE-MIT create mode 120000 serde_with_macros/README.md create mode 100644 serde_with_macros/src/lib.rs diff --git a/.travis.yml b/.travis.yml index c4d15f59..e7305726 100644 --- a/.travis.yml +++ b/.travis.yml @@ -70,7 +70,7 @@ before_script: script: - set -e - | - for FEATURES_COMMAND in "--no-default-features" "" "--features=chrono" "--features=json" "--all-features" + for FEATURES_COMMAND in "--no-default-features" "" "--features=chrono" "--features=json" "--features=macros" "--all-features" do cargo build --verbose --all ${FEATURES_COMMAND} # skip this step if clippy is not available, e.g., bad nightly diff --git a/CHANGELOG.md b/CHANGELOG.md index 767f4d1e..9373ddbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Added + +* Add `skip_serializing_null` attribute, which adds `#[serde(skip_serializing_if = "Option::is_none")]` for each Option in a struct. + This is helpfull for APIs which have many optional fields. + ## [1.2.0] ### Added diff --git a/Cargo.toml b/Cargo.toml index b7796345..b60e3fa8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,3 +1,8 @@ +[workspace] +members = [ + "serde_with_macros", +] + [package] name = "serde_with" version = "1.2.0" @@ -26,12 +31,15 @@ codecov = { repository = "jonasbb/serde_with", branch = "master", service = "git maintenance = { status = "actively-developed" } [features] +default = [ "macros" ] json = [ "serde_json" ] +macros = [ "serde_with_macros" ] [dependencies] chrono = { version = "0.4.1", features = [ "serde" ], optional = true } serde = "1.0.75" serde_json = { version = "1.0.1", optional = true } +serde_with_macros = { path = "./serde_with_macros", version = "0.1.0", optional = true} [dev-dependencies] fnv = "1.0.6" diff --git a/serde_with_macros/Cargo.toml b/serde_with_macros/Cargo.toml new file mode 100644 index 00000000..f8e72d92 --- /dev/null +++ b/serde_with_macros/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "serde_with_macros" +version = "0.1.0" +authors = ["jonasbb"] + +description = "proc-macro library for serde_with" +documentation = "https://docs.rs/serde_with_macros/" +repository = "https://github.com/jonasbb/serde_with/serde_with_macros" +readme = "README.md" +keywords = ["serde", "utilities", "serialization", "deserialization"] +license = "MIT/Apache-2.0" + +[lib] +proc-macro = true + +[badges] +travis-ci = { repository = "jonasbb/serde_with", branch = "master" } +codecov = { repository = "jonasbb/serde_with", branch = "master", service = "github" } +maintenance = { status = "actively-developed" } + +[dependencies] diff --git a/serde_with_macros/LICENSE-APACHE b/serde_with_macros/LICENSE-APACHE new file mode 120000 index 00000000..965b606f --- /dev/null +++ b/serde_with_macros/LICENSE-APACHE @@ -0,0 +1 @@ +../LICENSE-APACHE \ No newline at end of file diff --git a/serde_with_macros/LICENSE-MIT b/serde_with_macros/LICENSE-MIT new file mode 120000 index 00000000..76219eb7 --- /dev/null +++ b/serde_with_macros/LICENSE-MIT @@ -0,0 +1 @@ +../LICENSE-MIT \ No newline at end of file diff --git a/serde_with_macros/README.md b/serde_with_macros/README.md new file mode 120000 index 00000000..32d46ee8 --- /dev/null +++ b/serde_with_macros/README.md @@ -0,0 +1 @@ +../README.md \ No newline at end of file diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs new file mode 100644 index 00000000..e69de29b diff --git a/src/lib.rs b/src/lib.rs index c0bfefee..0101fcad 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,6 +83,8 @@ extern crate chrono as chrono_crate; pub extern crate serde; #[cfg(feature = "json")] extern crate serde_json; +#[cfg(feature = "macros")] +extern crate serde_with_macros; #[cfg(feature = "chrono")] pub mod chrono; @@ -94,6 +96,11 @@ pub mod rust; #[doc(hidden)] pub mod with_prefix; +// Re-Export all proc_macros, as these should be seen as part of the serde_with crate +#[cfg(feature = "macros")] +#[doc(inline)] +pub use serde_with_macros::*; + /// Separator for string-based collection de/serialization pub trait Separator { /// Return the string delimiting two elements in the string-based collection From 62866b8ff100da8ed8b4c3144ba0a9aae05b6538 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Tue, 19 Mar 2019 20:12:58 +0100 Subject: [PATCH 2/9] Add first version supporting named structs --- Cargo.toml | 2 +- serde_with_macros/Cargo.toml | 24 ++++++- serde_with_macros/src/lib.rs | 71 +++++++++++++++++++ .../tests/skip_serializing_null.rs | 65 +++++++++++++++++ 4 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 serde_with_macros/tests/skip_serializing_null.rs diff --git a/Cargo.toml b/Cargo.toml index b60e3fa8..449f0419 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,7 +39,7 @@ macros = [ "serde_with_macros" ] chrono = { version = "0.4.1", features = [ "serde" ], optional = true } serde = "1.0.75" serde_json = { version = "1.0.1", optional = true } -serde_with_macros = { path = "./serde_with_macros", version = "0.1.0", optional = true} +serde_with_macros = { path = "./serde_with_macros", version = "1.0.0", optional = true} [dev-dependencies] fnv = "1.0.6" diff --git a/serde_with_macros/Cargo.toml b/serde_with_macros/Cargo.toml index f8e72d92..81cd0b57 100644 --- a/serde_with_macros/Cargo.toml +++ b/serde_with_macros/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "serde_with_macros" -version = "0.1.0" +version = "1.0.0" authors = ["jonasbb"] description = "proc-macro library for serde_with" @@ -19,3 +19,25 @@ codecov = { repository = "jonasbb/serde_with", branch = "master", service = "git maintenance = { status = "actively-developed" } [dependencies] +proc-macro2 = { version = "0.4.27" } +quote = "0.6.11" + +[dependencies.syn] +version = "0.15.29" +default-features = false +features = [ + "extra-traits", # Only for debugging + "full", + "parsing", + "printing", + "proc-macro", +] + +[dev-dependencies] +pretty_assertions = "0.5.1" +serde = { version = "1.0.75", features = [ "derive" ] } +serde_json = "1.0.25" +version-sync = "0.7.0" + +[package.metadata.docs.rs] +all-features = true diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index e69de29b..093973b8 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -0,0 +1,71 @@ +#![deny( + missing_debug_implementations, + missing_copy_implementations, + trivial_casts, + trivial_numeric_casts, + unused_extern_crates, + unused_import_braces, + unused_qualifications, + variant_size_differences +)] +#![cfg_attr(feature = "cargo-clippy", allow(renamed_and_removed_lints))] +#![doc(html_root_url = "https://docs.rs/serde_with_macros/1.0.0")] + +extern crate proc_macro; +extern crate quote; +extern crate syn; + +use proc_macro::TokenStream; +use quote::quote; +use syn::{parse::Parser, parse_macro_input, Attribute, Fields, ItemStruct, Path, Type}; + +/// Return `true`, if the type path refers to `std::option::Option` +/// +/// Accepts +/// +/// * `Option` +/// * `std::option::Option`, with or without leading `::` +/// * `core::option::Option`, with or without leading `::` +fn is_std_option(path: &Path) -> bool { + (path.leading_colon.is_none() && path.segments.len() == 1 && path.segments[0].ident == "Option") + || (path.segments.len() == 3 + && (path.segments[0].ident == "std" || path.segments[0].ident == "core") + && path.segments[1].ident == "option" + && path.segments[2].ident == "Option") +} + +#[proc_macro_attribute] +pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStream { + let mut input = parse_macro_input!(input as ItemStruct); + + // For each field in the struct given by `input`, add the `skip_serializing_if` attribute, + // if and only if, it is of type `Option` + match &mut input.fields { + // simple, no fields, do nothing + Fields::Unit => {} + + Fields::Named(ref mut fields) => { + fields.named.iter_mut().for_each(|field| { + if let Type::Path(path) = &field.ty { + if is_std_option(&path.path) { + // FIXME if skip_serializing_if already exists, do not add it again + // Add the `skip_serializing_if` attribute + let attr_tokens = quote!( + #[serde(skip_serializing_if = "Option::is_none")] + ); + let parser = Attribute::parse_outer; + let attrs = parser + .parse2(attr_tokens) + .expect("Static attr tokens should not panic"); + field.attrs.extend(attrs); + } + } + }) + } + + Fields::Unnamed(ref mut _fields) => unimplemented!("Unnamed"), + }; + + // Hand the modified input back to the compiler + TokenStream::from(quote!(#input)) +} diff --git a/serde_with_macros/tests/skip_serializing_null.rs b/serde_with_macros/tests/skip_serializing_null.rs new file mode 100644 index 00000000..d623de84 --- /dev/null +++ b/serde_with_macros/tests/skip_serializing_null.rs @@ -0,0 +1,65 @@ +extern crate pretty_assertions; +extern crate serde; +extern crate serde_json; +extern crate serde_with_macros; + +use pretty_assertions::assert_eq; +use serde::{Deserialize, Serialize}; +use serde_json::json; +use serde_with_macros::skip_serializing_null; + +macro_rules! test { + ($fn:ident, $struct:ident) => { + #[test] + fn $fn() { + let expected = json!({}); + let data = $struct { + a: None, + b: None, + c: None, + d: None, + }; + let res = serde_json::to_value(&data).unwrap(); + assert_eq!(expected, res); + assert_eq!(data, serde_json::from_value(res).unwrap()); + } + }; +} + +macro_rules! test_tuple { + ($fn:ident, $struct:ident) => { + #[test] + fn $fn() { + let expected = json!({}); + let data = $struct(None, None); + let res = serde_json::to_value(&data).unwrap(); + assert_eq!(expected, res); + assert_eq!(data, serde_json::from_value(res).unwrap()); + } + }; +} + +#[skip_serializing_null] +#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] +struct DataBasic { + a: Option, + b: Option, + c: Option, + d: Option, +} +test!(test_basic, DataBasic); + +#[skip_serializing_null] +#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] +struct DataFullyQualified { + a: ::std::option::Option, + b: std::option::Option, + c: ::std::option::Option, + d: core::option::Option, +} +test!(test_fully_qualified, DataFullyQualified); + +#[skip_serializing_null] +#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] +struct DataTuple(Option, std::option::Option); +test_tuple!(test_tuple, DataTuple); From 71bfd0340a782c36ba5dbf20a4def887c6124c7e Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Tue, 19 Mar 2019 20:16:08 +0100 Subject: [PATCH 3/9] Add html_root_url test --- serde_with_macros/tests/version_numbers.rs | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 serde_with_macros/tests/version_numbers.rs diff --git a/serde_with_macros/tests/version_numbers.rs b/serde_with_macros/tests/version_numbers.rs new file mode 100644 index 00000000..9cbd3b3a --- /dev/null +++ b/serde_with_macros/tests/version_numbers.rs @@ -0,0 +1,6 @@ +extern crate version_sync; + +#[test] +fn test_html_root_url() { + version_sync::assert_html_root_url_updated!("src/lib.rs"); +} From e0d07d879807037daff59fb1b2dd8436227ee234 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Tue, 19 Mar 2019 22:03:03 +0100 Subject: [PATCH 4/9] Add support for enums in addition to tuples --- serde_with_macros/src/lib.rs | 78 ++++++++++++------- .../tests/skip_serializing_null.rs | 32 +++++++- 2 files changed, 80 insertions(+), 30 deletions(-) diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index 093973b8..b69cc22b 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -12,12 +12,14 @@ #![doc(html_root_url = "https://docs.rs/serde_with_macros/1.0.0")] extern crate proc_macro; +extern crate proc_macro2; extern crate quote; extern crate syn; use proc_macro::TokenStream; +use proc_macro2::Span; use quote::quote; -use syn::{parse::Parser, parse_macro_input, Attribute, Fields, ItemStruct, Path, Type}; +use syn::{parse::Parser, Attribute, Error, Field, Fields, ItemEnum, ItemStruct, Path, Type}; /// Return `true`, if the type path refers to `std::option::Option` /// @@ -34,38 +36,60 @@ fn is_std_option(path: &Path) -> bool { && path.segments[2].ident == "Option") } -#[proc_macro_attribute] -pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStream { - let mut input = parse_macro_input!(input as ItemStruct); +/// Add the skip_serializing_if annotation to each field of the struct +fn skip_serializing_null_add_attr_to_field<'a>(fields: impl IntoIterator) { + fields.into_iter().for_each(|field| { + if let Type::Path(path) = &field.ty { + if is_std_option(&path.path) { + // FIXME if skip_serializing_if already exists, do not add it again + // Add the `skip_serializing_if` attribute + let attr_tokens = quote!( + #[serde(skip_serializing_if = "Option::is_none")] + ); + let parser = Attribute::parse_outer; + let attrs = parser + .parse2(attr_tokens) + .expect("Static attr tokens should not panic"); + field.attrs.extend(attrs); + } + } + }) +} - // For each field in the struct given by `input`, add the `skip_serializing_if` attribute, - // if and only if, it is of type `Option` - match &mut input.fields { +/// Handle a single struct or a single enum variant +fn skip_serializing_null_handle_fields(fields: &mut Fields) { + match fields { // simple, no fields, do nothing Fields::Unit => {} - Fields::Named(ref mut fields) => { - fields.named.iter_mut().for_each(|field| { - if let Type::Path(path) = &field.ty { - if is_std_option(&path.path) { - // FIXME if skip_serializing_if already exists, do not add it again - // Add the `skip_serializing_if` attribute - let attr_tokens = quote!( - #[serde(skip_serializing_if = "Option::is_none")] - ); - let parser = Attribute::parse_outer; - let attrs = parser - .parse2(attr_tokens) - .expect("Static attr tokens should not panic"); - field.attrs.extend(attrs); - } - } - }) + skip_serializing_null_add_attr_to_field(fields.named.iter_mut()) + } + Fields::Unnamed(ref mut fields) => { + skip_serializing_null_add_attr_to_field(fields.unnamed.iter_mut()) } - - Fields::Unnamed(ref mut _fields) => unimplemented!("Unnamed"), }; +} +#[proc_macro_attribute] +pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStream { + // For each field in the struct given by `input`, add the `skip_serializing_if` attribute, + // if and only if, it is of type `Option` + let res = if let Ok(mut input) = syn::parse::(input.clone()) { + skip_serializing_null_handle_fields(&mut input.fields); + quote!(#input) + } else if let Ok(mut input) = syn::parse::(input.clone()) { + input.variants.iter_mut().for_each(|variant| { + skip_serializing_null_handle_fields(&mut variant.fields); + }); + quote!(#input) + } else { + let span = Span::call_site(); + Error::new( + span, + "The attribute can only be applied to struct or enum definitions.", + ) + .to_compile_error() + }; // Hand the modified input back to the compiler - TokenStream::from(quote!(#input)) + TokenStream::from(res) } diff --git a/serde_with_macros/tests/skip_serializing_null.rs b/serde_with_macros/tests/skip_serializing_null.rs index d623de84..22201179 100644 --- a/serde_with_macros/tests/skip_serializing_null.rs +++ b/serde_with_macros/tests/skip_serializing_null.rs @@ -30,11 +30,10 @@ macro_rules! test_tuple { ($fn:ident, $struct:ident) => { #[test] fn $fn() { - let expected = json!({}); + let expected = json!([]); let data = $struct(None, None); let res = serde_json::to_value(&data).unwrap(); assert_eq!(expected, res); - assert_eq!(data, serde_json::from_value(res).unwrap()); } }; } @@ -60,6 +59,33 @@ struct DataFullyQualified { test!(test_fully_qualified, DataFullyQualified); #[skip_serializing_null] -#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Eq, PartialEq, Serialize)] struct DataTuple(Option, std::option::Option); test_tuple!(test_tuple, DataTuple); + +#[skip_serializing_null] +#[derive(Debug, Eq, PartialEq, Serialize)] +enum DataEnum { + Tuple(Option, std::option::Option), + Struct { + a: Option, + b: Option, + }, +} + +#[test] +fn test_enum() { + let expected = json!({ + "Tuple": [] + }); + let data = DataEnum::Tuple(None, None); + let res = serde_json::to_value(&data).unwrap(); + assert_eq!(expected, res); + + let expected = json!({ + "Struct": {} + }); + let data = DataEnum::Struct { a: None, b: None }; + let res = serde_json::to_value(&data).unwrap(); + assert_eq!(expected, res); +} From ffcaf000f9d21da541707c0e24204bb32c87c836 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Wed, 20 Mar 2019 00:08:54 +0100 Subject: [PATCH 5/9] Do not add attribute, if there is an existing skip-serialization_if --- serde_with_macros/src/lib.rs | 48 ++++++++++++++++++- .../tests/skip_serializing_null.rs | 34 +++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index b69cc22b..235c4043 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -19,7 +19,10 @@ extern crate syn; use proc_macro::TokenStream; use proc_macro2::Span; use quote::quote; -use syn::{parse::Parser, Attribute, Error, Field, Fields, ItemEnum, ItemStruct, Path, Type}; +use syn::{ + parse::Parser, Attribute, Error, Field, Fields, ItemEnum, ItemStruct, Meta, NestedMeta, Path, + Type, +}; /// Return `true`, if the type path refers to `std::option::Option` /// @@ -36,11 +39,54 @@ fn is_std_option(path: &Path) -> bool { && path.segments[2].ident == "Option") } +/// Determine if the `field` has an attribute with given `namespace` and `name` +/// +/// On the example of +/// `#[serde(skip_serializing_if = "Option::is_none")]` +// +/// * `serde` is the outermost path, here namespace +/// * it contains a Meta::List +/// * which contains in another Meta a Meta::NameValue +/// * with the name being `skip_serializing_if` +#[cfg_attr(feature = "cargo-clippy", allow(cmp_owned))] +fn field_has_attribute(field: &Field, namespace: &str, name: &str) -> bool { + // On the example of + // #[serde(skip_serializing_if = "Option::is_none")] + // + // `serde` is the outermost path, here namespace + // it contains a Meta::List + // which contains in another Meta a Meta::NameValue + // with the name being `skip_serializing_if` + + for attr in &field.attrs { + if attr.path.is_ident(namespace) { + // Ignore non parsable attributes, as these are not important for us + if let Ok(expr) = attr.parse_meta() { + if let Meta::List(expr) = expr { + for expr in expr.nested { + if let NestedMeta::Meta(Meta::NameValue(expr)) = expr { + if expr.ident.to_string() == name { + return true; + } + } + } + } + } + } + } + false +} + /// Add the skip_serializing_if annotation to each field of the struct fn skip_serializing_null_add_attr_to_field<'a>(fields: impl IntoIterator) { fields.into_iter().for_each(|field| { if let Type::Path(path) = &field.ty { if is_std_option(&path.path) { + // Do nothing, if the value already exists + if field_has_attribute(&field, "serde", "skip_serializing_if") { + return; + } + // FIXME if skip_serializing_if already exists, do not add it again // Add the `skip_serializing_if` attribute let attr_tokens = quote!( diff --git a/serde_with_macros/tests/skip_serializing_null.rs b/serde_with_macros/tests/skip_serializing_null.rs index 22201179..34217a4f 100644 --- a/serde_with_macros/tests/skip_serializing_null.rs +++ b/serde_with_macros/tests/skip_serializing_null.rs @@ -58,6 +58,40 @@ struct DataFullyQualified { } test!(test_fully_qualified, DataFullyQualified); +fn never(_t: &T) -> bool { + false +} + +#[skip_serializing_null] +#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] +struct DataExistingAnnotation { + #[serde(skip_serializing_if = "Option::is_none")] + a: Option, + #[serde(default, skip_serializing_if = "Option::is_none", rename = "abc")] + b: Option, + #[serde(default)] + c: Option, + #[serde(skip_serializing_if = "never")] + #[serde(rename = "name")] + d: Option, +} + +#[test] +fn test_existing_annotation() { + let expected = json!({ + "name": null + }); + let data = DataExistingAnnotation { + a: None, + b: None, + c: None, + d: None, + }; + let res = serde_json::to_value(&data).unwrap(); + assert_eq!(expected, res); + assert_eq!(data, serde_json::from_value(res).unwrap()); +} + #[skip_serializing_null] #[derive(Debug, Eq, PartialEq, Serialize)] struct DataTuple(Option, std::option::Option); From f7f64e336155276c741663bafc94d033b0cb16eb Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Thu, 21 Mar 2019 22:18:34 +0100 Subject: [PATCH 6/9] Also parse `serialize_always` --- CHANGELOG.md | 2 + serde_with_macros/src/lib.rs | 109 ++++++++++++------ .../tests/skip_serializing_null.rs | 35 +++++- 3 files changed, 108 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9373ddbd..14b6127d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Add `skip_serializing_null` attribute, which adds `#[serde(skip_serializing_if = "Option::is_none")]` for each Option in a struct. This is helpfull for APIs which have many optional fields. + The effect of can be negated by adding `serialize_always` on those fields, which should always be serialized. + Existing `skip_serializing_if` will never be modified and those fields keep their behavior. ## [1.2.0] diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index 235c4043..73b920a5 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -24,6 +24,36 @@ use syn::{ Type, }; +#[proc_macro_attribute] +pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStream { + let res = match skip_serializing_null_do(input) { + Ok(res) => res, + Err(msg) => { + let span = Span::call_site(); + Error::new(span, msg).to_compile_error() + } + }; + TokenStream::from(res) +} + +fn skip_serializing_null_do(input: TokenStream) -> Result { + // For each field in the struct given by `input`, add the `skip_serializing_if` attribute, + // if and only if, it is of type `Option` + if let Ok(mut input) = syn::parse::(input.clone()) { + skip_serializing_null_handle_fields(&mut input.fields)?; + Ok(quote!(#input)) + } else if let Ok(mut input) = syn::parse::(input.clone()) { + input + .variants + .iter_mut() + .map(|variant| skip_serializing_null_handle_fields(&mut variant.fields)) + .collect::>()?; + Ok(quote!(#input)) + } else { + Err("The attribute can only be applied to struct or enum definitions.".into()) + } +} + /// Return `true`, if the type path refers to `std::option::Option` /// /// Accepts @@ -78,16 +108,40 @@ fn field_has_attribute(field: &Field, namespace: &str, name: &str) -> bool { } /// Add the skip_serializing_if annotation to each field of the struct -fn skip_serializing_null_add_attr_to_field<'a>(fields: impl IntoIterator) { - fields.into_iter().for_each(|field| { - if let Type::Path(path) = &field.ty { +fn skip_serializing_null_add_attr_to_field<'a>( + fields: impl IntoIterator, +) -> Result<(), String> { + fields.into_iter().map(|field| ->Result<(), String> { + if let Type::Path(path) = &field.ty.clone() { if is_std_option(&path.path) { - // Do nothing, if the value already exists - if field_has_attribute(&field, "serde", "skip_serializing_if") { - return; + let has_skip_serializing_if = + field_has_attribute(&field, "serde", "skip_serializing_if"); + + // Remove the `serialize_always` attribute + let mut has_always_attr = false; + field.attrs.retain(|attr| { + let has_attr = attr.path.is_ident("serialize_always"); + has_always_attr |= has_attr; + !has_attr + }); + + // Error on conflicting attributes + if has_always_attr && has_skip_serializing_if { + let mut msg = r#"The attributes `serialize_always` and `serde(skip_serializing_if = "...")` cannot be used on the same field"#.to_string(); + if let Some(ident) = &field.ident { + msg += ": `"; + msg += &ident.to_string(); + msg += "`"; + } + msg +="."; + return Err(msg); + } + + // Do nothing if `skip_serializing_if` or `serialize_always` is already present + if has_skip_serializing_if || has_always_attr { + return Ok(()); } - // FIXME if skip_serializing_if already exists, do not add it again // Add the `skip_serializing_if` attribute let attr_tokens = quote!( #[serde(skip_serializing_if = "Option::is_none")] @@ -97,45 +151,30 @@ fn skip_serializing_null_add_attr_to_field<'a>(fields: impl IntoIterator Result<(), String> { match fields { // simple, no fields, do nothing - Fields::Unit => {} + Fields::Unit => Ok(()), Fields::Named(ref mut fields) => { skip_serializing_null_add_attr_to_field(fields.named.iter_mut()) } Fields::Unnamed(ref mut fields) => { skip_serializing_null_add_attr_to_field(fields.unnamed.iter_mut()) } - }; -} - -#[proc_macro_attribute] -pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStream { - // For each field in the struct given by `input`, add the `skip_serializing_if` attribute, - // if and only if, it is of type `Option` - let res = if let Ok(mut input) = syn::parse::(input.clone()) { - skip_serializing_null_handle_fields(&mut input.fields); - quote!(#input) - } else if let Ok(mut input) = syn::parse::(input.clone()) { - input.variants.iter_mut().for_each(|variant| { - skip_serializing_null_handle_fields(&mut variant.fields); - }); - quote!(#input) - } else { - let span = Span::call_site(); - Error::new( - span, - "The attribute can only be applied to struct or enum definitions.", - ) - .to_compile_error() - }; - // Hand the modified input back to the compiler - TokenStream::from(res) + } } diff --git a/serde_with_macros/tests/skip_serializing_null.rs b/serde_with_macros/tests/skip_serializing_null.rs index 34217a4f..7fe1595c 100644 --- a/serde_with_macros/tests/skip_serializing_null.rs +++ b/serde_with_macros/tests/skip_serializing_null.rs @@ -78,9 +78,7 @@ struct DataExistingAnnotation { #[test] fn test_existing_annotation() { - let expected = json!({ - "name": null - }); + let expected = json!({ "name": null }); let data = DataExistingAnnotation { a: None, b: None, @@ -92,6 +90,37 @@ fn test_existing_annotation() { assert_eq!(data, serde_json::from_value(res).unwrap()); } +#[skip_serializing_null] +#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] +struct DataSerializeAlways { + #[serialize_always] + a: Option, + #[serialize_always] + b: Option, + c: i64, + #[serialize_always] + d: Option, +} + +#[test] +fn test_serialize_always() { + let expected = json!({ + "a": null, + "b": null, + "c": 0, + "d": null + }); + let data = DataSerializeAlways { + a: None, + b: None, + c: 0, + d: None, + }; + let res = serde_json::to_value(&data).unwrap(); + assert_eq!(expected, res); + assert_eq!(data, serde_json::from_value(res).unwrap()); +} + #[skip_serializing_null] #[derive(Debug, Eq, PartialEq, Serialize)] struct DataTuple(Option, std::option::Option); From 5ae2c21ecab5261fa694207d3d61b056b552ef25 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Sat, 23 Mar 2019 19:29:35 +0100 Subject: [PATCH 7/9] Rename macro to skip_serializing_none --- CHANGELOG.md | 2 +- serde_with_macros/src/lib.rs | 18 +++++++++--------- .../tests/skip_serializing_null.rs | 14 +++++++------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14b6127d..96b0dcc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added -* Add `skip_serializing_null` attribute, which adds `#[serde(skip_serializing_if = "Option::is_none")]` for each Option in a struct. +* Add `skip_serializing_none` attribute, which adds `#[serde(skip_serializing_if = "Option::is_none")]` for each Option in a struct. This is helpfull for APIs which have many optional fields. The effect of can be negated by adding `serialize_always` on those fields, which should always be serialized. Existing `skip_serializing_if` will never be modified and those fields keep their behavior. diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index 73b920a5..05b6ecf8 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -25,8 +25,8 @@ use syn::{ }; #[proc_macro_attribute] -pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStream { - let res = match skip_serializing_null_do(input) { +pub fn skip_serializing_none(_args: TokenStream, input: TokenStream) -> TokenStream { + let res = match skip_serializing_none_do(input) { Ok(res) => res, Err(msg) => { let span = Span::call_site(); @@ -36,17 +36,17 @@ pub fn skip_serializing_null(_args: TokenStream, input: TokenStream) -> TokenStr TokenStream::from(res) } -fn skip_serializing_null_do(input: TokenStream) -> Result { +fn skip_serializing_none_do(input: TokenStream) -> Result { // For each field in the struct given by `input`, add the `skip_serializing_if` attribute, // if and only if, it is of type `Option` if let Ok(mut input) = syn::parse::(input.clone()) { - skip_serializing_null_handle_fields(&mut input.fields)?; + skip_serializing_none_handle_fields(&mut input.fields)?; Ok(quote!(#input)) } else if let Ok(mut input) = syn::parse::(input.clone()) { input .variants .iter_mut() - .map(|variant| skip_serializing_null_handle_fields(&mut variant.fields)) + .map(|variant| skip_serializing_none_handle_fields(&mut variant.fields)) .collect::>()?; Ok(quote!(#input)) } else { @@ -108,7 +108,7 @@ fn field_has_attribute(field: &Field, namespace: &str, name: &str) -> bool { } /// Add the skip_serializing_if annotation to each field of the struct -fn skip_serializing_null_add_attr_to_field<'a>( +fn skip_serializing_none_add_attr_to_field<'a>( fields: impl IntoIterator, ) -> Result<(), String> { fields.into_iter().map(|field| ->Result<(), String> { @@ -166,15 +166,15 @@ fn skip_serializing_null_add_attr_to_field<'a>( } /// Handle a single struct or a single enum variant -fn skip_serializing_null_handle_fields(fields: &mut Fields) -> Result<(), String> { +fn skip_serializing_none_handle_fields(fields: &mut Fields) -> Result<(), String> { match fields { // simple, no fields, do nothing Fields::Unit => Ok(()), Fields::Named(ref mut fields) => { - skip_serializing_null_add_attr_to_field(fields.named.iter_mut()) + skip_serializing_none_add_attr_to_field(fields.named.iter_mut()) } Fields::Unnamed(ref mut fields) => { - skip_serializing_null_add_attr_to_field(fields.unnamed.iter_mut()) + skip_serializing_none_add_attr_to_field(fields.unnamed.iter_mut()) } } } diff --git a/serde_with_macros/tests/skip_serializing_null.rs b/serde_with_macros/tests/skip_serializing_null.rs index 7fe1595c..7fb43bc9 100644 --- a/serde_with_macros/tests/skip_serializing_null.rs +++ b/serde_with_macros/tests/skip_serializing_null.rs @@ -6,7 +6,7 @@ extern crate serde_with_macros; use pretty_assertions::assert_eq; use serde::{Deserialize, Serialize}; use serde_json::json; -use serde_with_macros::skip_serializing_null; +use serde_with_macros::skip_serializing_none; macro_rules! test { ($fn:ident, $struct:ident) => { @@ -38,7 +38,7 @@ macro_rules! test_tuple { }; } -#[skip_serializing_null] +#[skip_serializing_none] #[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] struct DataBasic { a: Option, @@ -48,7 +48,7 @@ struct DataBasic { } test!(test_basic, DataBasic); -#[skip_serializing_null] +#[skip_serializing_none] #[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] struct DataFullyQualified { a: ::std::option::Option, @@ -62,7 +62,7 @@ fn never(_t: &T) -> bool { false } -#[skip_serializing_null] +#[skip_serializing_none] #[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] struct DataExistingAnnotation { #[serde(skip_serializing_if = "Option::is_none")] @@ -90,7 +90,7 @@ fn test_existing_annotation() { assert_eq!(data, serde_json::from_value(res).unwrap()); } -#[skip_serializing_null] +#[skip_serializing_none] #[derive(Debug, Eq, PartialEq, Serialize, Deserialize)] struct DataSerializeAlways { #[serialize_always] @@ -121,12 +121,12 @@ fn test_serialize_always() { assert_eq!(data, serde_json::from_value(res).unwrap()); } -#[skip_serializing_null] +#[skip_serializing_none] #[derive(Debug, Eq, PartialEq, Serialize)] struct DataTuple(Option, std::option::Option); test_tuple!(test_tuple, DataTuple); -#[skip_serializing_null] +#[skip_serializing_none] #[derive(Debug, Eq, PartialEq, Serialize)] enum DataEnum { Tuple(Option, std::option::Option), From f9d5fa0be347c0a74bf3a133f441f761e3de88a3 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Sun, 31 Mar 2019 22:58:19 +0200 Subject: [PATCH 8/9] Add documentation for skip_serializing_none --- serde_with_macros/src/lib.rs | 98 ++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index 05b6ecf8..1d51a93a 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -11,6 +11,12 @@ #![cfg_attr(feature = "cargo-clippy", allow(renamed_and_removed_lints))] #![doc(html_root_url = "https://docs.rs/serde_with_macros/1.0.0")] +//! proc-macro extensions for [`serde_with`] +//! +//! This crate should not be used alone, but through the [`serde_with`] crate. +//! +//! [`serde_with`]: https://crates.io/crates/serde_with/ + extern crate proc_macro; extern crate proc_macro2; extern crate quote; @@ -24,6 +30,98 @@ use syn::{ Type, }; +/// Add `skip_serializing_if` annotations to [`Option`] fields. +/// +/// The attribute can be added to structs and enums. +/// +/// # Example +/// +/// JSON APIs sometimes have many optional values. +/// Missing values should not be serialized, to keep the serialized format smaller. +/// Such a data type might look like: +/// +/// ```rust +/// # extern crate serde; +/// # use serde::Serialize; +/// # +/// #[derive(Serialize)] +/// struct Data { +/// #[serde(skip_serializing_if = "Option::is_none")] +/// a: Option, +/// #[serde(skip_serializing_if = "Option::is_none")] +/// b: Option, +/// #[serde(skip_serializing_if = "Option::is_none")] +/// c: Option, +/// #[serde(skip_serializing_if = "Option::is_none")] +/// d: Option, +/// } +/// ``` +/// +/// The `skip_serializing_if` annotation is repetitive and harms readability. +/// Instead the same struct can be written as: +/// +/// ```rust +/// # extern crate serde; +/// # extern crate serde_with_macros; +/// # +/// # use serde::Serialize; +/// # use serde_with_macros::skip_serializing_none; +/// #[skip_serializing_none] +/// #[derive(Serialize)] +/// struct Data { +/// a: Option, +/// b: Option, +/// c: Option, +/// d: Option, +/// } +/// ``` +/// +/// Existing `skip_serializing_if` annotations will not be altered. +/// +/// If some values should always be serialized, then the `serialize_always` can be used. +/// +/// # Limitations +/// +/// The `serialize_always` cannot be used together with a manual `skip_serializing_if` annotations, as these conflict in their meaning. +/// A compile error will be generated if this occurs. +/// +/// The `skip_serializing_none` only works if the type is called `Option`, `std::option::Option`, or `core::option::Option`. +/// Type aliasing on `Option` and giving it another name, will cause this field to be ignored. +/// This cannot be supported, as proc-macros run before type checking, thus it is not possible to determine if a type alias refers to an `Option`. +/// +/// ```rust,ignore +/// # extern crate serde; +/// # extern crate serde_with_macros; +/// # +/// # use serde::Serialize; +/// # use serde_with_macros::skip_serializing_none; +/// type MyOption = Option; +/// +/// #[skip_serializing_none] +/// #[derive(Serialize)] +/// struct Data { +/// a: MyOption, // This field will not be skipped +/// } +/// ``` +/// +/// Likewise, if you import a type and name it `Option`, the `skip_serializing_if` attributes will be added and compile errors will occur, if `Option::is_none` is not a valid function. +/// Here the function `Vec::is_none` does not exist and therefore the example fails to compile. +/// +/// ```rust,compile_fail +/// # extern crate serde; +/// # extern crate serde_with_macros; +/// # +/// # use serde::Serialize; +/// # use serde_with_macros::skip_serializing_none; +/// use std::vec::Vec as Option; +/// +/// #[skip_serializing_none] +/// #[derive(Serialize)] +/// struct Data { +/// a: Option, +/// } +/// ``` +/// #[proc_macro_attribute] pub fn skip_serializing_none(_args: TokenStream, input: TokenStream) -> TokenStream { let res = match skip_serializing_none_do(input) { From cf89b4758b584ba2f61f03578fccf850ff3de579 Mon Sep 17 00:00:00 2001 From: Jonas Bushart Date: Tue, 2 Apr 2019 21:16:16 +0200 Subject: [PATCH 9/9] Extend documentation a bit --- serde_with_macros/src/lib.rs | 6 +++--- src/lib.rs | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/serde_with_macros/src/lib.rs b/serde_with_macros/src/lib.rs index 1d51a93a..81af895f 100644 --- a/serde_with_macros/src/lib.rs +++ b/serde_with_macros/src/lib.rs @@ -85,9 +85,9 @@ use syn::{ /// The `serialize_always` cannot be used together with a manual `skip_serializing_if` annotations, as these conflict in their meaning. /// A compile error will be generated if this occurs. /// -/// The `skip_serializing_none` only works if the type is called `Option`, `std::option::Option`, or `core::option::Option`. -/// Type aliasing on `Option` and giving it another name, will cause this field to be ignored. -/// This cannot be supported, as proc-macros run before type checking, thus it is not possible to determine if a type alias refers to an `Option`. +/// The `skip_serializing_none` only works if the type is called [`Option`], [`std::option::Option`], or [`core::option::Option`]. +/// Type aliasing an [`Option`] and giving it another name, will cause this field to be ignored. +/// This cannot be supported, as proc-macros run before type checking, thus it is not possible to determine if a type alias refers to an [`Option`]. /// /// ```rust,ignore /// # extern crate serde; diff --git a/src/lib.rs b/src/lib.rs index 0101fcad..c7e50c17 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -74,6 +74,13 @@ //! //! However, this will prohibit you from applying deserialize on the value returned by serializing a struct. //! +//! # Attributes +//! +//! The crate comes with custom attributes, which futher extend how serde serialization can be customized. +//! They are enabled by default, but can be disabled, by removing the default features from this crate. +//! +//! [The documentation for the custom attributes can be found here.](serde_with_macros) +//! //! [with-annotation]: https://serde.rs/field-attrs.html#serdewith--module //! [serde#553]: https://github.com/serde-rs/serde/issues/553