implement descriptions for enum variants

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

proxmox-api-macro/src/api/enums.rs

@@ -4,8 +4,6 @@ use anyhow::Error;
 
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::quote_spanned;
-use syn::punctuated::Punctuated;
-use syn::Token;
 
 use super::Schema;
 use crate::serde;
@@ -42,25 +40,35 @@ pub fn handle_enum(
 
     let container_attrs = serde::ContainerAttrib::try_from(&enum_ty.attrs[..])?;
 
-    // with_capacity(enum_ty.variants.len());
+    let mut variants = TokenStream::new();
-    // doesn't exist O.o
-    let mut variants = Punctuated::<syn::LitStr, Token![,]>::new();
     for variant in &mut enum_ty.variants {
         match &variant.fields {
             syn::Fields::Unit => (),
             _ => bail!(variant => "api macro does not support enums with fields"),
         }
 
+        let (comment, _doc_span) = util::get_doc_comments(&variant.attrs)?;
+        if comment.is_empty() {
+            bail!(variant => "enum variant needs a description");
+        }
+
         let attrs = serde::SerdeAttrib::try_from(&variant.attrs[..])?;
-        if let Some(renamed) = attrs.rename {
+        let variant_string = if let Some(renamed) = attrs.rename {
-            variants.push(renamed.into_lit_str());
+            renamed.into_lit_str()
         } else if let Some(rename_all) = container_attrs.rename_all {
             let name = rename_all.apply_to_variant(&variant.ident.to_string());
-            variants.push(syn::LitStr::new(&name, variant.ident.span()));
+            syn::LitStr::new(&name, variant.ident.span())
         } else {
             let name = &variant.ident;
-            variants.push(syn::LitStr::new(&name.to_string(), name.span()));
+            syn::LitStr::new(&name.to_string(), name.span())
-        }
+        };
+
+        variants.extend(quote_spanned! { variant.ident.span() =>
+            ::proxmox::api::schema::EnumEntry {
+                value: #variant_string,
+                description: #comment,
+            },
+        });
     }
 
     let name = &enum_ty.ident;

proxmox-api-macro/tests/types.rs

@@ -3,7 +3,7 @@
 
 #![allow(dead_code)]
 
-use proxmox::api::schema;
+use proxmox::api::schema::{self, EnumEntry};
 use proxmox_api_macro::api;
 
 use anyhow::Error;
@@ -13,7 +13,10 @@ use serde_json::Value;
 #[api(
     type: String,
     description: "A string",
-    format: &schema::ApiStringFormat::Enum(&["ok", "not-ok"]),
+    format: &schema::ApiStringFormat::Enum(&[
+        EnumEntry::new("ok", "Ok"),
+        EnumEntry::new("not-ok", "Not OK"),
+    ]),
 )]
 //#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct OkString(String);
@@ -22,7 +25,10 @@ pub struct OkString(String);
 fn ok_string() {
     const TEST_SCHEMA: &'static ::proxmox::api::schema::Schema =
         &::proxmox::api::schema::StringSchema::new("A string")
-            .format(&schema::ApiStringFormat::Enum(&["ok", "not-ok"]))
+            .format(&schema::ApiStringFormat::Enum(&[
+                EnumEntry::new("ok", "Ok"),
+                EnumEntry::new("not-ok", "Not OK"),
+            ]))
             .schema();
     assert_eq!(TEST_SCHEMA, OkString::API_SCHEMA);
 }
