[pbs-devel] [PATCH] api macro: reuse generated default const for "unwrap_or"

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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>
---

So, I did not checked the surrounding stuff in depth, but it seemed relatively
clear what is happening here. So, while a good double check is surely good, and
soem implementation details may be discussed, it does not breaks tests (and I
negative checked that, to ensure there are tests for this) plus my use case
works too, the "constant is never used" warning goes away.

 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 e94594f..1d32c60 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, });
             }
         }
     }
-- 
2.27.0