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

Improve error reporting about mismatched signature in with and default attributes #2558

Merged
merged 3 commits into from
Oct 21, 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
55 changes: 47 additions & 8 deletions serde_derive/src/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,11 @@ fn deserialize_transparent(cont: &Container, params: &Parameters) -> Fragment {
} else {
let value = match field.attrs.default() {
attr::Default::Default => quote!(_serde::__private::Default::default()),
attr::Default::Path(path) => quote!(#path()),
// If #path 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(default = "...")]
// ^^^^^
attr::Default::Path(path) => quote_spanned!(path.span()=> #path()),
attr::Default::None => quote!(_serde::__private::PhantomData),
};
quote!(#member: #value)
Expand Down Expand Up @@ -751,7 +755,11 @@ fn deserialize_seq(
attr::Default::Default => Some(quote!(
let __default: Self::Value = _serde::__private::Default::default();
)),
attr::Default::Path(path) => Some(quote!(
// If #path 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(default = "...")]
// ^^^^^
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
let __default: Self::Value = #path();
)),
attr::Default::None => {
Expand Down Expand Up @@ -833,7 +841,11 @@ fn deserialize_seq_in_place(
attr::Default::Default => Some(quote!(
let __default: #this_type #ty_generics = _serde::__private::Default::default();
)),
attr::Default::Path(path) => Some(quote!(
// If #path 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(default = "...")]
// ^^^^^
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
let __default: #this_type #ty_generics = #path();
)),
attr::Default::None => {
Expand Down Expand Up @@ -867,7 +879,11 @@ fn deserialize_newtype_struct(
}
}
Some(path) => {
quote! {
// If #path 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 = "...")]
// ^^^^^
quote_spanned! {path.span()=>
#path(__e)?
}
}
Expand Down Expand Up @@ -2634,7 +2650,11 @@ fn deserialize_map(
attr::Default::Default => Some(quote!(
let __default: Self::Value = _serde::__private::Default::default();
)),
attr::Default::Path(path) => Some(quote!(
// If #path 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(default = "...")]
// ^^^^^
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
let __default: Self::Value = #path();
)),
attr::Default::None => {
Expand Down Expand Up @@ -2801,7 +2821,11 @@ fn deserialize_map_in_place(
attr::Default::Default => Some(quote!(
let __default: #this_type #ty_generics = _serde::__private::Default::default();
)),
attr::Default::Path(path) => Some(quote!(
// If #path 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(default = "...")]
// ^^^^^
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
let __default: #this_type #ty_generics = #path();
)),
attr::Default::None => {
Expand Down Expand Up @@ -2840,6 +2864,13 @@ fn wrap_deserialize_with(
split_with_de_lifetime(params);
let delife = params.borrowed.de_lifetime();

// 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)?
};
let wrapper = quote! {
#[doc(hidden)]
struct __DeserializeWith #de_impl_generics #where_clause {
Expand All @@ -2854,7 +2885,7 @@ fn wrap_deserialize_with(
__D: _serde::Deserializer<#delife>,
{
_serde::__private::Ok(__DeserializeWith {
value: #deserialize_with(__deserializer)?,
value: #value,
phantom: _serde::__private::PhantomData,
lifetime: _serde::__private::PhantomData,
})
Expand Down Expand Up @@ -2945,7 +2976,11 @@ fn expr_is_missing(field: &Field, cattrs: &attr::Container) -> Fragment {
return quote_expr!(#func());
}
attr::Default::Path(path) => {
return quote_expr!(#path());
// If #path 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(default = "...")]
// ^^^^^
return Fragment::Expr(quote_spanned!(path.span()=> #path()));
}
attr::Default::None => { /* below */ }
}
Expand Down Expand Up @@ -2988,6 +3023,10 @@ fn expr_is_missing_seq(
return quote_spanned!(span=> #assign_to _serde::__private::Default::default());
}
attr::Default::Path(path) => {
// If #path 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(default = "...")]
// ^^^^^
return quote_spanned!(path.span()=> #assign_to #path());
}
attr::Default::None => { /* below */ }
Expand Down
9 changes: 5 additions & 4 deletions serde_derive/src/internals/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::iter::FromIterator;
use syn::meta::ParseNestedMeta;
use syn::parse::ParseStream;
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::{parse_quote, token, Ident, Lifetime, Token};

// This module handles parsing of `#[serde(...)]` attributes. The entrypoints
Expand Down Expand Up @@ -898,13 +899,13 @@ impl Variant {
ser_path
.path
.segments
.push(Ident::new("serialize", Span::call_site()).into());
.push(Ident::new("serialize", ser_path.span()).into());
serialize_with.set(&meta.path, ser_path);
let mut de_path = path;
de_path
.path
.segments
.push(Ident::new("deserialize", Span::call_site()).into());
.push(Ident::new("deserialize", de_path.span()).into());
deserialize_with.set(&meta.path, de_path);
}
} else if meta.path == SERIALIZE_WITH {
Expand Down Expand Up @@ -1180,13 +1181,13 @@ impl Field {
ser_path
.path
.segments
.push(Ident::new("serialize", Span::call_site()).into());
.push(Ident::new("serialize", ser_path.span()).into());
serialize_with.set(&meta.path, ser_path);
let mut de_path = path;
de_path
.path
.segments
.push(Ident::new("deserialize", Span::call_site()).into());
.push(Ident::new("deserialize", de_path.span()).into());
deserialize_with.set(&meta.path, de_path);
}
} else if meta.path == BOUND {
Expand Down
8 changes: 7 additions & 1 deletion serde_derive/src/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1229,9 +1229,15 @@ fn wrap_serialize_with(
})
});

quote!({
// 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 = "...")]
// ^^^^^
quote_spanned!(serialize_with.span()=> {
#[doc(hidden)]
struct __SerializeWith #wrapper_impl_generics #where_clause {
// If #field_tys is empty, `values` does not used
#[allow(dead_code)]
values: (#(&'__a #field_tys, )*),
phantom: _serde::__private::PhantomData<#this_type #ty_generics>,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//! Ensures that error message points to the path in attribute
use serde_derive::Deserialize;

#[derive(Deserialize)]
#[serde(tag = "tag", content = "content")]
enum Enum {
// Newtype variants does not use the provided path, so it is forbidden here
// Newtype(#[serde(default = "main")] u8),
Tuple(
u8,
#[serde(default = "main")] i8,
),
Struct {
#[serde(default = "main")]
f1: u8,
f2: u8,
#[serde(default = "main")]
f3: i8,
},
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs:11:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `i8`
| `match` arms have incompatible types
...
11 | #[serde(default = "main")] i8,
| ^^^^^^ expected `i8`, found `()`

error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs:14:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `u8`
| `match` arms have incompatible types
...
14 | #[serde(default = "main")]
| ^^^^^^ expected `u8`, found `()`

error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs:17:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `i8`
| `match` arms have incompatible types
...
17 | #[serde(default = "main")]
| ^^^^^^ expected `i8`, found `()`
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//! Ensures that error message points to the path in attribute
use serde_derive::Deserialize;

#[derive(Deserialize)]
enum Enum {
// Newtype variants does not use the provided path, so it is forbidden here
// Newtype(#[serde(default = "main")] u8),
Tuple(
u8,
#[serde(default = "main")] i8,
),
Struct {
#[serde(default = "main")]
f1: u8,
f2: u8,
#[serde(default = "main")]
f3: i8,
},
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs:10:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `i8`
| `match` arms have incompatible types
...
10 | #[serde(default = "main")] i8,
| ^^^^^^ expected `i8`, found `()`

error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs:13:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `u8`
| `match` arms have incompatible types
...
13 | #[serde(default = "main")]
| ^^^^^^ expected `u8`, found `()`

error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs:16:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `i8`
| `match` arms have incompatible types
...
16 | #[serde(default = "main")]
| ^^^^^^ expected `i8`, found `()`
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//! Ensures that error message points to the path in attribute
use serde_derive::Deserialize;

#[derive(Deserialize)]
#[serde(tag = "tag")]
enum Enum {
// Newtype variants does not use the provided path, so it is forbidden here
// Newtype(#[serde(default = "main")] u8),
// Tuple variants does not supported in internally tagged enums
Struct {
#[serde(default = "main")]
f1: u8,
f2: u8,
#[serde(default = "main")]
f3: i8,
},
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_internally_tagged.rs:11:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `u8`
| `match` arms have incompatible types
...
11 | #[serde(default = "main")]
| ^^^^^^ expected `u8`, found `()`

error[E0308]: `match` arms have incompatible types
--> tests/ui/default-attribute/incorrect_type_enum_internally_tagged.rs:14:27
|
4 | #[derive(Deserialize)]
| -----------
| |
| this is found to be of type `i8`
| `match` arms have incompatible types
...
14 | #[serde(default = "main")]
| ^^^^^^ expected `i8`, found `()`
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//! Ensures that error message points to the path in attribute
use serde_derive::Deserialize;

#[derive(Deserialize)]
#[serde(untagged)]
enum Enum {
// Newtype variants does not use the provided path, so it is forbidden here
// Newtype(#[serde(default = "main")] u8),
Tuple(
u8,
#[serde(default = "main")] i8,
),
Struct {
#[serde(default = "main")]
f1: u8,
f2: u8,
#[serde(default = "main")]
f3: i8,
},
}

fn main() {}
Loading