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

Fix hygiene of macro-generated local variable accesses in serde(with) wrappers #2845

Merged
merged 2 commits into from
Oct 22, 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
5 changes: 3 additions & 2 deletions serde_derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2882,13 +2882,14 @@ fn wrap_deserialize_with(
let (de_impl_generics, de_ty_generics, ty_generics, where_clause) =
split_with_de_lifetime(params);
let delife = params.borrowed.de_lifetime();
let deserializer_var = quote!(__deserializer);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, it would be useful to explain why this line (and in other similar places) placed in it's own TokenStream so this part was not accidentally optimized (yes, new tests will catch that, but this can create unnecessary friction for developers whos not aware of that problem). Even reference to GitHub issue with very short comment will be enougth


// If #deserialize_with returns wrong type, error will be reported here (^^^^^).
// We attach span of the path to the function so it will be reported
// on the #[serde(with = "...")]
// ^^^^^
let value = quote_spanned! {deserialize_with.span()=>
#deserialize_with(__deserializer)?
#deserialize_with(#deserializer_var)?
};
let wrapper = quote! {
#[doc(hidden)]
Expand All @@ -2899,7 +2900,7 @@ fn wrap_deserialize_with(
}

impl #de_impl_generics _serde::Deserialize<#delife> for __DeserializeWith #de_ty_generics #where_clause {
fn deserialize<__D>(__deserializer: __D) -> _serde::__private::Result<Self, __D::Error>
fn deserialize<__D>(#deserializer_var: __D) -> _serde::__private::Result<Self, __D::Error>
where
__D: _serde::Deserializer<#delife>,
{
Expand Down
7 changes: 5 additions & 2 deletions serde_derive/src/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1220,12 +1220,15 @@ fn wrap_serialize_with(
})
});

let self_var = quote!(self);
let serializer_var = quote!(__s);

// If #serialize_with returns wrong type, error will be reported on here.
// We attach span of the path to this piece so error will be reported
// on the #[serde(with = "...")]
// ^^^^^
let wrapper_serialize = quote_spanned! {serialize_with.span()=>
#serialize_with(#(self.values.#field_access, )* __s)
#serialize_with(#(#self_var.values.#field_access, )* #serializer_var)
};

quote!({
Expand All @@ -1236,7 +1239,7 @@ fn wrap_serialize_with(
}

impl #wrapper_impl_generics _serde::Serialize for __SerializeWith #wrapper_ty_generics #where_clause {
fn serialize<__S>(&self, __s: __S) -> _serde::__private::Result<__S::Ok, __S::Error>
fn serialize<__S>(&#self_var, #serializer_var: __S) -> _serde::__private::Result<__S::Ok, __S::Error>
where
__S: _serde::Serializer,
{
Expand Down
31 changes: 31 additions & 0 deletions test_suite/tests/regression/issue2844.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use serde_derive::{Deserialize, Serialize};

macro_rules! declare_in_macro {
($with:literal) => {
#[derive(Serialize, Deserialize)]
pub struct S {
#[serde(with = $with)]
f: i32,
}
};
}

declare_in_macro!("with");

mod with {
use serde::{Deserializer, Serializer};

pub fn serialize<S>(_: &i32, _: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
unimplemented!()
}

pub fn deserialize<'de, D>(_: D) -> Result<i32, D::Error>
where
D: Deserializer<'de>,
{
unimplemented!()
}
}
8 changes: 6 additions & 2 deletions test_suite/tests/ui/with/incorrect_type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ note: required by a bound in `w::serialize`
error[E0061]: this function takes 1 argument but 2 arguments were supplied
--> tests/ui/with/incorrect_type.rs:15:25
|
14 | #[derive(Serialize, Deserialize)]
| --------- unexpected argument #2 of type `__S`
15 | struct W(#[serde(with = "w")] u8, u8);
| ^^^ unexpected argument #2 of type `__S`
| ^^^
|
note: function defined here
--> tests/ui/with/incorrect_type.rs:9:12
Expand Down Expand Up @@ -68,8 +70,10 @@ note: required by a bound in `w::serialize`
error[E0061]: this function takes 1 argument but 2 arguments were supplied
--> tests/ui/with/incorrect_type.rs:18:35
|
17 | #[derive(Serialize, Deserialize)]
| --------- unexpected argument #2 of type `__S`
18 | struct S(#[serde(serialize_with = "w::serialize")] u8, u8);
| ^^^^^^^^^^^^^^ unexpected argument #2 of type `__S`
| ^^^^^^^^^^^^^^
|
note: function defined here
--> tests/ui/with/incorrect_type.rs:9:12
Expand Down