Re: [pdm-devel] [PATCH proxmox v2 1/3] pve-api-types: schema2rust: generate arrays for types with format `-list`

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

I followed this up with a cleanup, see comments below.

On Fri, Oct 24, 2025 at 04:42:47PM +0200, Hannes Laimer wrote:
> Since [1] the PVE API does take in actual arrays for parameters with a
> format that ends in `-list`. With this we generate `Vec<String>` for
> those, return types are not affected, so they will still just generate a
> `String`.
> 
> [1] pve-common 69d9edcc ("section config: implement array support")
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> since v2, thanks @Wolfgang:
>  - check if ref(schema->format)
>  - drop not needed defind-ness check
>  - use `0` as init value for `__list_format_as_array` for consistency
>     (insterad of undef)
>  - small perl improvments
> 
>  pve-api-types/generator-lib/Schema2Rust.pm | 41 +++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm
> index c5ac4aad..322f6a01 100644
> --- a/pve-api-types/generator-lib/Schema2Rust.pm
> +++ b/pve-api-types/generator-lib/Schema2Rust.pm
> @@ -39,6 +39,7 @@ my $dedup_enum = {};
>  my $dedup_array_types = {};
>  
>  our $__err_path = '';
> +our $__list_format_as_array = 0;
>  
>  my sub to_doc_comment : prototype($);
>  my sub strip_doc_comment : prototype($);
> @@ -509,6 +510,8 @@ my sub print_method_without_body : prototype($$$$$) {
>                  print {$out} "        .maybe_bool_arg(\"$name\", p_$rust_name)\n";
>              } elsif ($arg->{is_string_list}) {
>                  print {$out} "        .maybe_list_arg(\"$name\", p_$rust_name)\n";
> +            } elsif ($arg->{is_option_vec}) {
> +                print {$out} "        .maybe_list_arg(\"$name\", &p_$rust_name)\n";

Replaced this to also deal with non-optional args.

>              } elsif ($arg->{optional}) {
>                  print {$out} "        .maybe_arg(\"$name\", &p_$rust_name)\n";
>              } else {
> @@ -534,7 +537,7 @@ my sub print_method_without_body : prototype($$$$$) {
>  
>                  if ($arg->{type} eq 'Option<bool>') {
>                      print {$out} "        .maybe_bool_arg(\"$name\", $rust_name)\n";
> -                } elsif ($arg->{is_string_list}) {
> +                } elsif ($arg->{is_string_list} || $arg->{is_option_vec}) {

Replaced this to also deal with non-optional args.

>                      print {$out} "        .maybe_list_arg(\"$name\", &$rust_name)\n";
>                  } elsif ($arg->{optional}) {
>                      print {$out} "        .maybe_arg(\"$name\", &$rust_name)\n";
> @@ -809,6 +812,27 @@ my sub type_of : prototype($) {
>      my ($schema) = @_;
>  
>      my $kind = $schema->{type};
> +
> +    # If type is 'string' but format ends with `-list`, treat as array
> +    # but only for inputs to endpoints.
> +    #
> +    # We want a typed Vec<String>, and since [1] the PVE API does accept
> +    # actual arrays for parameters with a format ending in `-list`.
> +    # This is only for inputs though, so we can only have Vec's for
> +    # inputs, returns are still string lists.
> +    #
> +    # [1] pve-common 69d9edcc ("section config: implement array support")
> +    if (
> +        $kind eq 'string'
> +        && defined($schema->{format})
> +        && !ref($schema->{format})
> +        && $schema->{format} =~ /-list$/
> +    ) {
> +        if ($__list_format_as_array) {
> +            return 'array';
> +        }
> +    }

I moved this into handle_def. `type_of` should return the original type,
not apply magic, as this is the *initial* thing we check in handle_def,
and we otherwise have no way of knowing that this was not *actually* an
array.

> +
>      return $kind if $kind;
>  
>      if (exists $schema->{properties}) {
> @@ -1132,6 +1156,14 @@ my sub array_type : prototype($$$) {
>          optional => undef,
>          description => '',
>      };
> +    if (!$schema->{items} && !ref($schema->{format}) && $schema->{format} =~ /-list$/) {
> +        my $format_name = $schema->{format} =~ s/-list$//r;
> +        $schema->{items} = {
> +            description => "List item of type $format_name.",
> +            format => $format_name,
> +            type => 'string',
> +        };
> +    }
>  
>      my $items = delete $schema->{items} or die "missing 'items' in array schema\n";
>      my $description = $items->{description};
> @@ -1242,6 +1274,7 @@ my sub make_struct_field : prototype($$$$) {
>      #my $schema = $$inout_schema;
>      # (in case the type was already an option type, don't duplicate the `Option<>`)
>      if ($optional && !$def->{optional}) {
> +        $def->{is_option_vec} = ($def->{type} =~ /^Vec<.*>$/) ? 1 : 0;

^ I removed this completely. It is not necessary, if the `$def` contains
the info that this is a "string list turned into an array".

The context here does not imply anything about why the type is currently
a `Vec` in rust, and `is_option_vec` is not telling enough to mean "this
is actually a string-list we want to concatenate with NUL bytes when
used as a parameter".

Also... why only deal with *optional* ones - we currently don't have
mandatory ones, but they do exist in the API.

>          $def->{type} = "Option<$def->{type}>";
>          $def->{api}->{optional} = $def->{optional} = true;
>          push $def->{attrs}->@*, "#[serde(default, skip_serializing_if = \"Option::is_none\")]";
> @@ -1578,6 +1611,8 @@ my sub url_parameters : prototype($) {
>  my sub method_parameters : prototype($$$$$) {
>      my ($def, $api_url, $param_name, $api_method, $rust_method_name) = @_;
>  
> +    local $__list_format_as_array = 1;
> +
>      my $url_params = url_parameters($api_url);
>  
>      my $parameters = $api_method->{parameters} // {};
> @@ -1611,6 +1646,7 @@ my sub method_parameters : prototype($$$$$) {
>          }
>  
>          if ($def->{optional}) {
> +            $def->{is_option_vec} = ($def->{type} =~ /^Vec<.*>$/) ? 1 : 0;

Same as above.

>              $def->{type} = "Option<$def->{type}>";
>          }
>  
> @@ -1638,6 +1674,7 @@ my sub method_parameters : prototype($$$$$) {
>  
>              handle_def($def, \$schema, namify_type($rust_method_name, $param));
>              if ($def->{optional}) {
> +                $def->{is_option_vec} = ($def->{type} =~ /^Vec<.*>$/) ? 1 : 0;

Same as above.

>                  $def->{type} = "Option<$def->{type}>";
>              }
>              push @$input, $def;
> @@ -1676,6 +1713,8 @@ my sub method_parameters : prototype($$$$$) {
>  my sub method_return_type : prototype($$$$) {
>      my ($def, $method, $return_name, $extra) = @_;
>  
> +    local $__list_format_as_array = 0;
> +
>      if (defined(my $returns = $extra->{'output-type'})) {
>          $def->{output_type} = $returns;
>          return;
> -- 
> 2.47.3


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel