refactor struct handling

less spaghetti code accumulation, more purpose oriented
functions

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

proxmox-api-macro/src/api_macro.rs

@@ -477,6 +477,15 @@ fn handle_struct_unnamed(
     })
 }
 
+struct StructField<'i, 't> {
+    def: ParameterDefinition,
+    ident: &'i Ident,
+    mem_id: isize,
+    string: String,
+    strlit: syn::LitStr,
+    ty: &'t syn::Type,
+}
+
 fn handle_struct_named(
     mut definition: Object,
     type_ident: &Ident,
@@ -498,60 +507,149 @@ fn handle_struct_named(
         // We currently fill the actual `default` values from the schema into Option<Foo>, but
         // really Option<Foo> should default to None even when there's a Default as our accessors
         // will fill in the default at use-time...
-        panic!("derive_default is not finished");
+        bail!("derive_default is not finished");
     }
 
-    let field_count = item.named.len();
-
     let type_s = type_ident.to_string();
     let type_span = type_ident.span();
     let type_str = syn::LitStr::new(&type_s, type_span);
-    let struct_type_str = syn::LitStr::new(&format!("struct {}", type_s), type_span);
-    let struct_type_field_str =
-        syn::LitStr::new(&format!("struct {} field name", type_s), type_span);
-    let visitor_ident = Ident::new(&format!("{}Visitor", type_s), type_span);
-
-    let mut serialize_entries = TokenStream::new();
-    let mut field_option_init_list = TokenStream::new();
-    let mut field_option_check_or_default_list = TokenStream::new();
-    let mut field_name_str_list = TokenStream::new(); // ` "member1", "member2", `
-    let mut field_ident_list = TokenStream::new(); // ` member1, member2, `
-    let mut field_name_matches = TokenStream::new(); // ` "member0" => 0, "member1" => 1, `
-    let mut field_value_matches = TokenStream::new();
-    let mut auto_methods = TokenStream::new();
-    let mut default_impl = TokenStream::new();
 
     let mut mem_id: isize = 0;
+    let mut fields = Vec::new();
     for field in item.named.iter() {
         mem_id += 1;
-
         let field_ident = field
             .ident
             .as_ref()
             .ok_or_else(|| c_format_err!(field => "missing field name"))?;
-        let field_s = field_ident.to_string();
+        let field_string = field_ident.to_string();
 
-        let def = field_def.remove(&field_s).ok_or_else(
+        let field_strlit = syn::LitStr::new(&field_string, field_ident.span());
-            || c_format_err!(field => "missing api description entry for field {}", field_s),
+
+        let def = field_def.remove(&field_string).ok_or_else(
+            || c_format_err!(field => "missing api description entry for field {}", field_string),
         )?;
         let def = ParameterDefinition::from_expression(def)?;
+        fields.push(StructField {
+            def,
+            ident: field_ident,
+            mem_id,
+            string: field_string,
+            strlit: field_strlit,
+            ty: &field.ty,
+        });
+    }
 
-        let field_span = field_ident.span();
+    let impl_serialize =
-        let field_str = syn::LitStr::new(&field_s, field_span);
+        named_struct_derive_serialize(item.span(), type_ident, &type_str, &fields)?;
+    let impl_deserialize =
+        named_struct_derive_deserialize(item.span(), type_ident, &type_str, &fields)?;
+    let accessors =
+        named_struct_impl_accessors(item.span(), type_ident, &fields)?;
 
-        field_name_str_list.extend(quote_spanned! { field_span => #field_str, });
+    let impl_default = if derive_default {
-        field_ident_list.extend(quote_spanned! { field_span => #field_ident, });
+        named_struct_impl_default(item.span(), type_ident, &fields)?
+    } else {
+        TokenStream::new()
+    };
 
-        serialize_entries.extend(quote_spanned! { field_span =>
+    let description = common.description;
+    let parse_cli = common.cli.quote(&type_ident);
+    Ok(quote_spanned! { item.span() =>
+        #impl_serialize
+
+        #impl_deserialize
+
+        #impl_default
+
+        #accessors
+
+        impl ::proxmox::api::ApiType for #type_ident {
+            fn type_info() -> &'static ::proxmox::api::TypeInfo {
+                const INFO: ::proxmox::api::TypeInfo = ::proxmox::api::TypeInfo {
+                    name: #type_str,
+                    description: #description,
+                    complete_fn: None, // FIXME!
+                    parse_cli: #parse_cli,
+                };
+                &INFO
+            }
+
+            fn verify(&self) -> ::std::result::Result<(), ::failure::Error> {
+                // FIXME: #verifiers
+                Ok(())
+            }
+        }
+    })
+}
+
+fn named_struct_derive_serialize(
+    span: Span,
+    type_ident: &Ident,
+    type_str: &syn::LitStr,
+    fields: &[StructField],
+) -> Result<TokenStream, Error> {
+    let field_count = fields.len();
+
+    let mut entries = TokenStream::new();
+    for field in fields {
+        let field_ident = field.ident;
+        let field_span = field.ident.span();
+        let field_str = &field.strlit;
+        entries.extend(quote_spanned! { field_span =>
             if !::proxmox::api::ApiType::should_skip_serialization(&self.#field_ident) {
                 state.serialize_field(#field_str, &self.#field_ident)?;
             }
         });
+    }
 
-        field_option_init_list.extend(quote_spanned! { field_span =>
+    Ok(quote_spanned! { span =>
-            let mut #field_ident = None;
+        impl ::serde::ser::Serialize for #type_ident {
+            fn serialize<S>(&self, serializer: S) -> ::std::result::Result<S::Ok, S::Error>
+            where
+                S: ::serde::ser::Serializer,
+            {
+                use ::serde::ser::SerializeStruct;
+                let mut state = serializer.serialize_struct(#type_str, #field_count)?;
+                #entries
+                state.end()
+            }
+        }
+    })
+}
+
+fn named_struct_derive_deserialize(
+    span: Span,
+    type_ident: &Ident,
+    type_str: &syn::LitStr,
+    fields: &[StructField],
+) -> Result<TokenStream, Error> {
+    let type_s = type_ident.to_string();
+    let struct_type_str = syn::LitStr::new(&format!("struct {}", type_s), type_ident.span());
+    let struct_type_field_str =
+        syn::LitStr::new(&format!("struct {} field name", type_s), type_ident.span());
+    let visitor_ident = Ident::new(&format!("{}Visitor", type_s), type_ident.span());
+
+    let mut field_ident_list = TokenStream::new(); // ` member1, member2, `
+    let mut field_name_matches = TokenStream::new(); // ` "member0" => 0, "member1" => 1, `
+    let mut field_name_str_list = TokenStream::new(); // ` "member1", "member2", `
+    let mut field_option_check_or_default_list = TokenStream::new();
+    let mut field_option_init_list = TokenStream::new();
+    let mut field_value_matches = TokenStream::new();
+    for field in fields {
+        let field_ident = field.ident;
+        let field_span = field.ident.span();
+        let field_str = &field.strlit;
+        let mem_id = field.mem_id;
+
+        field_ident_list.extend(quote_spanned! { field_span => #field_ident, });
+
+        field_name_matches.extend(quote_spanned! { field_span =>
+            #field_str => Field(#mem_id),
         });
 
+        field_name_str_list.extend(quote_spanned! { field_span => #field_str, });
+
         field_option_check_or_default_list.extend(quote_spanned! { field_span =>
             let #field_ident = ::proxmox::api::ApiType::deserialization_check(
                 #field_ident,
@@ -559,9 +657,10 @@ fn handle_struct_named(
             )?;
         });
 
-        field_name_matches.extend(quote_spanned! { field_span =>
+        field_option_init_list.extend(quote_spanned! { field_span =>
-            #field_str => Field(#mem_id),
+            let mut #field_ident = None;
         });
+
         field_value_matches.extend(quote_spanned! { field_span =>
             Field(#mem_id) => {
                 if #field_ident.is_some() {
@@ -570,68 +669,9 @@ fn handle_struct_named(
                 #field_ident = Some(_api_macro_map_.next_value()?);
             }
         });
-
-        if let Some(default) = def.default {
-            let field_ty = &field.ty;
-            let set_field_ident = Ident::new(&format!("set_{}", field_s), field_ident.span());
-
-            auto_methods.extend(quote_spanned! { default.span() =>
-                pub fn #field_ident(
-                    &self,
-                ) -> &<#field_ty as ::proxmox::api::meta::OrDefault>::Output {
-                    const DEF: <#field_ty as ::proxmox::api::meta::OrDefault>::Output = #default;
-                    ::proxmox::api::meta::OrDefault::or_default(&self.#field_ident, &DEF)
-                }
-
-                pub fn #set_field_ident(
-                    &mut self,
-                    value: <#field_ty as ::proxmox::api::meta::OrDefault>::Output,
-                ) {
-                    ::proxmox::api::meta::OrDefault::set(&mut self.#field_ident, value)
-                }
-            });
-
-            if derive_default {
-                default_impl.extend(quote_spanned! { field_span =>
-                    #field_ident: #default.into(),
-                });
-            }
-        } else {
-            if derive_default {
-                default_impl.extend(quote_spanned! { field_span =>
-                    #field_ident: Default::default(),
-                });
-            }
-        }
-    }
-
-    if derive_default {
-        default_impl = quote_spanned! { item.span() =>
-            impl ::std::default::Default for #type_ident {
-                fn default() -> Self {
-                    Self {
-                        #default_impl
-                    }
-                }
-            }
-        };
-    }
-
-    let description = common.description;
-    let parse_cli = common.cli.quote(&type_ident);
-    Ok(quote_spanned! { item.span() =>
-        impl ::serde::ser::Serialize for #type_ident {
-            fn serialize<S>(&self, serializer: S) -> ::std::result::Result<S::Ok, S::Error>
-            where
-                S: ::serde::ser::Serializer,
-            {
-                use ::serde::ser::SerializeStruct;
-                let mut state = serializer.serialize_struct(#type_str, #field_count)?;
-                #serialize_entries
-                state.end()
-            }
     }
 
+    Ok(quote_spanned! { span =>
         impl<'de> ::serde::de::Deserialize<'de> for #type_ident {
             fn deserialize<D>(deserializer: D) -> ::std::result::Result<Self, D::Error>
             where
@@ -713,28 +753,72 @@ fn handle_struct_named(
                 deserializer.deserialize_struct(#type_str, FIELDS, #visitor_ident)
             }
         }
-
+    })
-        impl ::proxmox::api::ApiType for #type_ident {
-            fn type_info() -> &'static ::proxmox::api::TypeInfo {
-                const INFO: ::proxmox::api::TypeInfo = ::proxmox::api::TypeInfo {
-                    name: #type_str,
-                    description: #description,
-                    complete_fn: None, // FIXME!
-                    parse_cli: #parse_cli,
-                };
-                &INFO
 }
 
-            fn verify(&self) -> ::std::result::Result<(), ::failure::Error> {
+fn named_struct_impl_accessors(
-                // FIXME: #verifiers
+    span: Span,
-                Ok(())
+    type_ident: &Ident,
+    fields: &[StructField],
+) -> Result<TokenStream, Error> {
+    let mut accessor_methods = TokenStream::new();
+
+    for field in fields {
+        if let Some(ref default) = field.def.default {
+            let field_ident = field.ident;
+            let field_ty = &field.ty;
+            let set_field_ident = Ident::new(&format!("set_{}", field.string), field_ident.span());
+
+            accessor_methods.extend(quote_spanned! { default.span() =>
+                pub fn #field_ident(
+                    &self,
+                ) -> &<#field_ty as ::proxmox::api::meta::OrDefault>::Output {
+                    const DEF: <#field_ty as ::proxmox::api::meta::OrDefault>::Output = #default;
+                    ::proxmox::api::meta::OrDefault::or_default(&self.#field_ident, &DEF)
+                }
+
+                pub fn #set_field_ident(
+                    &mut self,
+                    value: <#field_ty as ::proxmox::api::meta::OrDefault>::Output,
+                ) {
+                    ::proxmox::api::meta::OrDefault::set(&mut self.#field_ident, value)
+                }
+            });
         }
     }
 
-        #default_impl
+    Ok(quote_spanned! { span =>
-
         impl #type_ident {
-            #auto_methods
+            #accessor_methods
+        }
+    })
+}
+
+fn named_struct_impl_default(
+    span: Span,
+    type_ident: &Ident,
+    fields: &[StructField],
+) -> Result<TokenStream, Error> {
+    let mut entries = TokenStream::new();
+    for field in fields {
+        let field_ident = field.ident;
+        if let Some(ref default) = field.def.default {
+            entries.extend(quote_spanned! { field.ident.span() =>
+                #field_ident: #default.into(),
+            });
+        } else {
+            entries.extend(quote_spanned! { field.ident.span() =>
+                #field_ident: Default::default(),
+            });
+        }
+    }
+    Ok(quote_spanned! { span =>
+        impl ::std::default::Default for #type_ident {
+            fn default() -> Self {
+                Self {
+                    #entries
+                }
+            }
         }
     })
 }