Skip to content

Commit 04bb76b

Browse files
oli-obkoli-obk
authored
Merge pull request #2558 from Mingun/correct-span
Improve error reporting about mismatched signature in `with` and `default` attributes
2 parents 8b0f482 + 74b538b commit 04bb76b

27 files changed

+628
-13
lines changed

‎serde_derive/src/de.rs‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,11 @@ fn deserialize_transparent(cont: &Container, params: &Parameters) -> Fragment {
371371
} else {
372372
let value = match field.attrs.default() {
373373
attr::Default::Default => quote!(_serde::__private::Default::default()),
374-
attr::Default::Path(path) => quote!(#path()),
374+
// If #path returns wrong type, error will be reported here (^^^^^).
375+
// We attach span of the path to the function so it will be reported
376+
// on the #[serde(default = "...")]
377+
// ^^^^^
378+
attr::Default::Path(path) => quote_spanned!(path.span()=> #path()),
375379
attr::Default::None => quote!(_serde::__private::PhantomData),
376380
};
377381
quote!(#member: #value)
@@ -757,7 +761,11 @@ fn deserialize_seq(
757761
attr::Default::Default => Some(quote!(
758762
let __default: Self::Value = _serde::__private::Default::default();
759763
)),
760-
attr::Default::Path(path) => Some(quote!(
764+
// If #path returns wrong type, error will be reported here (^^^^^).
765+
// We attach span of the path to the function so it will be reported
766+
// on the #[serde(default = "...")]
767+
// ^^^^^
768+
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
761769
let __default: Self::Value = #path();
762770
)),
763771
attr::Default::None => {
@@ -839,7 +847,11 @@ fn deserialize_seq_in_place(
839847
attr::Default::Default => Some(quote!(
840848
let __default: #this_type #ty_generics = _serde::__private::Default::default();
841849
)),
842-
attr::Default::Path(path) => Some(quote!(
850+
// If #path returns wrong type, error will be reported here (^^^^^).
851+
// We attach span of the path to the function so it will be reported
852+
// on the #[serde(default = "...")]
853+
// ^^^^^
854+
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
843855
let __default: #this_type #ty_generics = #path();
844856
)),
845857
attr::Default::None => {
@@ -873,7 +885,11 @@ fn deserialize_newtype_struct(
873885
}
874886
}
875887
Some(path) => {
876-
quote! {
888+
// If #path returns wrong type, error will be reported here (^^^^^).
889+
// We attach span of the path to the function so it will be reported
890+
// on the #[serde(with = "...")]
891+
// ^^^^^
892+
quote_spanned! {path.span()=>
877893
#path(__e)?
878894
}
879895
}
@@ -2647,7 +2663,11 @@ fn deserialize_map(
26472663
attr::Default::Default => Some(quote!(
26482664
let __default: Self::Value = _serde::__private::Default::default();
26492665
)),
2650-
attr::Default::Path(path) => Some(quote!(
2666+
// If #path returns wrong type, error will be reported here (^^^^^).
2667+
// We attach span of the path to the function so it will be reported
2668+
// on the #[serde(default = "...")]
2669+
// ^^^^^
2670+
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
26512671
let __default: Self::Value = #path();
26522672
)),
26532673
attr::Default::None => {
@@ -2817,7 +2837,11 @@ fn deserialize_map_in_place(
28172837
attr::Default::Default => Some(quote!(
28182838
let __default: #this_type #ty_generics = _serde::__private::Default::default();
28192839
)),
2820-
attr::Default::Path(path) => Some(quote!(
2840+
// If #path returns wrong type, error will be reported here (^^^^^).
2841+
// We attach span of the path to the function so it will be reported
2842+
// on the #[serde(default = "...")]
2843+
// ^^^^^
2844+
attr::Default::Path(path) => Some(quote_spanned!(path.span()=>
28212845
let __default: #this_type #ty_generics = #path();
28222846
)),
28232847
attr::Default::None => {
@@ -2856,6 +2880,13 @@ fn wrap_deserialize_with(
28562880
split_with_de_lifetime(params);
28572881
let delife = params.borrowed.de_lifetime();
28582882

2883+
// If #deserialize_with returns wrong type, error will be reported here (^^^^^).
2884+
// We attach span of the path to the function so it will be reported
2885+
// on the #[serde(with = "...")]
2886+
// ^^^^^
2887+
let value = quote_spanned! {deserialize_with.span()=>
2888+
#deserialize_with(__deserializer)?
2889+
};
28592890
let wrapper = quote! {
28602891
#[doc(hidden)]
28612892
struct __DeserializeWith #de_impl_generics #where_clause {
@@ -2870,7 +2901,7 @@ fn wrap_deserialize_with(
28702901
__D: _serde::Deserializer<#delife>,
28712902
{
28722903
_serde::__private::Ok(__DeserializeWith {
2873-
value: #deserialize_with(__deserializer)?,
2904+
value: #value,
28742905
phantom: _serde::__private::PhantomData,
28752906
lifetime: _serde::__private::PhantomData,
28762907
})
@@ -2961,7 +2992,11 @@ fn expr_is_missing(field: &Field, cattrs: &attr::Container) -> Fragment {
29612992
return quote_expr!(#func());
29622993
}
29632994
attr::Default::Path(path) => {
2964-
return quote_expr!(#path());
2995+
// If #path returns wrong type, error will be reported here (^^^^^).
2996+
// We attach span of the path to the function so it will be reported
2997+
// on the #[serde(default = "...")]
2998+
// ^^^^^
2999+
return Fragment::Expr(quote_spanned!(path.span()=> #path()));
29653000
}
29663001
attr::Default::None => { /* below */ }
29673002
}
@@ -3004,6 +3039,10 @@ fn expr_is_missing_seq(
30043039
return quote_spanned!(span=> #assign_to _serde::__private::Default::default());
30053040
}
30063041
attr::Default::Path(path) => {
3042+
// If #path returns wrong type, error will be reported here (^^^^^).
3043+
// We attach span of the path to the function so it will be reported
3044+
// on the #[serde(default = "...")]
3045+
// ^^^^^
30073046
return quote_spanned!(path.span()=> #assign_to #path());
30083047
}
30093048
attr::Default::None => { /* below */ }

‎serde_derive/src/internals/attr.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use std::iter::FromIterator;
88
use syn::meta::ParseNestedMeta;
99
use syn::parse::ParseStream;
1010
use syn::punctuated::Punctuated;
11+
use syn::spanned::Spanned;
1112
use syn::{parse_quote, token, Ident, Lifetime, Token};
1213

1314
// This module handles parsing of `#[serde(...)]` attributes. The entrypoints
@@ -888,13 +889,13 @@ impl Variant {
888889
ser_path
889890
.path
890891
.segments
891-
.push(Ident::new("serialize", Span::call_site()).into());
892+
.push(Ident::new("serialize", ser_path.span()).into());
892893
serialize_with.set(&meta.path, ser_path);
893894
let mut de_path = path;
894895
de_path
895896
.path
896897
.segments
897-
.push(Ident::new("deserialize", Span::call_site()).into());
898+
.push(Ident::new("deserialize", de_path.span()).into());
898899
deserialize_with.set(&meta.path, de_path);
899900
}
900901
} else if meta.path == SERIALIZE_WITH {
@@ -1170,13 +1171,13 @@ impl Field {
11701171
ser_path
11711172
.path
11721173
.segments
1173-
.push(Ident::new("serialize", Span::call_site()).into());
1174+
.push(Ident::new("serialize", ser_path.span()).into());
11741175
serialize_with.set(&meta.path, ser_path);
11751176
let mut de_path = path;
11761177
de_path
11771178
.path
11781179
.segments
1179-
.push(Ident::new("deserialize", Span::call_site()).into());
1180+
.push(Ident::new("deserialize", de_path.span()).into());
11801181
deserialize_with.set(&meta.path, de_path);
11811182
}
11821183
} else if meta.path == BOUND {

‎serde_derive/src/ser.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1220,9 +1220,15 @@ fn wrap_serialize_with(
12201220
})
12211221
});
12221222

1223-
quote!({
1223+
// If #serialize_with returns wrong type, error will be reported on here.
1224+
// We attach span of the path to this piece so error will be reported
1225+
// on the #[serde(with = "...")]
1226+
// ^^^^^
1227+
quote_spanned!(serialize_with.span()=> {
12241228
#[doc(hidden)]
12251229
struct __SerializeWith #wrapper_impl_generics #where_clause {
1230+
// If #field_tys is empty, `values` does not used
1231+
#[allow(dead_code)]
12261232
values: (#(&'__a #field_tys, )*),
12271233
phantom: _serde::__private::PhantomData<#this_type #ty_generics>,
12281234
}

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//! Ensures that error message points to the path in attribute
2+
use serde_derive::Deserialize;
3+
4+
#[derive(Deserialize)]
5+
#[serde(tag = "tag", content = "content")]
6+
enum Enum {
7+
// Newtype variants does not use the provided path, so it is forbidden here
8+
// Newtype(#[serde(default = "main")] u8),
9+
Tuple(
10+
u8,
11+
#[serde(default = "main")] i8,
12+
),
13+
Struct {
14+
#[serde(default = "main")]
15+
f1: u8,
16+
f2: u8,
17+
#[serde(default = "main")]
18+
f3: i8,
19+
},
20+
}
21+
22+
fn main() {}

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.stderr‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
error[E0308]: `match` arms have incompatible types
2+
--> tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs:11:27
3+
|
4+
4 | #[derive(Deserialize)]
5+
| -----------
6+
| |
7+
| this is found to be of type `i8`
8+
| `match` arms have incompatible types
9+
...
10+
11 | #[serde(default = "main")] i8,
11+
| ^^^^^^ expected `i8`, found `()`
12+
13+
error[E0308]: `match` arms have incompatible types
14+
--> tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs:14:27
15+
|
16+
4 | #[derive(Deserialize)]
17+
| -----------
18+
| |
19+
| this is found to be of type `u8`
20+
| `match` arms have incompatible types
21+
...
22+
14 | #[serde(default = "main")]
23+
| ^^^^^^ expected `u8`, found `()`
24+
25+
error[E0308]: `match` arms have incompatible types
26+
--> tests/ui/default-attribute/incorrect_type_enum_adjacently_tagged.rs:17:27
27+
|
28+
4 | #[derive(Deserialize)]
29+
| -----------
30+
| |
31+
| this is found to be of type `i8`
32+
| `match` arms have incompatible types
33+
...
34+
17 | #[serde(default = "main")]
35+
| ^^^^^^ expected `i8`, found `()`

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
//! Ensures that error message points to the path in attribute
2+
use serde_derive::Deserialize;
3+
4+
#[derive(Deserialize)]
5+
enum Enum {
6+
// Newtype variants does not use the provided path, so it is forbidden here
7+
// Newtype(#[serde(default = "main")] u8),
8+
Tuple(
9+
u8,
10+
#[serde(default = "main")] i8,
11+
),
12+
Struct {
13+
#[serde(default = "main")]
14+
f1: u8,
15+
f2: u8,
16+
#[serde(default = "main")]
17+
f3: i8,
18+
},
19+
}
20+
21+
fn main() {}

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_externally_tagged.stderr‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
error[E0308]: `match` arms have incompatible types
2+
--> tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs:10:27
3+
|
4+
4 | #[derive(Deserialize)]
5+
| -----------
6+
| |
7+
| this is found to be of type `i8`
8+
| `match` arms have incompatible types
9+
...
10+
10 | #[serde(default = "main")] i8,
11+
| ^^^^^^ expected `i8`, found `()`
12+
13+
error[E0308]: `match` arms have incompatible types
14+
--> tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs:13:27
15+
|
16+
4 | #[derive(Deserialize)]
17+
| -----------
18+
| |
19+
| this is found to be of type `u8`
20+
| `match` arms have incompatible types
21+
...
22+
13 | #[serde(default = "main")]
23+
| ^^^^^^ expected `u8`, found `()`
24+
25+
error[E0308]: `match` arms have incompatible types
26+
--> tests/ui/default-attribute/incorrect_type_enum_externally_tagged.rs:16:27
27+
|
28+
4 | #[derive(Deserialize)]
29+
| -----------
30+
| |
31+
| this is found to be of type `i8`
32+
| `match` arms have incompatible types
33+
...
34+
16 | #[serde(default = "main")]
35+
| ^^^^^^ expected `i8`, found `()`

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_internally_tagged.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//! Ensures that error message points to the path in attribute
2+
use serde_derive::Deserialize;
3+
4+
#[derive(Deserialize)]
5+
#[serde(tag = "tag")]
6+
enum Enum {
7+
// Newtype variants does not use the provided path, so it is forbidden here
8+
// Newtype(#[serde(default = "main")] u8),
9+
// Tuple variants does not supported in internally tagged enums
10+
Struct {
11+
#[serde(default = "main")]
12+
f1: u8,
13+
f2: u8,
14+
#[serde(default = "main")]
15+
f3: i8,
16+
},
17+
}
18+
19+
fn main() {}

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_internally_tagged.stderr‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
error[E0308]: `match` arms have incompatible types
2+
--> tests/ui/default-attribute/incorrect_type_enum_internally_tagged.rs:11:27
3+
|
4+
4 | #[derive(Deserialize)]
5+
| -----------
6+
| |
7+
| this is found to be of type `u8`
8+
| `match` arms have incompatible types
9+
...
10+
11 | #[serde(default = "main")]
11+
| ^^^^^^ expected `u8`, found `()`
12+
13+
error[E0308]: `match` arms have incompatible types
14+
--> tests/ui/default-attribute/incorrect_type_enum_internally_tagged.rs:14:27
15+
|
16+
4 | #[derive(Deserialize)]
17+
| -----------
18+
| |
19+
| this is found to be of type `i8`
20+
| `match` arms have incompatible types
21+
...
22+
14 | #[serde(default = "main")]
23+
| ^^^^^^ expected `i8`, found `()`

‎test_suite/tests/ui/default-attribute/incorrect_type_enum_untagged.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//! Ensures that error message points to the path in attribute
2+
use serde_derive::Deserialize;
3+
4+
#[derive(Deserialize)]
5+
#[serde(untagged)]
6+
enum Enum {
7+
// Newtype variants does not use the provided path, so it is forbidden here
8+
// Newtype(#[serde(default = "main")] u8),
9+
Tuple(
10+
u8,
11+
#[serde(default = "main")] i8,
12+
),
13+
Struct {
14+
#[serde(default = "main")]
15+
f1: u8,
16+
f2: u8,
17+
#[serde(default = "main")]
18+
f3: i8,
19+
},
20+
}
21+
22+
fn main() {}

0 commit comments

Comments
 (0)