From f60e8fe9ffae357d1762b70c1dfff20f0550b774 Mon Sep 17 00:00:00 2001 From: jmjoy Date: Thu, 25 Jun 2026 23:24:55 +0800 Subject: [PATCH] refactor: simplify materialize_missing to accept full defaults list --- phper/src/values.rs | 27 +++++++---- tests/integration/src/execute_data.rs | 49 ++++++++------------ tests/integration/tests/php/execute_data.php | 19 ++++---- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/phper/src/values.rs b/phper/src/values.rs index bafcc42..0c83f66 100644 --- a/phper/src/values.rs +++ b/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, ) -> 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(()) } diff --git a/tests/integration/src/execute_data.rs b/tests/integration/src/execute_data.rs index dd2f27e..be26a49 100644 --- a/tests/integration/src/execute_data.rs +++ b/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 { 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 { - 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 { - 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", diff --git a/tests/integration/tests/php/execute_data.php b/tests/integration/tests/php/execute_data.php index 5ecc839..0669d0e 100644 --- a/tests/integration/tests/php/execute_data.php +++ b/tests/integration/tests/php/execute_data.php @@ -13,16 +13,19 @@ 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", @@ -30,7 +33,7 @@ function () { materialize_missing_exceed_error(); }, "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",