macro: add cli property and remove specialization

Drop #!feature(specialization) in favor of having a `cli`
property for types to decide whether they are CLI
compatible.

The unconstrained_type! macro now has both ParseCli and
ParseCliFromStr in view, and requires one of the two to be
implemented for a type. This means that if a type implements
FromStr, it should "just work".

For types created without the help of the #[api] macro,
there's a shortcut to exclude a type from the CLI via
the no_cli_type!{typename} macro.

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

proxmox-api-macro/src/api_def.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::convert::TryFrom;
 
 use proc_macro2::TokenStream;
 
@@ -8,9 +9,85 @@ use quote::{quote, ToTokens};
 
 use super::parsing::Expression;
 
+#[derive(Clone)]
+pub enum CliMode {
+    Disabled,
+    ParseCli, // By default we try proxmox::cli::ParseCli
+    FromStr,
+    Function(syn::Expr),
+}
+
+impl Default for CliMode {
+    fn default() -> Self {
+        CliMode::ParseCli
+    }
+}
+
+impl TryFrom<Expression> for CliMode {
+    type Error = Error;
+    fn try_from(expr: Expression) -> Result<Self, Error> {
+        if expr.is_ident("FromStr") {
+            return Ok(CliMode::FromStr);
+        }
+
+        if let Ok(value) = expr.is_lit_bool() {
+            return Ok(if value.value {
+                CliMode::ParseCli
+            } else {
+                CliMode::Disabled
+            });
+        }
+
+        Ok(CliMode::Function(expr.expect_expr()?))
+    }
+}
+
+impl CliMode {
+    pub fn quote(&self, name: &proc_macro2::Ident) -> TokenStream {
+        match self {
+            CliMode::Disabled => quote! { None },
+            CliMode::ParseCli => quote! { Some(<#name as ::proxmox::api::cli::ParseCli>::parse_cli) },
+            CliMode::FromStr => quote! {
+                Some(<#name as ::proxmox::api::cli::ParseCliFromStr>::parse_cli)
+            },
+            CliMode::Function(func) => quote! { Some(#func) },
+        }
+    }
+}
+
+#[derive(Builder)]
+pub struct CommonTypeDefinition {
+    pub description: syn::LitStr,
+    #[builder(default)]
+    pub cli: CliMode,
+}
+
+impl CommonTypeDefinition {
+    fn builder() -> CommonTypeDefinitionBuilder {
+        CommonTypeDefinitionBuilder::default()
+    }
+
+    pub fn from_object(obj: &mut HashMap<String, Expression>) -> Result<Self, Error> {
+        let mut def = Self::builder();
+
+        if let Some(value) = obj.remove("description") {
+            def.description(value.expect_lit_str()?);
+        }
+        if let Some(value) = obj.remove("cli") {
+            def.cli(CliMode::try_from(value)?);
+        }
+
+        match def.build() {
+            Ok(r) => Ok(r),
+            Err(err) => bail!("{}", err),
+        }
+    }
+}
+
 #[derive(Builder)]
 pub struct ParameterDefinition {
-    pub description: syn::LitStr,
+    #[builder(default)]
+    pub description: Option<syn::LitStr>,
     #[builder(default)]
     pub validate: Option<syn::Expr>,
     #[builder(default)]
@@ -30,7 +107,7 @@ impl ParameterDefinition {
         for (key, value) in obj {
             match key.as_str() {
                 "description" => {
-                    def.description(value.expect_lit_str()?);
+                    def.description(Some(value.expect_lit_str()?));
                 }
                 "validate" => {
                     def.validate(Some(value.expect_expr()?));

proxmox-api-macro/src/api_macro.rs

@@ -6,7 +6,7 @@ use failure::{bail, format_err, Error};
 use quote::{quote, ToTokens};
 use syn::{Expr, Token};
 
-use super::api_def::ParameterDefinition;
+use super::api_def::{CommonTypeDefinition, ParameterDefinition};
 use super::parsing::*;
 
 pub fn api_macro(attr: TokenStream, item: TokenStream) -> Result<TokenStream, Error> {
@@ -423,7 +423,7 @@ fn handle_struct(
 }
 
 fn handle_struct_unnamed(
-    definition: HashMap<String, Expression>,
+    mut definition: HashMap<String, Expression>,
     name: &Ident,
     item: &syn::FieldsUnnamed,
 ) -> Result<TokenStream, Error> {
@@ -434,6 +434,7 @@ fn handle_struct_unnamed(
 
     //let field = fields.first().unwrap().value();
 
+    let common = CommonTypeDefinition::from_object(&mut definition)?;
     let apidef = ParameterDefinition::from_object(definition)?;
 
     let validator = match apidef.validate {
@@ -441,14 +442,18 @@ fn handle_struct_unnamed(
         None => quote! { ::proxmox::api::ApiType::verify(&self.0) },
     };
 
+    let description = common.description;
+    let parse_cli = common.cli.quote(&name);
     Ok(quote! {
         impl ::proxmox::api::ApiType for #name {
             fn type_info() -> &'static ::proxmox::api::TypeInfo {
+                use ::proxmox::api::cli::ParseCli;
+                use ::proxmox::api::cli::ParseCliFromStr;
                 const INFO: ::proxmox::api::TypeInfo = ::proxmox::api::TypeInfo {
                     name: stringify!(#name),
-                    description: "FIXME",
+                    description: #description,
                     complete_fn: None, // FIXME!
-                    parse_cli: Some(<#name as ::proxmox::api::cli::ParseCli>::parse_cli),
+                    parse_cli: #parse_cli,
                 };
                 &INFO
             }
@@ -461,32 +466,28 @@ fn handle_struct_unnamed(
 }
 
 fn handle_struct_named(
-    definition: HashMap<String, Expression>,
+    mut definition: HashMap<String, Expression>,
     name: &Ident,
     item: &syn::FieldsNamed,
 ) -> Result<TokenStream, Error> {
     let mut verify_entries = None;
-    let mut description = None;
+    let common = CommonTypeDefinition::from_object(&mut definition)?;
     for (key, value) in definition {
         match key.as_str() {
             "fields" => {
                 verify_entries = Some(handle_named_struct_fields(item, value.expect_object()?)?);
             }
-            "description" => {
-                description = Some(value.expect_lit_str()?);
-            }
             other => bail!("unknown api definition field: {}", other),
         }
     }
 
-    let description = description
-        .ok_or_else(|| format_err!("missing 'description' for type {}", name.to_string()))?;
-
     use std::iter::FromIterator;
     let verifiers = TokenStream::from_iter(
         verify_entries.ok_or_else(|| format_err!("missing 'fields' definition for struct"))?,
     );
 
+    let description = common.description;
+    let parse_cli = common.cli.quote(&name);
     Ok(quote! {
         impl ::proxmox::api::ApiType for #name {
             fn type_info() -> &'static ::proxmox::api::TypeInfo {
@@ -494,7 +495,7 @@ fn handle_struct_named(
                     name: stringify!(#name),
                     description: #description,
                     complete_fn: None, // FIXME!
-                    parse_cli: Some(<#name as ::proxmox::api::cli::ParseCli>::parse_cli),
+                    parse_cli: #parse_cli,
                 };
                 &INFO
             }

proxmox-api-macro/src/parsing.rs

@@ -196,6 +196,19 @@ impl Expression {
         }
     }
 
+    pub fn is_lit_bool(&self) -> Result<syn::LitBool, Error> {
+        match self {
+            Expression::Expr(expr) => match expr {
+                Expr::Lit(lit) => match &lit.lit {
+                    Lit::Bool(lit) => Ok(lit.clone()),
+                    other => bail!("expected boolean literal, got: {:?}", other),
+                },
+                other => bail!("expected boolean literal, got: {:?}", other),
+            },
+            _ => bail!("expected boolean literal"),
+        }
+    }
+
     pub fn expect_lit_bool(self) -> Result<syn::LitBool, Error> {
         match self {
             Expression::Expr(expr) => match expr {
@@ -232,6 +245,16 @@ impl Expression {
             _ => bail!("expected a type name, got {:?}", self),
         }
     }
+
+    pub fn is_ident(&self, ident: &str) -> bool {
+        match self {
+            Expression::Expr(expr) => match expr {
+                Expr::Path(path) => path.path.is_ident(Ident::new(ident, Span::call_site())),
+                _ => false,
+            },
+            _ => false,
+        }
+    }
 }
 
 pub fn parse_object2(tokens: TokenStream) -> Result<HashMap<String, Expression>, Error> {

proxmox-api-macro/tests/basic.rs

@@ -15,6 +15,9 @@ use proxmox::api::{api, Router};
 #[repr(transparent)]
 pub struct HostOrIp(String);
 
+// We don't bother with the CLI interface in this test:
+proxmox::api::no_cli_type! {HostOrIp}
+
 // Simplified for example purposes
 fn validate_hostname(name: &str) -> Result<(), Error> {
     if name == "<bad>" {
@@ -35,6 +38,7 @@ fn validate_hostname(name: &str) -> Result<(), Error> {
             maximum: 10000,
         },
     },
+    cli: false,
 })]
 #[derive(Deserialize, Serialize)]
 pub struct Person {

proxmox-api/src/api_type.rs

@@ -226,6 +226,9 @@ impl<T: ApiType> ApiType for Result<T, Error> {
 /// This is not supposed to be used, but can be if needed. This will provide an empty `ApiType`
 /// declaration with no description and no verifier.
 ///
+/// This requires that the type already implements the `ParseCli` trait (or has a `parse_cli` type
+/// of the same signature in view from any other trait).
+///
 /// This rarely makes sense, but sometimes a `string` is just a `string`.
 #[macro_export]
 macro_rules! unconstrained_api_type {
@@ -236,11 +239,12 @@ macro_rules! unconstrained_api_type {
             }
 
             fn type_info() -> &'static $crate::TypeInfo {
+                use $crate::cli::ParseCli;
                 const INFO: $crate::TypeInfo = $crate::TypeInfo {
                     name: stringify!($type),
                     description: stringify!($type),
                     complete_fn: None,
-                    parse_cli: Some(<$type as $crate::cli::ParseCli>::parse_cli),
+                    parse_cli: Some(<$type>::parse_cli),
                 };
                 &INFO
             }

proxmox-api/src/cli.rs

@@ -289,16 +289,42 @@ pub trait ParseCli {
     fn parse_cli(name: &str, value: Option<&str>) -> Result<Value, Error>;
 }
 
-// Saves us another mass impl macro such as the one below:
+/// This is a version of ParseCli with a default implementation falling to FromStr.
-impl<T> ParseCli for T {
+pub trait ParseCliFromStr
-    default fn parse_cli(name: &str, _value: Option<&str>) -> Result<Value, Error> {
+where
-        bail!(
+    Self: FromStr + Serialize,
-            "invalid type for command line interface found for parameter '{}'",
+    <Self as FromStr>::Err: Into<Error>,
-            name
+{
-        );
+    fn parse_cli(name: &str, value: Option<&str>) -> Result<Value, Error> {
+        parse_cli_from_str::<Self>(name, value)
     }
 }
 
+impl<T> ParseCliFromStr for T
+where
+    T: FromStr + Serialize,
+    <T as FromStr>::Err: Into<Error>,
+{}
+
+#[macro_export]
+macro_rules! no_cli_type {
+    ($type:ty $(, $more:ty)*) => {
+        impl $crate::cli::ParseCli for $type {
+            fn parse_cli(name: &str, _value: Option<&str>) -> Result<Value, Error> {
+                bail!(
+                    "invalid type for command line interface found for parameter '{}'",
+                    name
+                );
+            }
+        }
+
+        $crate::impl_parse_cli_from_str!{$($more),*}
+    };
+    () => {};
+}
+
+no_cli_type! {Vec<String>}
+
 #[macro_export]
 macro_rules! impl_parse_cli_from_str {
     ($type:ty $(, $more:ty)*) => {

proxmox-api/src/lib.rs

@@ -6,14 +6,6 @@
 //! Note that you'll rarely need the [`Router`] type itself, as you'll most likely be creating them
 //! with the `router` macro provided by the `proxmox-api-macro` crate.
 
-// This saves us some repetition (which we could do via macros), but this makes the code shorter
-// and easier to review.
-// FIXME: While the RFC has been approved a while ago and the implementation is there, there isn't
-// much activity on the issue tracker for this, so should we remove this?
-// Currently this is only used in cli.rs for a `default fn` which could instead made explicit for
-// our types
-#![feature(specialization)]
-
 use std::future::Future;
 use std::pin::Pin;
 

proxmox-api/tests/router.rs

@@ -151,6 +151,8 @@ mod methods {
     #[derive(Deserialize, Serialize)]
     pub struct CubicMeters(f64);
 
+    // We don't bother with the CLI interface in this test:
+    proxmox_api::no_cli_type! {CubicMeters}
     proxmox_api::unconstrained_api_type! {CubicMeters}
 
     #[derive(Deserialize, Serialize)]