[pbs-devel] [PATCH proxmox 11/18] api-macro: object schema entry tuple -> struct

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs  | 79 +++++++++++++---------------
 proxmox-api-macro/src/api/mod.rs     | 51 +++++++++++-------
 proxmox-api-macro/src/api/structs.rs | 36 ++++++-------
 3 files changed, 89 insertions(+), 77 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index 3e85d8c..d7b0021 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -263,16 +263,14 @@ fn handle_function_signature(
         };
 
         // For any named type which exists on the function signature...
-        if let Some((_ident, optional, ref mut schema)) =
-            input_schema.find_obj_property_by_ident_mut(&pat.ident.to_string())
-        {
+        if let Some(entry) = input_schema.find_obj_property_by_ident_mut(&pat.ident.to_string()) {
             // try to infer the type in the schema if it is not specified explicitly:
-            let is_option = util::infer_type(schema, &*pat_type.ty)?;
-            let has_default = schema.find_schema_property("default").is_some();
-            if !is_option && *optional && !has_default {
+            let is_option = util::infer_type(&mut entry.schema, &*pat_type.ty)?;
+            let has_default = entry.schema.find_schema_property("default").is_some();
+            if !is_option && entry.optional && !has_default {
                 error!(pat_type => "optional types need a default or be an Option<T>");
             }
-            if has_default && !*optional {
+            if has_default && !entry.optional {
                 error!(pat_type => "non-optional parameter cannot have a default");
             }
         } else {
@@ -313,40 +311,39 @@ fn handle_function_signature(
         //        bail out with an error.
         let pat_ident = pat.ident.unraw();
         let mut param_name: FieldName = pat_ident.clone().into();
-        let param_type = if let Some((name, optional, schema)) =
-            input_schema.find_obj_property_by_ident(&pat_ident.to_string())
-        {
-            if let SchemaItem::Inferred(span) = &schema.item {
-                bail!(*span, "failed to infer type");
-            }
-            param_name = name.clone();
-            // Found an explicit parameter: extract it:
-            ParameterType::Other(&pat_type.ty, *optional, schema)
-        } else if is_api_method_type(&pat_type.ty) {
-            if api_method_param.is_some() {
-                error!(pat_type => "multiple ApiMethod parameters found");
-                continue;
-            }
-            api_method_param = Some(param_list.len());
-            ParameterType::ApiMethod
-        } else if is_rpc_env_type(&pat_type.ty) {
-            if rpc_env_param.is_some() {
-                error!(pat_type => "multiple RpcEnvironment parameters found");
-                continue;
-            }
-            rpc_env_param = Some(param_list.len());
-            ParameterType::RpcEnv
-        } else if is_value_type(&pat_type.ty) {
-            if value_param.is_some() {
-                error!(pat_type => "multiple additional Value parameters found");
+        let param_type =
+            if let Some(entry) = input_schema.find_obj_property_by_ident(&pat_ident.to_string()) {
+                if let SchemaItem::Inferred(span) = &entry.schema.item {
+                    bail!(*span, "failed to infer type");
+                }
+                param_name = entry.name.clone();
+                // Found an explicit parameter: extract it:
+                ParameterType::Other(&pat_type.ty, entry.optional, &entry.schema)
+            } else if is_api_method_type(&pat_type.ty) {
+                if api_method_param.is_some() {
+                    error!(pat_type => "multiple ApiMethod parameters found");
+                    continue;
+                }
+                api_method_param = Some(param_list.len());
+                ParameterType::ApiMethod
+            } else if is_rpc_env_type(&pat_type.ty) {
+                if rpc_env_param.is_some() {
+                    error!(pat_type => "multiple RpcEnvironment parameters found");
+                    continue;
+                }
+                rpc_env_param = Some(param_list.len());
+                ParameterType::RpcEnv
+            } else if is_value_type(&pat_type.ty) {
+                if value_param.is_some() {
+                    error!(pat_type => "multiple additional Value parameters found");
+                    continue;
+                }
+                value_param = Some(param_list.len());
+                ParameterType::Value
+            } else {
+                error!(&pat_ident => "unexpected parameter {:?}", pat_ident.to_string());
                 continue;
-            }
-            value_param = Some(param_list.len());
-            ParameterType::Value
-        } else {
-            error!(&pat_ident => "unexpected parameter {:?}", pat_ident.to_string());
-            continue;
-        };
+            };
 
         param_list.push((param_name, param_type));
     }
@@ -594,7 +591,7 @@ impl<'a> DefaultParameters<'a> {
     fn get_default(&self, param_tokens: TokenStream) -> Result<syn::Expr, syn::Error> {
         let param_name: syn::LitStr = syn::parse2(param_tokens)?;
         match self.0.find_obj_property_by_ident(&param_name.value()) {
-            Some((_ident, _optional, schema)) => match schema.find_schema_property("default") {
+            Some(entry) => match entry.schema.find_schema_property("default") {
                 Some(def) => Ok(def.clone()),
                 None => bail!(param_name => "no default found in schema"),
             },
diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs
index 3cf1309..4690654 100644
--- a/proxmox-api-macro/src/api/mod.rs
+++ b/proxmox-api-macro/src/api/mod.rs
@@ -176,15 +176,12 @@ impl Schema {
         }
     }
 
-    fn find_obj_property_by_ident(&self, key: &str) -> Option<&(FieldName, bool, Schema)> {
+    fn find_obj_property_by_ident(&self, key: &str) -> Option<&ObjectEntry> {
         self.as_object()
             .and_then(|obj| obj.find_property_by_ident(key))
     }
 
-    fn find_obj_property_by_ident_mut(
-        &mut self,
-        key: &str,
-    ) -> Option<&mut (FieldName, bool, Schema)> {
+    fn find_obj_property_by_ident_mut(&mut self, key: &str) -> Option<&mut ObjectEntry> {
         self.as_object_mut()
             .and_then(|obj| obj.find_property_by_ident_mut(key))
     }
@@ -384,10 +381,26 @@ impl SchemaItem {
     }
 }
 
+pub struct ObjectEntry {
+    pub name: FieldName,
+    pub optional: bool,
+    pub schema: Schema,
+}
+
+impl ObjectEntry {
+    pub fn new(name: FieldName, optional: bool, schema: Schema) -> Self {
+        Self {
+            name,
+            optional,
+            schema,
+        }
+    }
+}
+
 #[derive(Default)]
 /// Contains a sorted list of properties:
 pub struct SchemaObject {
-    properties_: Vec<(FieldName, bool, Schema)>,
+    properties_: Vec<ObjectEntry>,
 }
 
 impl SchemaObject {
@@ -403,12 +416,12 @@ impl SchemaObject {
     }
 
     #[inline]
-    fn properties_mut(&mut self) -> &mut [(FieldName, bool, Schema)] {
+    fn properties_mut(&mut self) -> &mut [ObjectEntry] {
         &mut self.properties_
     }
 
     fn sort_properties(&mut self) {
-        self.properties_.sort_by(|a, b| (a.0).cmp(&b.0));
+        self.properties_.sort_by(|a, b| (a.name).cmp(&b.name));
     }
 
     fn try_extract_from(obj: &mut JSONObject) -> Result<Self, syn::Error> {
@@ -432,7 +445,7 @@ impl SchemaObject {
                             .transpose()?
                             .unwrap_or(false);
 
-                        properties.push((key, optional, schema.try_into()?));
+                        properties.push(ObjectEntry::new(key, optional, schema.try_into()?));
 
                         Ok(properties)
                     },
@@ -444,30 +457,32 @@ impl SchemaObject {
 
     fn to_schema_inner(&self, ts: &mut TokenStream) -> Result<(), Error> {
         for element in self.properties_.iter() {
-            let key = element.0.as_str();
-            let optional = element.1;
+            let key = element.name.as_str();
+            let optional = element.optional;
             let mut schema = TokenStream::new();
-            element.2.to_schema(&mut schema)?;
+            element.schema.to_schema(&mut schema)?;
             ts.extend(quote! { (#key, #optional, &#schema), });
         }
         Ok(())
     }
 
-    fn find_property_by_ident(&self, key: &str) -> Option<&(FieldName, bool, Schema)> {
-        self.properties_.iter().find(|p| p.0.as_ident_str() == key)
+    fn find_property_by_ident(&self, key: &str) -> Option<&ObjectEntry> {
+        self.properties_
+            .iter()
+            .find(|p| p.name.as_ident_str() == key)
     }
 
-    fn find_property_by_ident_mut(&mut self, key: &str) -> Option<&mut (FieldName, bool, Schema)> {
+    fn find_property_by_ident_mut(&mut self, key: &str) -> Option<&mut ObjectEntry> {
         self.properties_
             .iter_mut()
-            .find(|p| p.0.as_ident_str() == key)
+            .find(|p| p.name.as_ident_str() == key)
     }
 
     fn remove_property_by_ident(&mut self, key: &str) -> bool {
         match self
             .properties_
             .iter()
-            .position(|(name, _, _)| name.as_ident_str() == key)
+            .position(|entry| entry.name.as_ident_str() == key)
         {
             Some(index) => {
                 self.properties_.remove(index);
@@ -477,7 +492,7 @@ impl SchemaObject {
         }
     }
 
-    fn extend_properties(&mut self, new_fields: Vec<(FieldName, bool, Schema)>) {
+    fn extend_properties(&mut self, new_fields: Vec<ObjectEntry>) {
         self.properties_.extend(new_fields);
         self.sort_properties();
     }
diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs
index db6a290..e55378d 100644
--- a/proxmox-api-macro/src/api/structs.rs
+++ b/proxmox-api-macro/src/api/structs.rs
@@ -19,7 +19,7 @@ use proc_macro2::{Ident, Span, TokenStream};
 use quote::quote_spanned;
 
 use super::Schema;
-use crate::api::{self, SchemaItem};
+use crate::api::{self, ObjectEntry, SchemaItem};
 use crate::serde;
 use crate::util::{self, FieldName, JSONObject, Maybe};
 
@@ -126,19 +126,19 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
     //
     // NOTE: We remove references we're "done with" and in the end fail with a list of extraneous
     // fields if there are any.
-    let mut schema_fields: HashMap<String, &mut (FieldName, bool, Schema)> = HashMap::new();
+    let mut schema_fields: HashMap<String, &mut ObjectEntry> = HashMap::new();
 
     // We also keep a reference to the SchemaObject around since we derive missing fields
     // automatically.
     if let api::SchemaItem::Object(ref mut obj) = &mut schema.item {
         for field in obj.properties_mut() {
-            schema_fields.insert(field.0.as_str().to_string(), field);
+            schema_fields.insert(field.name.as_str().to_string(), field);
         }
     } else {
         error!(schema.span, "structs need an object schema");
     }
 
-    let mut new_fields: Vec<(FieldName, bool, Schema)> = Vec::new();
+    let mut new_fields: Vec<ObjectEntry> = Vec::new();
 
     let container_attrs = serde::ContainerAttrib::try_from(&stru.attrs[..])?;
 
@@ -170,19 +170,19 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
                     if attrs.flatten {
                         to_remove.push(name.clone());
 
-                        let name = &field_def.0;
-                        let optional = &field_def.1;
-                        let schema = &field_def.2;
-                        if schema.description.is_explicit() {
+                        if field_def.schema.description.is_explicit() {
                             error!(
-                                name.span(),
+                                field_def.name.span(),
                                 "flattened field should not have a description, \
                                  it does not appear in serialized data as a field",
                             );
                         }
 
-                        if *optional {
-                            error!(name.span(), "optional flattened fields are not supported");
+                        if field_def.optional {
+                            error!(
+                                field_def.name.span(),
+                                "optional flattened fields are not supported"
+                            );
                         }
                     }
 
@@ -190,12 +190,12 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 
                     if attrs.flatten {
                         all_of_schemas.extend(quote::quote! {&});
-                        field_def.2.to_schema(&mut all_of_schemas)?;
+                        field_def.schema.to_schema(&mut all_of_schemas)?;
                         all_of_schemas.extend(quote::quote! {,});
                     }
                 }
                 None => {
-                    let mut field_def = (
+                    let mut field_def = ObjectEntry::new(
                         FieldName::new(name.clone(), span),
                         false,
                         Schema::blank(span),
@@ -204,7 +204,7 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 
                     if attrs.flatten {
                         all_of_schemas.extend(quote::quote! {&});
-                        field_def.2.to_schema(&mut all_of_schemas)?;
+                        field_def.schema.to_schema(&mut all_of_schemas)?;
                         all_of_schemas.extend(quote::quote! {,});
                         to_remove.push(name.clone());
                     } else {
@@ -309,11 +309,11 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 ///
 /// For each field we derive the description from doc-attributes if available.
 fn handle_regular_field(
-    field_def: &mut (FieldName, bool, Schema),
+    field_def: &mut ObjectEntry,
     field: &syn::Field,
     derived: bool, // whether this field was missing in the schema
 ) -> Result<(), Error> {
-    let schema: &mut Schema = &mut field_def.2;
+    let schema: &mut Schema = &mut field_def.schema;
 
     if schema.description.is_none() {
         let (doc_comment, doc_span) = util::get_doc_comments(&field.attrs)?;
@@ -324,8 +324,8 @@ fn handle_regular_field(
 
     if is_option_type(&field.ty) {
         if derived {
-            field_def.1 = true;
-        } else if !field_def.1 {
+            field_def.optional = true;
+        } else if !field_def.optional {
             error!(&field.ty => "non-optional Option type?");
         }
     }
-- 
2.20.1