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

Allow #[serde(default)] on tuple structs #2553

Merged
merged 1 commit into from Aug 6, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 34 additions & 23 deletions serde_derive/src/de.rs
Expand Up @@ -723,19 +723,11 @@ fn deserialize_seq(
})
}
};
let value_if_none = match field.attrs.default() {
attr::Default::Default => quote!(_serde::__private::Default::default()),
attr::Default::Path(path) => quote!(#path()),
attr::Default::None => quote!(
return _serde::__private::Err(_serde::de::Error::invalid_length(#index_in_seq, &#expecting));
),
};
let value_if_none = expr_is_missing_seq(None, index_in_seq, field, cattrs, expecting);
let assign = quote! {
let #var = match #visit {
_serde::__private::Some(__value) => __value,
_serde::__private::None => {
#value_if_none
}
_serde::__private::None => #value_if_none,
};
};
index_in_seq += 1;
Expand Down Expand Up @@ -811,24 +803,14 @@ fn deserialize_seq_in_place(
self.place.#member = #default;
}
} else {
let value_if_none = match field.attrs.default() {
attr::Default::Default => quote!(
self.place.#member = _serde::__private::Default::default();
),
attr::Default::Path(path) => quote!(
self.place.#member = #path();
),
attr::Default::None => quote!(
return _serde::__private::Err(_serde::de::Error::invalid_length(#index_in_seq, &#expecting));
),
};
let value_if_none = expr_is_missing_seq(Some(quote!(self.place.#member = )), index_in_seq, field, cattrs, expecting);
let write = match field.attrs.deserialize_with() {
None => {
quote! {
if let _serde::__private::None = _serde::de::SeqAccess::next_element_seed(&mut __seq,
_serde::__private::de::InPlaceSeed(&mut self.place.#member))?
{
#value_if_none
#value_if_none;
}
}
}
Expand All @@ -841,7 +823,7 @@ fn deserialize_seq_in_place(
self.place.#member = __wrap.value;
}
_serde::__private::None => {
#value_if_none
#value_if_none;
}
}
})
Expand Down Expand Up @@ -2993,6 +2975,35 @@ fn expr_is_missing(field: &Field, cattrs: &attr::Container) -> Fragment {
}
}

fn expr_is_missing_seq(
assign_to: Option<TokenStream>,
index: usize,
field: &Field,
cattrs: &attr::Container,
expecting: &str,
) -> TokenStream {
match field.attrs.default() {
attr::Default::Default => {
let span = field.original.span();
return quote_spanned!(span=> #assign_to _serde::__private::Default::default());
}
attr::Default::Path(path) => {
return quote_spanned!(path.span()=> #assign_to #path());
}
attr::Default::None => { /* below */ }
}

match *cattrs.default() {
attr::Default::Default | attr::Default::Path(_) => {
let member = &field.member;
return quote!(#assign_to __default.#member);
}
attr::Default::None => quote!(
return _serde::__private::Err(_serde::de::Error::invalid_length(#index, &#expecting))
),
}
}

fn effective_style(variant: &Variant) -> Style {
match variant.style {
Style::Newtype if variant.fields[0].attrs.skip_deserializing() => Style::Unit,
Expand Down
20 changes: 10 additions & 10 deletions serde_derive/src/internals/attr.rs
Expand Up @@ -405,20 +405,20 @@ impl Container {
if let Some(path) = parse_lit_into_expr_path(cx, DEFAULT, &meta)? {
match &item.data {
syn::Data::Struct(syn::DataStruct { fields, .. }) => match fields {
syn::Fields::Named(_) => {
syn::Fields::Named(_) | syn::Fields::Unnamed(_) => {
default.set(&meta.path, Default::Path(path));
}
syn::Fields::Unnamed(_) | syn::Fields::Unit => {
let msg = "#[serde(default = \"...\")] can only be used on structs with named fields";
syn::Fields::Unit => {
let msg = "#[serde(default = \"...\")] can only be used on structs with fields";
cx.syn_error(meta.error(msg));
}
},
syn::Data::Enum(_) => {
let msg = "#[serde(default = \"...\")] can only be used on structs with named fields";
let msg = "#[serde(default = \"...\")] can only be used on structs with fields";
cx.syn_error(meta.error(msg));
}
syn::Data::Union(_) => {
let msg = "#[serde(default = \"...\")] can only be used on structs with named fields";
let msg = "#[serde(default = \"...\")] can only be used on structs with fields";
cx.syn_error(meta.error(msg));
}
}
Expand All @@ -427,20 +427,20 @@ impl Container {
// #[serde(default)]
match &item.data {
syn::Data::Struct(syn::DataStruct { fields, .. }) => match fields {
syn::Fields::Named(_) => {
syn::Fields::Named(_) | syn::Fields::Unnamed(_) => {
default.set(meta.path, Default::Default);
}
syn::Fields::Unnamed(_) | syn::Fields::Unit => {
let msg = "#[serde(default)] can only be used on structs with named fields";
syn::Fields::Unit => {
let msg = "#[serde(default)] can only be used on structs with fields";
cx.error_spanned_by(fields, msg);
}
},
syn::Data::Enum(_) => {
let msg = "#[serde(default)] can only be used on structs with named fields";
let msg = "#[serde(default)] can only be used on structs with fields";
cx.syn_error(meta.error(msg));
}
syn::Data::Union(_) => {
let msg = "#[serde(default)] can only be used on structs with named fields";
let msg = "#[serde(default)] can only be used on structs with fields";
cx.syn_error(meta.error(msg));
}
}
Expand Down
38 changes: 37 additions & 1 deletion serde_derive/src/internals/check.rs
@@ -1,11 +1,12 @@
use crate::internals::ast::{Container, Data, Field, Style};
use crate::internals::attr::{Identifier, TagType};
use crate::internals::attr::{Default, Identifier, TagType};
use crate::internals::{ungroup, Ctxt, Derive};
use syn::{Member, Type};

// Cross-cutting checks that require looking at more than a single attrs object.
// Simpler checks should happen when parsing and building the attrs.
pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
check_default_on_tuple(cx, cont);
check_remote_generic(cx, cont);
check_getter(cx, cont);
check_flatten(cx, cont);
Expand All @@ -17,6 +18,41 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
check_from_and_try_from(cx, cont);
}

/// If some field of tuple is marked as `#[serde(default)]` then all subsequent
/// fields also should be marked with that attribute or the struct itself should
/// have this attribute. This is because using default value for a field is
/// possible only if the sequence is exhausted that means that all subsequent
/// fields will fail to deserialize and should provide a default value if we want
/// the successful deserialization.
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 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!("struct or field must have #[serde(default)] because previous field {} have #[serde(default)]", first),
);
}
continue;
}
if let None = first_default_index {
first_default_index = Some(i);
}
}
}
}
// TODO: Warn if container has default and all fields also marked with default
// when warnings in proc-macro become available
}

// Remote derive definition type must have either all of the generics of the
// remote type:
//
Expand Down
2 changes: 1 addition & 1 deletion test_suite/tests/ui/default-attribute/enum.stderr
@@ -1,4 +1,4 @@
error: #[serde(default)] can only be used on structs with named fields
error: #[serde(default)] can only be used on structs with fields
--> tests/ui/default-attribute/enum.rs:4:9
|
4 | #[serde(default)]
Expand Down
2 changes: 1 addition & 1 deletion test_suite/tests/ui/default-attribute/enum_path.stderr
@@ -1,4 +1,4 @@
error: #[serde(default = "...")] can only be used on structs with named fields
error: #[serde(default = "...")] can only be used on structs with fields
--> tests/ui/default-attribute/enum_path.rs:4:9
|
4 | #[serde(default = "default_e")]
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

48 changes: 48 additions & 0 deletions test_suite/tests/ui/default-attribute/tuple_struct.rs
@@ -0,0 +1,48 @@
use serde_derive::Deserialize;

/// No errors expected
#[derive(Deserialize)]
struct T0(u8, u8);

/// No errors expected:
/// - if both fields are provided, both gets value from data
/// - if only one field is provided, the second gets default value
#[derive(Deserialize)]
struct T1(u8, #[serde(default)] u8);

/// Errors expected -- the first field can get default value only if sequence is
/// empty, but that mean that all other fields cannot be deserialized without
/// errors, so the `#[serde(default)]` attribute is superfluous
#[derive(Deserialize)]
struct T2(#[serde(default)] u8, u8, u8);

/// No errors expected:
/// - if both fields are provided, both gets value from data
/// - if only one field is provided, the second gets default value
/// - if none fields are provided, both gets default value
#[derive(Deserialize)]
struct T3(#[serde(default)] u8, #[serde(default)] u8);

////////////////////////////////////////////////////////////////////////////////

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T4(u8, u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T5(#[serde(default)] u8, u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T6(u8, #[serde(default)] u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T7(#[serde(default)] u8, #[serde(default)] u8);

fn main() {}
11 changes: 11 additions & 0 deletions test_suite/tests/ui/default-attribute/tuple_struct.stderr
@@ -0,0 +1,11 @@
error: struct or field must have #[serde(default)] because previous field 0 have #[serde(default)]
--> tests/ui/default-attribute/tuple_struct.rs:17:33
|
17 | struct T2(#[serde(default)] u8, u8, u8);
| ^^

error: struct or field must have #[serde(default)] because previous field 0 have #[serde(default)]
--> tests/ui/default-attribute/tuple_struct.rs:17:37
|
17 | struct T2(#[serde(default)] u8, u8, u8);
| ^^
77 changes: 77 additions & 0 deletions test_suite/tests/ui/default-attribute/tuple_struct_path.rs
@@ -0,0 +1,77 @@
use serde_derive::Deserialize;

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

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

/// Errors expected -- the first field can get default value only if sequence is
/// empty, but that mean that all other fields cannot be deserialized without
/// errors, so the `#[serde(default)]` attribute is superfluous
#[derive(Deserialize)]
struct T2(#[serde(default = "d")] u8, u8, u8);

/// No errors expected:
/// - if both fields are provided, both gets value from data
/// - if only one field is provided, the second gets default value
/// - if none fields are provided, both gets default value
#[derive(Deserialize)]
struct T3(#[serde(default = "d")] u8, #[serde(default = "d")] u8);

////////////////////////////////////////////////////////////////////////////////

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T1D(#[serde(default = "d")] u8, u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T2D(u8, #[serde(default = "d")] u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize, Default)]
#[serde(default)]
struct T3D(#[serde(default = "d")] u8, #[serde(default = "d")] u8);

////////////////////////////////////////////////////////////////////////////////

/// No errors expected -- missing fields gets default values
#[derive(Deserialize)]
#[serde(default = "d")]
struct T1Path(#[serde(default)] u8, u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize)]
#[serde(default = "d")]
struct T2Path(u8, #[serde(default)] u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize)]
#[serde(default = "d")]
struct T3Path(#[serde(default)] u8, #[serde(default)] u8);

////////////////////////////////////////////////////////////////////////////////

/// No errors expected -- missing fields gets default values
#[derive(Deserialize)]
#[serde(default = "d")]
struct T1PathD(#[serde(default = "d")] u8, u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize)]
#[serde(default = "d")]
struct T2PathD(u8, #[serde(default = "d")] u8);

/// No errors expected -- missing fields gets default values
#[derive(Deserialize)]
#[serde(default = "d")]
struct T3PathD(#[serde(default = "d")] u8, #[serde(default = "d")] u8);

fn main() {}
11 changes: 11 additions & 0 deletions test_suite/tests/ui/default-attribute/tuple_struct_path.stderr
@@ -0,0 +1,11 @@
error: struct or field must have #[serde(default)] because previous field 0 have #[serde(default)]
--> tests/ui/default-attribute/tuple_struct_path.rs:17:39
|
17 | struct T2(#[serde(default = "d")] u8, u8, u8);
| ^^

error: struct or field must have #[serde(default)] because previous field 0 have #[serde(default)]
--> tests/ui/default-attribute/tuple_struct_path.rs:17:43
|
17 | struct T2(#[serde(default = "d")] u8, u8, u8);
| ^^