Skip to content

Commit

Permalink
Merge #262
Browse files Browse the repository at this point in the history 
262: Change processing of `#[serde_as(...)]` attributes on fields. r=jonasbb a=jonasbb

The attributes will no longer be stripped during proc-macro processing.
Instead, a private derive macro is applied to the struct/enum which captures them and makes them inert, thus allowing compilation.

This should have no effect on the generated code and on the runtime behavior.
It eases integration of third-party crates with `serde_with`, since they can now process the `#[serde_as(...)]` field attributes reliably.
Before this was impossible for derive macros and lead to akward ordering constraints on the attribute macros.

Previous discussion in: #260

@Lehona Could you please check if this solves your issue?

Co-authored-by: Jonas Bushart <[email protected]>
  • Loading branch information
@bors @jonasbb
bors[bot] and jonasbb authored Feb 15, 2021
2 parents f8471b2 + 7bc626d commit 60c8b16
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 55 deletions.
10 changes: 10 additions & 0 deletions serde_with_macros/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
* Improve error messages when `#[serde_as(..)]` is misused as a field attribute.
Thanks to @Lehona for reporting the bug in #233.
* Internal cleanup for assembling and parsing attributes during `serde_as` processing.
* Change processing on `#[serde_as(...)]` attributes on fields.

The attributes will no longer be stripped during proc-macro processing.
Instead, a private derive macro is applied to the struct/enum which captures them and makes them inert, thus allowing compilation.

This should have no effect on the generated code and on the runtime behavior.
It eases integration of third-party crates with `serde_with`, since they can now process the `#[serde_as(...)]` field attributes reliably.
Before this was impossible for derive macros and lead to akward ordering constraints on the attribute macros.

Thanks to @Lehona for reporting this problem and to @dtolnay for suggesting the dummy derive macro.

## [1.3.0]

Expand Down
53 changes: 28 additions & 25 deletions serde_with_macros/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ where
/// Like [apply_function_to_struct_and_enum_fields] but for darling errors
fn apply_function_to_struct_and_enum_fields_darling<F>(
input: TokenStream,
serde_with_crate_path: &Path,
function: F,
) -> Result<TokenStream2, DarlingError>
where
Expand Down Expand Up @@ -157,10 +158,17 @@ where
}
}

// Add a dummy derive macro which consumes (makes inert) all field attributes
let attr_tokens = quote!(
#[derive(#serde_with_crate_path::__private_consume_serde_as_attributes)]
);
let consume_serde_as_attribute = Attribute::parse_outer.parse2(attr_tokens)?;

