From aadc69027cc2bd9276cf13065f641b9037244820 Mon Sep 17 00:00:00 2001 From: Mingun Date: Tue, 8 Aug 2023 19:24:24 +0500 Subject: [PATCH] Disallow tuple variants with incorrect order of #[serde(default)] attributes (review this commit with "ignore whitespace changes" option on) --- serde_derive/src/internals/check.rs | 58 ++++++++++++------- test_suite/tests/ui/default-attribute/enum.rs | 18 ++++++ .../tests/ui/default-attribute/enum.stderr | 12 ++++ .../tests/ui/default-attribute/enum_path.rs | 22 +++++++ .../ui/default-attribute/enum_path.stderr | 16 ++++- 5 files changed, 102 insertions(+), 24 deletions(-) diff --git a/serde_derive/src/internals/check.rs b/serde_derive/src/internals/check.rs index 52b0f379f..166551c3c 100644 --- a/serde_derive/src/internals/check.rs +++ b/serde_derive/src/internals/check.rs @@ -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); } } } diff --git a/test_suite/tests/ui/default-attribute/enum.rs b/test_suite/tests/ui/default-attribute/enum.rs index 8ecb398e6..cb2431a66 100644 --- a/test_suite/tests/ui/default-attribute/enum.rs +++ b/test_suite/tests/ui/default-attribute/enum.rs @@ -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 }, } diff --git a/test_suite/tests/ui/default-attribute/enum.stderr b/test_suite/tests/ui/default-attribute/enum.stderr index 27fbc1466..51101b7cc 100644 --- a/test_suite/tests/ui/default-attribute/enum.stderr +++ b/test_suite/tests/ui/default-attribute/enum.stderr @@ -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), + | ^^ diff --git a/test_suite/tests/ui/default-attribute/enum_path.rs b/test_suite/tests/ui/default-attribute/enum_path.rs index ef43372d1..4f2dd4eb4 100644 --- a/test_suite/tests/ui/default-attribute/enum_path.rs +++ b/test_suite/tests/ui/default-attribute/enum_path.rs @@ -1,8 +1,30 @@ use serde_derive::Deserialize; +fn d() -> 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 }, } diff --git a/test_suite/tests/ui/default-attribute/enum_path.stderr b/test_suite/tests/ui/default-attribute/enum_path.stderr index 65ef3ac43..7ab56b36f 100644 --- a/test_suite/tests/ui/default-attribute/enum_path.stderr +++ b/test_suite/tests/ui/default-attribute/enum_path.stderr @@ -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), + | ^^