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

Disallow tuple variants with incorrect order of #[serde(default)] attributes #2561

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
58 changes: 36 additions & 22 deletions serde_derive/src/internals/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,49 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
check_from_and_try_from(cx, cont);
}

fn check_default_on_tuple(cx: &Ctxt, cont: &Container) {
match &cont.data {
Data::Enum(variants) => {
for variant in variants {
if let Style::Tuple = variant.style {
check_default_on_tuple_fields(cx, &variant.fields);
}
}
}
Data::Struct(Style::Tuple, fields) => {
if let Default::None = cont.attrs.default() {
check_default_on_tuple_fields(cx, &fields);
}
}
_ => {}
}
}

// If some field of a tuple struct is marked #[serde(default)] then all fields
// after it must also be marked with that attribute, or the struct must have a
// container-level serde(default) attribute. A field's default value is only
// used for tuple fields if the sequence is exhausted at that point; that means
// all subsequent fields will fail to deserialize if they don't have their own
// default.
fn check_default_on_tuple(cx: &Ctxt, cont: &Container) {
if let Default::None = cont.attrs.default() {
if let Data::Struct(Style::Tuple, fields) = &cont.data {
let mut first_default_index = None;
for (i, field) in fields.iter().enumerate() {
// Skipped fields automatically get the #[serde(default)]
// attribute. We are interested only on non-skipped fields here.
if field.attrs.skip_deserializing() {
continue;
}
if let Default::None = field.attrs.default() {
if let Some(first) = first_default_index {
cx.error_spanned_by(
field.ty,
format!("field must have #[serde(default)] because previous field {} has #[serde(default)]", first),
);
}
continue;
}
if first_default_index.is_none() {
first_default_index = Some(i);
}
fn check_default_on_tuple_fields(cx: &Ctxt, fields: &[Field]) {
let mut first_default_index = None;
for (i, field) in fields.iter().enumerate() {
// Skipped fields automatically get the #[serde(default)]
// attribute. We are interested only on non-skipped fields here.
if field.attrs.skip_deserializing() {
continue;
}
if let Default::None = field.attrs.default() {
if let Some(first) = first_default_index {
cx.error_spanned_by(
field.ty,
format!("field must have #[serde(default)] because previous field {} has #[serde(default)]", first),
);
}
continue;
}
if first_default_index.is_none() {
first_default_index = Some(i);
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions test_suite/tests/ui/default-attribute/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,24 @@ use serde_derive::Deserialize;
#[derive(Deserialize)]
#[serde(default)]
enum E {
// No errors expected.
T0(u8, u8),

// No errors expected:
// - If both fields are provided, both get value from data.
// - If only one field is provided, the second gets default value.
T1(u8, #[serde(default)] u8),

// ERROR: The first field can get default value only if sequence is empty, but
// that mean that all other fields cannot be deserialized without errors.
T2(#[serde(default)] u8, u8, u8),

// No errors expected:
// - If both fields are provided, both get value from data.
// - If only one field is provided, the second gets default value.
// - If no fields are provided, both get default value.
T3(#[serde(default)] u8, #[serde(default)] u8),

S { f: u8 },
}

Expand Down
12 changes: 12 additions & 0 deletions test_suite/tests/ui/default-attribute/enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,15 @@ error: #[serde(default)] can only be used on structs
|
4 | #[serde(default)]
| ^^^^^^^

error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
--> tests/ui/default-attribute/enum.rs:16:30
|
16 | T2(#[serde(default)] u8, u8, u8),
| ^^

error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
--> tests/ui/default-attribute/enum.rs:16:34
|
16 | T2(#[serde(default)] u8, u8, u8),
| ^^
22 changes: 22 additions & 0 deletions test_suite/tests/ui/default-attribute/enum_path.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,30 @@
use serde_derive::Deserialize;

fn d<T>() -> T {
unimplemented!()
}

#[derive(Deserialize)]
#[serde(default = "default_e")]
enum E {
// No errors expected.
T0(u8, u8),

// No errors expected:
// - If both fields are provided, both get value from data.
// - If only one field is provided, the second gets default value.
T1(u8, #[serde(default = "d")] u8),

// ERROR: The first field can get default value only if sequence is empty, but
// that mean that all other fields cannot be deserialized without errors.
T2(#[serde(default = "d")] u8, u8, u8),

// No errors expected:
// - If both fields are provided, both get value from data.
// - If only one field is provided, the second gets default value.
// - If no fields are provided, both get default value.
T3(#[serde(default = "d")] u8, #[serde(default = "d")] u8),

S { f: u8 },
}

Expand Down
16 changes: 14 additions & 2 deletions test_suite/tests/ui/default-attribute/enum_path.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
error: #[serde(default = "...")] can only be used on structs
--> tests/ui/default-attribute/enum_path.rs:4:9
--> tests/ui/default-attribute/enum_path.rs:8:9
|
4 | #[serde(default = "default_e")]
8 | #[serde(default = "default_e")]
| ^^^^^^^^^^^^^^^^^^^^^

error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
--> tests/ui/default-attribute/enum_path.rs:20:36
|
20 | T2(#[serde(default = "d")] u8, u8, u8),
| ^^

error: field must have #[serde(default)] because previous field 0 has #[serde(default)]
--> tests/ui/default-attribute/enum_path.rs:20:40
|
20 | T2(#[serde(default = "d")] u8, u8, u8),
| ^^