Don’t require explicit index for skipped fields

This patch allows for fields that are always skipped to not have an
explicit index attribute so that users don’t have to assign bogus
indices that are never used.

Author: robin-nitrokey

src/lib.rs

@@ -53,8 +53,10 @@ fn serialize_fields(
 ) -> Vec<proc_macro2::TokenStream> {
     fields
         .iter()
-        .filter_map(|field| {
-            let index = field.index + offset;
+        .filter(|field| !field.skip_serializing_if.is_always())
+        .map(|field| {
+            // index should only be none if the field is always skipped, so this should never panic
+            let index = field.index.expect("index must be set for fields that are not skipped") + offset;
             let member = &field.member;
             let serialize_member = match &field.serialize_with {
                 None => quote!(&self.#member),
@@ -85,15 +87,15 @@ fn serialize_fields(
 
             // println!("field {:?} index {:?}", &field.label, field.index);
             match &field.skip_serializing_if {
-                Skip::If(path) => Some(quote! {
+                Skip::If(path) => quote! {
                     if !#path(&self.#member) {
                         map.serialize_entry(&#index, #serialize_member)?;
                     }
-                }),
-                Skip::Always => None,
-                Skip::Never => Some(quote! {
+                },
+                Skip::Always => unreachable!(),
+                Skip::Never => quote! {
                     map.serialize_entry(&#index, #serialize_member)?;
-                }),
+                },
             }
         })
         .collect()
@@ -222,7 +224,8 @@ fn match_fields(
         .map(|field| {
             let label = field.label.clone();
             let ident = format_ident!("{}", &field.label);
-            let index = field.index + offset;
+            // index should only be none if the field is always skipped, so this should never panic
+            let index = field.index.expect("index must be set for fields that are not skipped") + offset;
             let span = field.original_span;
 
             let next_value = match &field.deserialize_with {

src/parse.rs

@@ -36,7 +36,7 @@ impl Skip {
 pub struct Field {
     pub label: String,
     pub member: syn::Member,
-    pub index: usize,
+    pub index: Option<usize>,
     pub skip_serializing_if: Skip,
     pub serialize_with: Option<syn::ExprPath>,
     pub deserialize_with: Option<syn::ExprPath>,
@@ -221,15 +221,24 @@ fn parse_field(
         }
     }
 
-    let index = if attrs.auto_index {
-        auto_index
+    if explicit_index.is_some() && skip_serializing_if.is_always() {
+        return Err(Error::new_spanned(
+            field,
+            "`#[serde(index = ?]` and `#[serde(skip)]` cannot be combined",
+        ));
+    }
+
+    let index = if skip_serializing_if.is_always() {
+        None
+    } else if attrs.auto_index {
+        Some(auto_index)
     } else if let Some(index) = explicit_index {
         indices.push(index);
-        index
+        Some(index)
     } else {
         return Err(Error::new_spanned(
             field,
-            "Field without index attribute and `#[serde(auto_index)]` is not enabled on the struct",
+            "Field without index or skip attribute and `#[serde(auto_index)]` is not enabled on the struct",
         ));
     };
     Ok(Field {

tests/basics.rs

@@ -629,13 +629,16 @@ mod index {
         test2: usize,
         #[serde(index = 0x5A)]
         test3: usize,
+        #[serde(skip)]
+        test4: usize,
     }
 
     fn indices_example() -> WithIndices {
         WithIndices {
             test1: 42,
             test2: 1,
             test3: 99,
+            test4: 0,
         }
     }