refactor: simplify materialize_missing to accept full defaults list (#241)

Author: jmjoy

phper/src/values.rs

@@ -242,24 +242,35 @@ impl ExecuteData {
         }
     }
 
-    /// Fills missing optional parameters from `num_args()` upward with the
-    /// given default values. Stops when the iterator runs out or
-    /// `common_num_args()` is reached.
+    /// Fills missing optional parameters with their default values.
+    ///
+    /// `defaults` is the full list of default values for **all** optional
+    /// parameters of the function. Already-provided optional arguments are
+    /// skipped automatically.
     ///
     /// `num_args` is updated to reflect the new count.
     ///
     /// # Errors
     ///
-    /// Returns [`CallArgError`] if filling would exceed `common_num_args()`.
+    /// Returns [`CallArgError`] if the total after filling would not match
+    /// `common_num_args()`.
     pub fn materialize_missing(
         &mut self, defaults: impl IntoIterator<Item = ZVal>,
     ) -> crate::Result<()> {
         let declared_len = self.common_num_args() as usize;
+        let required_len = self.common_required_num_args();
         let passed_len = self.num_args();
+
+        if passed_len >= declared_len {
+            return Ok(());
+        }
+
         let execute_data_ptr = self.as_mut_ptr();
+
+        let provided_optionals = passed_len.saturating_sub(required_len);
         let mut i = passed_len;
 
-        for mut default in defaults {
+        for mut default in defaults.into_iter().skip(provided_optionals) {
             if i >= declared_len {
                 return Err(CallArgError::new(i, declared_len).into());
             }
@@ -276,10 +287,8 @@ impl ExecuteData {
         if i != declared_len {
             return Err(CallArgError::new(i, declared_len).into());
         }
-        if i > passed_len {
-            unsafe {
-                phper_zend_set_call_num_args(execute_data_ptr, i.try_into().unwrap());
-            }
+        unsafe {
+            phper_zend_set_call_num_args(execute_data_ptr, i.try_into().unwrap());
         }
         Ok(())
     }

tests/integration/src/execute_data.rs

@@ -11,57 +11,48 @@
 use phper::{functions::Argument, modules::Module, values::ZVal};
 
 pub fn integrate(module: &mut Module) {
-    materialize_missing_fill(module);
-    materialize_missing_noop(module);
-    materialize_missing_partial(module);
-    materialize_missing_exceed_error(module);
-    materialize_missing_insufficient_error(module);
+    register_two_optionals(module);
+    register_no_optionals(module);
+    register_exceed_error(module);
+    register_insufficient_error(module);
 }
 
-fn materialize_missing_fill(module: &mut Module) {
+fn register_two_optionals(module: &mut Module) {
     module
         .add_function_with_execute_data(
-            "materialize_missing_fill",
+            "materialize_missing_two_optionals",
             |execute_data, _arguments| -> phper::Result<String> {
                 execute_data.materialize_missing([ZVal::from(42), ZVal::from("hello")])?;
                 let a = execute_data.get_parameter(0).expect_long()?;
                 let b = execute_data.get_parameter(1).expect_z_str()?.to_str()?;
-                Ok(format!("{}, {}", a, b))
+                let c = execute_data.get_parameter(2).expect_long()?;
+                let d = execute_data.get_parameter(3).expect_z_str()?.to_str()?;
+                Ok(format!("{}, {}, {}, {}", a, b, c, d))
             },
         )
-        .arguments([Argument::new("a").optional(), Argument::new("b").optional()]);
+        .arguments([
+            Argument::new("a"),
+            Argument::new("b"),
+            Argument::new("c").optional(),
+            Argument::new("d").optional(),
+        ]);
 }
 
-fn materialize_missing_noop(module: &mut Module) {
+fn register_no_optionals(module: &mut Module) {
     module
         .add_function_with_execute_data(
-            "materialize_missing_noop",
+            "materialize_missing_no_optionals",
             |execute_data, _arguments| -> phper::Result<String> {
-                let passed = execute_data.num_args();
                 execute_data.materialize_missing([])?;
                 let a = execute_data.get_parameter(0).expect_long()?;
                 let b = execute_data.get_parameter(1).expect_z_str()?.to_str()?;
-                Ok(format!("{}, {}, {}", passed, a, b))
-            },
-        )
-        .arguments([Argument::new("a"), Argument::new("b")]);
-}
-
-fn materialize_missing_partial(module: &mut Module) {
-    module
-        .add_function_with_execute_data(
-            "materialize_missing_partial",
-            |execute_data, _arguments| -> phper::Result<String> {
-                execute_data.materialize_missing([ZVal::from(42)])?;
-                let a = execute_data.get_parameter(0).expect_z_str()?.to_str()?;
-                let b = execute_data.get_parameter(1).expect_long()?;
                 Ok(format!("{}, {}", a, b))
             },
         )
-        .arguments([Argument::new("a").optional(), Argument::new("b").optional()]);
+        .arguments([Argument::new("a"), Argument::new("b")]);
 }
 
-fn materialize_missing_exceed_error(module: &mut Module) {
+fn register_exceed_error(module: &mut Module) {
     module
         .add_function_with_execute_data(
             "materialize_missing_exceed_error",
@@ -73,7 +64,7 @@ fn materialize_missing_exceed_error(module: &mut Module) {
         .arguments([Argument::new("a").optional(), Argument::new("b").optional()]);
 }
 
-fn materialize_missing_insufficient_error(module: &mut Module) {
+fn register_insufficient_error(module: &mut Module) {
     module
         .add_function_with_execute_data(
             "materialize_missing_insufficient_error",

tests/integration/tests/php/execute_data.php

@@ -13,24 +13,27 @@
 
 require_once __DIR__ . '/_common.php';
 
-// Test 1: fill all missing arguments with defaults
-assert_eq(materialize_missing_fill(), "42, hello");
+// 2 required + 2 optional, 2 args provided → fill both optionals
+assert_eq(materialize_missing_two_optionals(1, "world"), "1, world, 42, hello");
 
-// Test 2: provide all arguments, no filling needed
-assert_eq(materialize_missing_noop(1, "world"), "2, 1, world");
+// 2 required + 2 optional, 3 args provided → skip 1st default, fill 2nd
+assert_eq(materialize_missing_two_optionals(1, "world", 10), "1, world, 10, hello");
 
-// Test 3: partial fill - only second arg is missing
-assert_eq(materialize_missing_partial("hello"), "hello, 42");
+// 2 required + 2 optional, all 4 args provided → no-op
+assert_eq(materialize_missing_two_optionals(1, "world", 10, "foo"), "1, world, 10, foo");
 
-// Test 4: exceed declared args causes TypeError
+// no optional params → no-op
+assert_eq(materialize_missing_no_optionals(1, "world"), "1, world");
+
+// defaults exceed declared param count
 assert_throw(
     function () { materialize_missing_exceed_error(); },
     "TypeError",
     0,
     "call arg index 2 out of bounds: must be in [0, 2) (declared_len = 2)"
 );
 
-// Test 5: insufficient defaults causes TypeError
+// not enough defaults
 assert_throw(
     function () { materialize_missing_insufficient_error(); },
     "TypeError",