Skip to content

Commit 5740423

Browse files
committed
Disallow tuple variants with incorrect order of #[serde(default)] attributes
(review this commit with "ignore whitespace changes" option on)
1 parent 05a5b7e commit 5740423

File tree

5 files changed

+102
-24
lines changed

5 files changed

+102
-24
lines changed

serde_derive/src/internals/check.rs

+36-22
Original file line numberDiff line numberDiff line change
@@ -18,35 +18,49 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
1818
check_from_and_try_from(cx, cont);
1919
}
2020

21+
fn check_default_on_tuple(cx: &Ctxt, cont: &Container) {
22+
match &cont.data {
23+
Data::Enum(variants) => {
24+
for variant in variants {
25+
if let Style::Tuple = variant.style {
26+
check_default_on_tuple_fields(cx, &variant.fields);
27+
}
28+
}
29+
}
30+
Data::Struct(Style::Tuple, fields) => {
31+
if let Default::None = cont.attrs.default() {
32+
check_default_on_tuple_fields(cx, &fields);
33+
}
34+
}
35+
_ => {}
36+
}
37+
}
38+
2139
// If some field of a tuple struct is marked #[serde(default)] then all fields
2240
// after it must also be marked with that attribute, or the struct must have a
2341
// container-level serde(default) attribute. A field's default value is only
2442
// used for tuple fields if the sequence is exhausted at that point; that means
2543
// all subsequent fields will fail to deserialize if they don't have their own
2644
// default.
27-
fn check_default_on_tuple(cx: &Ctxt, cont: &Container) {
28-
if let Default::None = cont.attrs.default() {
29-
if let Data::Struct(Style::Tuple, fields) = &cont.data {
30-
let mut first_default_index = None;
31-
for (i, field) in fields.iter().enumerate() {
32-
// Skipped fields automatically get the #[serde(default)]
33-
// attribute. We are interested only on non-skipped fields here.
34-
if field.attrs.skip_deserializing() {
35-
continue;
36-
}
37-
if let Default::None = field.attrs.default() {
38-
if let Some(first) = first_default_index {
39-
cx.error_spanned_by(
40-
field.ty,
41-
format!("field must have #[serde(default)] because previous field {} has #[serde(default)]", first),
42-
);
43-
}
44-
continue;
45-
}
46-
if first_default_index.is_none() {
47-
first_default_index = Some(i);
48-
}
45+
fn check_default_on_tuple_fields(cx: &Ctxt, fields: &[Field]) {
46+
let mut first_default_index = None;
47+
for (i, field) in fields.iter().enumerate() {
48+
// Skipped fields automatically get the #[serde(default)]
49+
// attribute. We are interested only on non-skipped fields here.
50+
if field.attrs.skip_deserializing() {
51+
continue;
52+
}
53+
if let Default::None = field.attrs.default() {
54+
if let Some(first) = first_default_index {
55+
cx.error_spanned_by(
56+
field.ty,
57+
format!("field must have #[serde(default)] because previous field {} has #[serde(default)]", first),
58+
);
4959
}
60+
continue;
61+
}
62+
if first_default_index.is_none() {
63+
first_default_index = Some(i);
5064
}
5165
}
5266
}

test_suite/tests/ui/default-attribute/enum.rs

+18
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,24 @@ use serde_derive::Deserialize;
33
#[derive(Deserialize)]
44
#[serde(default)]
55
enum E {
6+
// No errors expected.
7+
T0(u8, u8),
8+
9+
// No errors expected:
10+
// - If both fields are provided, both get value from data.
11+
// - If only one field is provided, the second gets default value.
12+
T1(u8, #[serde(default)] u8),
13+
14+
// ERROR: The first field can get default value only if sequence is empty, but
15+
// that mean that all other fields cannot be deserialized without errors.
16+
T2(#[serde(default)] u8, u8, u8),
17+
18+
// No errors expected:
19+
// - If both fields are provided, both get value from data.
20+
// - If only one field is provided, the second gets default value.
21+
// - If no fields are provided, both get default value.
22+
T3(#[serde(default)] u8, #[serde(default)] u8),
23+
624
S { f: u8 },
725
}
826

test_suite/tests/ui/default-attribute/enum.stderr

+12
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,15 @@ error: #[serde(default)] can only be used on structs
33
|
44
4 | #[serde(default)]
55
| ^^^^^^^
6+
7+
error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
8+
--> tests/ui/default-attribute/enum.rs:16:30
9+
|
10+
16 | T2(#[serde(default)] u8, u8, u8),
11+
| ^^
12+
13+
error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
14+
--> tests/ui/default-attribute/enum.rs:16:34
15+
|
16+
16 | T2(#[serde(default)] u8, u8, u8),
17+
| ^^

test_suite/tests/ui/default-attribute/enum_path.rs

+22
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,30 @@
11
use serde_derive::Deserialize;
22

3+
fn d<T>() -> T {
4+
unimplemented!()
5+
}
6+
37
#[derive(Deserialize)]
48
#[serde(default = "default_e")]
59
enum E {
10+
// No errors expected.
11+
T0(u8, u8),
12+
13+
// No errors expected:
14+
// - If both fields are provided, both get value from data.
15+
// - If only one field is provided, the second gets default value.
16+
T1(u8, #[serde(default = "d")] u8),
17+
18+
// ERROR: The first field can get default value only if sequence is empty, but
19+
// that mean that all other fields cannot be deserialized without errors.
20+
T2(#[serde(default = "d")] u8, u8, u8),
21+
22+
// No errors expected:
23+
// - If both fields are provided, both get value from data.
24+
// - If only one field is provided, the second gets default value.
25+
// - If no fields are provided, both get default value.
26+
T3(#[serde(default = "d")] u8, #[serde(default = "d")] u8),
27+
628
S { f: u8 },
729
}
830

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
error: #[serde(default = "...")] can only be used on structs
2-
--> tests/ui/default-attribute/enum_path.rs:4:9
2+
--> tests/ui/default-attribute/enum_path.rs:8:9
33
|
4-
4 | #[serde(default = "default_e")]
4+
8 | #[serde(default = "default_e")]
55
| ^^^^^^^^^^^^^^^^^^^^^
6+
7+
error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
8+
--> tests/ui/default-attribute/enum_path.rs:20:36
9+
|
10+
20 | T2(#[serde(default = "d")] u8, u8, u8),
11+
| ^^
12+
13+
error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
14+
--> tests/ui/default-attribute/enum_path.rs:20:40
15+
|
16+
20 | T2(#[serde(default = "d")] u8, u8, u8),
17+
| ^^

0 commit comments

Comments
 (0)