@@ -107,9 +113,12 @@ fn renamed_struct() {
 #[serde(rename_all = "kebab-case")]
 /// A selection of either 'onekind', 'another-kind' or 'selection-number-three'.
 pub enum Selection {
+    /// The first kind.
     #[serde(rename = "onekind")]
     OneKind,
+    /// Some other kind.
     AnotherKind,
+    /// And yet another.
     SelectionNumberThree,
 }
 
@@ -120,9 +129,9 @@ fn selection_test() {
             "A selection of either \'onekind\', \'another-kind\' or \'selection-number-three\'.",
         )
         .format(&::proxmox::api::schema::ApiStringFormat::Enum(&[
-            "onekind",
+            EnumEntry::new("onekind", "The first kind."),
-            "another-kind",
+            EnumEntry::new("another-kind", "Some other kind."),
-            "selection-number-three",
+            EnumEntry::new("selection-number-three", "And yet another."),
         ]))
         .schema();
 

proxmox/src/api/cli/command.rs

@@ -20,26 +20,27 @@ use super::{completion::*, CliCommand, CliCommandMap, CommandLineInterface};
 /// - ``json-pretty``: JSON, human readable.
 ///
 pub const OUTPUT_FORMAT: Schema = StringSchema::new("Output format.")
-    .format(&ApiStringFormat::Enum(&["text", "json", "json-pretty"]))
+    .format(&ApiStringFormat::Enum(&[
+        EnumEntry::new("text", "plain text output"),
+        EnumEntry::new("json", "single-line json formatted output"),
+        EnumEntry::new("json-pretty", "pretty-printed json output"),
+    ]))
     .schema();
 
-fn parse_arguments(
+fn parse_arguments(prefix: &str, cli_cmd: &CliCommand, args: Vec<String>) -> Result<Value, Error> {
-    prefix: &str,
+    let (params, remaining) = match getopts::parse_arguments(
-    cli_cmd: &CliCommand,
+        &args,
-    args: Vec<String>,
+        cli_cmd.arg_param,
-) -> Result<Value, Error> {
+        &cli_cmd.fixed_param,
-
+        &cli_cmd.info.parameters,
-   let (params, remaining) =
+    ) {
-        match getopts::parse_arguments(
+        Ok((p, r)) => (p, r),
-            &args, cli_cmd.arg_param, &cli_cmd.fixed_param, &cli_cmd.info.parameters
+        Err(err) => {
-        ) {
+            let err_msg = err.to_string();
-            Ok((p, r)) => (p, r),
+            print_simple_usage_error(prefix, cli_cmd, &err_msg);
-            Err(err) => {
+            return Err(format_err!("{}", err_msg));
-                let err_msg = err.to_string();
+        }
-                print_simple_usage_error(prefix, cli_cmd, &err_msg);
+    };
-                return Err(format_err!("{}", err_msg));
-            }
-        };
 
     if !remaining.is_empty() {
         let err_msg = format!("got additional arguments: {:?}", remaining);
@@ -365,7 +366,10 @@ pub async fn run_async_cli_command<C: Into<CommandLineInterface>>(def: C) {
 
     let (prefix, args) = prepare_cli_command(&def);
 
-    if handle_command_future(Arc::new(def), &prefix, args).await.is_err() {
+    if handle_command_future(Arc::new(def), &prefix, args)
+        .await
+        .is_err()
+    {
         std::process::exit(-1);
     }
 }

proxmox/src/api/cli/completion.rs

@@ -37,12 +37,15 @@ fn get_property_completion(
     }
 
     match schema {
-        Schema::String(StringSchema { format: Some(format), .. }) => {
+        Schema::String(StringSchema {
-            if let ApiStringFormat::Enum(list) = format {
+            format: Some(format),
+            ..
+        }) => {
+            if let ApiStringFormat::Enum(variants) = format {
                 let mut completions = Vec::new();
-                for value in list.iter() {
+                for variant in variants.iter() {
-                    if value.starts_with(arg) {
+                    if variant.value.starts_with(arg) {
-                        completions.push((*value).to_string());
+                        completions.push(variant.value.to_string());
                     }
                 }
                 return completions;

proxmox/src/api/schema.rs

@@ -322,8 +322,8 @@ impl StringSchema {
                         bail!("value does not match the regex pattern");
                     }
                 }
-                ApiStringFormat::Enum(stringvec) => {
+                ApiStringFormat::Enum(variants) => {
-                    if stringvec.iter().find(|&e| *e == value) == None {
+                    if variants.iter().find(|&e| e.value == value).is_none() {
                         bail!("value '{}' is not defined in the enumeration.", value);
                     }
                 }
@@ -503,6 +503,21 @@ pub enum Schema {
     Array(ArraySchema),
 }
 
+/// A string enum entry. An enum entry must have a value and a description.
+#[derive(Clone, Debug)]
+#[cfg_attr(feature = "test-harness", derive(Eq, PartialEq))]
+pub struct EnumEntry {
+    pub value: &'static str,
+    pub description: &'static str,
+}
+
+impl EnumEntry {
+    /// Convenience method as long as we only have 2 mandatory fields in an `EnumEntry`.
+    pub const fn new(value: &'static str, description: &'static str) -> Self {
+        Self { value, description }
+    }
+}
+
 /// String microformat definitions.
 ///
 /// Strings are probably the most flexible data type, and there are
@@ -514,7 +529,10 @@ pub enum Schema {
 ///
 /// ```
 /// # use proxmox::api::{*, schema::*};
-/// const format: ApiStringFormat = ApiStringFormat::Enum(&["vm", "ct"]);
+/// const format: ApiStringFormat = ApiStringFormat::Enum(&[
+///     EnumEntry::new("vm", "A guest VM run via qemu"),
+///     EnumEntry::new("ct", "A guest container run via lxc"),
+/// ]);
 /// ```
 ///
 /// ## Regular Expressions
@@ -566,7 +584,7 @@ pub enum Schema {
 /// ```
 pub enum ApiStringFormat {
     /// Enumerate all valid strings
-    Enum(&'static [&'static str]),
+    Enum(&'static [EnumEntry]),
     /// Use a regular expression to describe valid strings.
     Pattern(&'static ConstRegexPattern),
     /// Use a schema to describe complex types encoded as string.
@@ -579,7 +597,7 @@ impl std::fmt::Debug for ApiStringFormat {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             ApiStringFormat::VerifyFn(fnptr) => write!(f, "VerifyFn({:p}", fnptr),
-            ApiStringFormat::Enum(strvec) => write!(f, "Enum({:?}", strvec),
+            ApiStringFormat::Enum(variants) => write!(f, "Enum({:?}", variants),
             ApiStringFormat::Pattern(regex) => write!(f, "Pattern({:?}", regex),
             ApiStringFormat::PropertyString(schema) => write!(f, "PropertyString({:?}", schema),
         }
@@ -874,12 +892,8 @@ pub fn verify_json_object(data: &Value, schema: &ObjectSchema) -> Result<(), Err
     for (key, value) in map {
         if let Some((_optional, prop_schema)) = schema.lookup(&key) {
             let result = match prop_schema {
-                Schema::Object(object_schema) => {
+                Schema::Object(object_schema) => verify_json_object(value, object_schema),
-                    verify_json_object(value, object_schema)
+                Schema::Array(array_schema) => verify_json_array(value, array_schema),
-                }
-                Schema::Array(array_schema) => {
-                    verify_json_array(value, array_schema)
-                }
                 _ => verify_json(value, prop_schema),
             };
             if let Err(err) = result {
@@ -1018,7 +1032,10 @@ fn test_query_string() {
                 "name",
                 false,
                 &StringSchema::new("Name.")
-                    .format(&ApiStringFormat::Enum(&["ev1", "ev2"]))
+                    .format(&ApiStringFormat::Enum(&[
+                        EnumEntry::new("ev1", "desc ev1"),
+                        EnumEntry::new("ev2", "desc ev2"),
+                    ]))
                     .schema(),
             )],
         );
@@ -1163,7 +1180,10 @@ fn test_verify_function() {
 
 #[test]
 fn test_verify_complex_object() {
-    const NIC_MODELS: ApiStringFormat = ApiStringFormat::Enum(&["e1000", "virtio"]);
+    const NIC_MODELS: ApiStringFormat = ApiStringFormat::Enum(&[
+        EnumEntry::new("e1000", "Intel E1000"),
+        EnumEntry::new("virtio", "Paravirtualized ethernet device"),
+    ]);
 
     const PARAM_SCHEMA: Schema = ObjectSchema::new(
         "Properties.",