// 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::<ItemStruct>(input.clone()) {
apply_on_fields(&mut input.fields, function)?;
input.attrs.extend(consume_serde_as_attribute);
Ok(quote!(#input))
} else if let Ok(mut input) = syn::parse::<ItemEnum>(input) {
let errors: Vec<DarlingError> = input
Expand All @@ -174,6 +182,7 @@ where
})
.collect();
if errors.is_empty() {
input.attrs.extend(consume_serde_as_attribute);
Ok(quote!(#input))
} else {
Err(DarlingError::multiple(errors))
Expand Down Expand Up @@ -453,9 +462,11 @@ pub fn serde_as(args: TokenStream, input: TokenStream) -> TokenStream {
};

// Convert any error message into a nice compiler error
let res = match apply_function_to_struct_and_enum_fields_darling(input, |field| {
serde_as_add_attr_to_field(field, &serde_with_crate_path)
}) {
let res = match apply_function_to_struct_and_enum_fields_darling(
input,
&serde_with_crate_path,
|field| serde_as_add_attr_to_field(field, &serde_with_crate_path),
) {
Ok(res) => res,
Err(err) => err.write_errors(),
};
Expand Down Expand Up @@ -505,28 +516,6 @@ fn serde_as_add_attr_to_field(
let serde_with_options = SerdeWithOptions::from_field(field)?;

let mut errors = Vec::new();

// Find index of serde_as attribute
let serde_as_idxs: Vec<_> = field
.attrs
.iter()
.enumerate()
.filter_map(|(idx, attr)| {
if attr.path.is_ident("serde_as") {
Some(idx)
} else {
None
}
})
.collect();
if serde_as_idxs.is_empty() {
return Ok(());
}
// remove serde_as Attribute as otherwise they cause compile errors
for idx in serde_as_idxs.into_iter().rev() {
field.attrs.remove(idx);
}

if !serde_as_options.has_any_set() {
errors.push(DarlingError::custom("An empty `serde_as` attribute on a field has no effect. You are missing an `as`, `serialize_as`, or `deserialize_as` parameter."));
}
Expand Down Expand Up @@ -885,3 +874,17 @@ fn serialize_display(input: DeriveInput, serde_with_crate_path: Path) -> TokenSt
}
}
}

#[doc(hidden)]
/// Private function. Not part of the public API
///
/// The only task of this derive macro is to consume any `serde_as` attributes and turn them into inert attributes.
/// This allows the serde_as macro to keep the field attributes without causing compiler errors.
/// The intend is that keeping the field attributes allows downstream crates to consume and akt on them without causing an ordering dependency to the serde_as macro.
/// Otherwise, downstream proc-macros would need to be places *in front of* the main `#[serde_as]` attribute, since otherwise the field attributes would already be stripped off.
///
/// More details about the use-cases in the Github discussion: <https://github.com/jonasbb/serde_with/discussions/260>.
#[proc_macro_derive(__private_consume_serde_as_attributes, attributes(serde_as))]
pub fn __private_consume_serde_as_attributes(_: TokenStream) -> TokenStream {
TokenStream::new()
}
60 changes: 30 additions & 30 deletions serde_with_macros/tests/compile-fail/serde_as.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,73 +1,73 @@
error: Cannot combine `as` with `deserialize_as`. Use `serialize_as` to specify different serialization code.
--> $DIR/serde_as.rs:9:5
--> $DIR/serde_as.rs:8:5
|
9 | a: u32,
8 | #[serde_as(as = "_", deserialize_as = "_")]
| ^

error: Cannot combine `as` with `serialize_as`. Use `deserialize_as` to specify different deserialization code.
--> $DIR/serde_as.rs:11:5
--> $DIR/serde_as.rs:10:5
|
11 | b: u32,
10 | #[serde_as(as = "_", serialize_as = "_")]
| ^

error: Cannot combine `as` with `deserialize_as`. Use `serialize_as` to specify different serialization code.
--> $DIR/serde_as.rs:13:5
--> $DIR/serde_as.rs:12:5
|
13 | c: u32,
12 | #[serde_as(as = "_", deserialize_as = "_", serialize_as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:21:5
--> $DIR/serde_as.rs:20:5
|
21 | #[serde(with = "u32")]
20 | #[serde_as(as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:24:5
--> $DIR/serde_as.rs:23:5
|
24 | #[serde(deserialize_with = "u32")]
23 | #[serde_as(as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:27:5
--> $DIR/serde_as.rs:26:5
|
27 | #[serde(serialize_with = "u32")]
26 | #[serde_as(as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:31:5
--> $DIR/serde_as.rs:30:5
|
31 | #[serde(with = "u32")]
30 | #[serde_as(deserialize_as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:34:5
--> $DIR/serde_as.rs:33:5
|
34 | #[serde(deserialize_with = "u32")]
33 | #[serde_as(deserialize_as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:37:5
--> $DIR/serde_as.rs:36:5
|
37 | #[serde(serialize_with = "u32")]
36 | #[serde_as(deserialize_as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:41:5
--> $DIR/serde_as.rs:40:5
|
41 | #[serde(with = "u32")]
40 | #[serde_as(serialize_as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:44:5
--> $DIR/serde_as.rs:43:5
|
44 | #[serde(deserialize_with = "u32")]
43 | #[serde_as(serialize_as = "_")]
| ^

error: Cannot combine `serde_as` with serde's `with`, `deserialize_with`, or `serialize_with`.
--> $DIR/serde_as.rs:47:5
--> $DIR/serde_as.rs:46:5
|
47 | #[serde(serialize_with = "u32")]
46 | #[serde_as(serialize_as = "_")]
| ^

error: Unknown field: `does_not_exist`
Expand Down Expand Up @@ -95,13 +95,13 @@ error: Unable to parse attribute: expected literal
| ^^^^^^^^^^^^^^

error: An empty `serde_as` attribute on a field has no effect. You are missing an `as`, `serialize_as`, or `deserialize_as` parameter.
--> $DIR/serde_as.rs:70:5
--> $DIR/serde_as.rs:69:5
|
70 | no_entries: u32,
| ^^^^^^^^^^
69 | #[serde_as]
| ^

error: An empty `serde_as` attribute on a field has no effect. You are missing an `as`, `serialize_as`, or `deserialize_as` parameter.
--> $DIR/serde_as.rs:72:5
--> $DIR/serde_as.rs:71:5
|
72 | no_entries_brackets: u32,
| ^^^^^^^^^^^^^^^^^^^
71 | #[serde_as()]
| ^

0 comments on commit 60c8b16

Please sign in to comment.