Disallow tuple variants with incorrect order of #[serde(default)] att…

…ributes

(review this commit with "ignore whitespace changes" option on)

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);
         }
     }
 }

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 },
 }
 

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),
+   |                                  ^^

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

@@ -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 },
 }
 

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),
+   |                                        ^^