From 7d34e6f3a403a3201fa28d4e079c56a7b50fcbb0 Mon Sep 17 00:00:00 2001
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Date: Mon, 26 Oct 2020 18:29:27 +0100
Subject: [PATCH] api macro: reuse generated default const for "unwrap_or"

Instead of setting a default value to a const and inside an
.unwrap_or_else closure, lets set it only to the const and reuse that
later in .unwrap_or

To achieve that we move the "unrwap_or" code for param plumbing code generation
a bit later so that we have easy access to the generated const name.
As all this code is related to optional/default-value stuff it does read still
relatively OK with that change, IMO.

This has the advantage of not getting a warning like:

>  warning: constant is never used: `API_METHOD_EXAMPLE_FOO_PARAM_DEFAULT_FORCE`
>   --> src/api2/node/foo.rs
>    |
> XY |             force: {
>    |             ^^^^^
>    = note: `#[warn(dead_code)]` on by default

When one has a API endpoint like:

> #[api(
>     input: {
>         properties: {
>             force: {
>                 type: bool,
>                 optional: true,
>                 default: false,
>             },
>         },
>     },
>     ...
> )]
> /// Example
> fn example_foo(force: bool) -> Result<(), Error> {
>     if force {
>         // do something
>     }
>     Ok(())
> }

It effectively changes the output for optional parameters with a default set
and no Option<T> from

> let p = p.unwrap_or_else(|| #default_value);

to

> let p = p.unwrap_or(#const_name_for_default);

where the "#const_name_for_default" is a pub const with value
"#default_value"

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index e94594f2..1d32c606 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -414,18 +414,8 @@ fn create_wrapper_function(
                             #name_str,
                         ))?
                     });
-                } else if util::is_option_type(ty).is_none() {
-                    // Optional parameter without an Option<T> type requires a default:
-                    if let Some(def) = &default_value {
-                        body.extend(quote_spanned! { span =>
-                            .unwrap_or_else(|| #def)
-                        });
-                    } else {
-                        bail!(ty => "Optional parameter without Option<T> requires a default");
-                    }
                 }
-                body.extend(quote_spanned! { span => ; });
-                args.extend(quote_spanned! { span => #arg_name, });
+                let no_option_type = util::is_option_type(ty).is_none();
 
                 if let Some(def) = &default_value {
                     let name_uc = name.as_ident().to_string().to_uppercase();
@@ -438,7 +428,17 @@ fn create_wrapper_function(
                     default_consts.extend(quote_spanned! { span =>
                         pub const #name: #ty = #def;
                     });
+                    if optional && no_option_type {
+                        // Optional parameter without an Option<T> type requires a default:
+                        body.extend(quote_spanned! { span =>
+                            .unwrap_or(#name)
+                        });
+                    }
+                } else if optional && no_option_type {
+                    bail!(ty => "Optional parameter without Option<T> requires a default");
                 }
+                body.extend(quote_spanned! { span => ; });
+                args.extend(quote_spanned! { span => #arg_name, });
             }
         }
     }