fix(args): fix variadic args

Fixes: #337
Co-authored-by: Yoram <[email protected]>

Author: Xenira

crates/macros/src/function.rs

@@ -158,11 +158,6 @@ impl<'a> Function<'a> {
             .iter()
             .map(TypedArg::arg_builder)
             .collect::<Result<Vec<_>>>()?;
-        let variadic = self.args.typed.iter().any(|arg| arg.variadic).then(|| {
-            quote! {
-                .variadic()
-            }
-        });
         let returns = self.output.as_ref().map(|output| {
             quote! {
                 .returns(
@@ -219,7 +214,6 @@ impl<'a> Function<'a> {
                         #(.arg(&mut #required_arg_names))*
                         .not_required()
                         #(.arg(&mut #not_required_arg_names))*
-                        #variadic
                         .parse();
                     if parse_result.is_err() {
                         return;
@@ -264,7 +258,6 @@ impl<'a> Function<'a> {
             #(.arg(#required_args))*
             .not_required()
             #(.arg(#not_required_args))*
-            #variadic
             #returns
             #docs
         })
@@ -432,6 +425,7 @@ impl<'a> Args<'a> {
                 let as_ref = ref_.mutability.is_some();
                 match ref_.elem.as_ref() {
                     Type::Slice(slice) => (
+                        // TODO: Allow specifying the variadic type.
                         slice.elem.to_token_stream().to_string() == "& Zval",
                         as_ref,
                         ty.clone(),
@@ -512,6 +506,21 @@ impl TypedArg<'_> {
     fn clean_ty(&self) -> Type {
         let mut ty = self.ty.clone();
         ty.drop_lifetimes();
+
+        // Variadic arguments are passed as slices, so we need to extract the
+        // inner type.
+        if self.variadic {
+            let reference = if let Type::Reference(r) = &ty {
+                r
+            } else {
+                return ty;
+            };
+
+            if let Type::Slice(inner) = &*reference.elem {
+                return *inner.elem.clone();
+            }
+        }
+
         ty
     }
 
@@ -563,6 +572,10 @@ impl TypedArg<'_> {
             quote! {
                 #name.val().unwrap_or(#default.into())
             }
+        } else if self.variadic {
+            quote! {
+                &#name.variadic_vals()
+            }
         } else if self.nullable {
             // Originally I thought we could just use the below case for `null` options, as
             // `val()` will return `Option<Option<T>>`, however, this isn't the case when

src/args.rs

@@ -25,7 +25,7 @@ pub struct Arg<'a> {
     _type: DataType,
     as_ref: bool,
     allow_null: bool,
-    variadic: bool,
+    pub(crate) variadic: bool,
     default_value: Option<String>,
     zval: Option<&'a mut Zval>,
     variadic_zvals: Vec<Option<&'a mut Zval>>,
@@ -250,9 +250,12 @@ impl<'a, 'b> ArgParser<'a, 'b> {
     /// should break execution after seeing an error type.
     pub fn parse(mut self) -> Result<()> {
         let max_num_args = self.args.len();
-        let min_num_args = self.min_num_args.unwrap_or(max_num_args);
+        let mut min_num_args = self.min_num_args.unwrap_or(max_num_args);
         let num_args = self.arg_zvals.len();
         let has_variadic = self.args.last().is_some_and(|arg| arg.variadic);
+        if has_variadic {
+            min_num_args = min_num_args.saturating_sub(1);
+        }
 
         if num_args < min_num_args || (!has_variadic && num_args > max_num_args) {
             // SAFETY: Exported C function is safe, return value is unused and parameters

src/builders/function.rs

@@ -127,12 +127,6 @@ impl<'a> FunctionBuilder<'a> {
         self
     }
 
-    /// Sets the function to be variadic
-    pub fn variadic(mut self) -> Self {
-        self.function.flags |= MethodFlags::Variadic.bits();
-        self
-    }
-
     /// Sets the return value of the function.
     ///
     /// # Parameters
@@ -163,10 +157,21 @@ impl<'a> FunctionBuilder<'a> {
     /// Returns a result containing the function entry if successful.
     pub fn build(mut self) -> Result<FunctionEntry> {
         let mut args = Vec::with_capacity(self.args.len() + 1);
+        let mut n_req = self.n_req.unwrap_or(self.args.len());
+        let variadic = self.args.last().is_some_and(|arg| arg.variadic);
+
+        if variadic {
+            self.function.flags |= MethodFlags::Variadic.bits();
+            n_req = n_req.saturating_sub(1);
+        }
 
         // argument header, retval etc
+        // The first argument is used as `zend_internal_function_info` for the function.
+        // That struct shares the same memory as `zend_internal_arg_info` which is used
+        // for the arguments.
         args.push(ArgInfo {
-            name: self.n_req.unwrap_or(self.args.len()) as *const _,
+            // required_num_args
+            name: n_req as *const _,
             type_: match self.retval {
                 Some(retval) => {
                     ZendType::empty_from_type(retval, self.ret_as_ref, false, self.ret_as_null)

tests/src/integration/variadic_args.php

@@ -0,0 +1,36 @@
+<?php
+
+require "_utils.php";
+
+$a = 'a';
+$b = 'b';
+$c = 'c';
+
+$array = [$b, 'a','c'];
+
+// Passing arguments as references
+$args = test_variadic_args();
+assert($args === [], 'Expected no arguments to be returned');
+
+$args = test_variadic_args($a);
+assert($args === ['a'], 'Expected to return argument $a');
+
+$args = test_variadic_args($a, $b, $c);
+assert($args === ['a', 'b', 'c'], 'Expected to return arguments $a, $b and $c');
+
+$args = test_variadic_args(...$array);
+assert($args === ['b', 'a', 'c'], 'Expected to return an array with the array $array');
+
+assert_exception_thrown('test_variadic_add_required');
+
+// Values directly passed
+$sum = test_variadic_add_required(1, 2, 3); // 1
+assert($sum === 6, 'Expected to return 6');
+
+$count = test_variadic_add_required(11); // 11
+assert($count === 11, 'Allow only one argument');
+
+$types = test_variadic_args('a', 1, ['abc', 'def', 0.01], true, new stdClass);
+assert(gettype(end($types[2])) === 'double', 'Type of argument 2 and its last element should be a float of 0.01');
+assert($types[3], 'Arg 4 should be boolean true');
+assert($types[4] instanceof stdClass, 'Last argument is an instance of an StdClass');

tests/src/integration/variadic_args.rs

@@ -0,0 +1,4 @@
+#[test]
+fn test_variadic_args() {
+    assert!(crate::integration::run_php("variadic_args.php"));
+}

tests/src/lib.rs

@@ -165,6 +165,22 @@ fn key_to_zval(key: ArrayKey) -> Zval {
     }
 }
 
+// Rust type &[&Zval] must be converted because to Vec<Zval> because of
+// lifetime hell.
+#[php_function]
+pub fn test_variadic_args(params: &[&Zval]) -> Vec<Zval> {
+    params.iter().map(|x| x.shallow_clone()).collect()
+}
+
+#[php_function]
+pub fn test_variadic_add_required(number: u32, numbers: &[&Zval]) -> u32 {
+    number
+        + numbers
+            .iter()
+            .map(|x| x.long().unwrap() as u32)
+            .sum::<u32>()
+}
+
 #[php_class]
 pub struct TestClass {
     string: String,
@@ -233,6 +249,8 @@ pub fn build_module(module: ModuleBuilder) -> ModuleBuilder {
         .function(wrap_function!(iter_back))
         .function(wrap_function!(iter_next_back))
         .function(wrap_function!(test_class))
+        .function(wrap_function!(test_variadic_args))
+        .function(wrap_function!(test_variadic_add_required))
 }
 
 #[cfg(test)]
@@ -304,4 +322,5 @@ mod integration {
     mod object;
     mod string;
     mod types;
+    mod variadic_args;
 }