diff --git a/build.rs b/build.rs index c740b3c4e..ccfd2deb8 100644 --- a/build.rs +++ b/build.rs @@ -231,19 +231,16 @@ fn generate_bindings(defines: &[(&str, &str)], includes: &[PathBuf]) -> Result Args<'a> { // If the variable is `&[&Zval]` treat it as the variadic argument. let default = defaults.remove(ident); - let nullable = type_is_nullable(ty.as_ref(), default.is_some())?; + let nullable = type_is_nullable(ty.as_ref())?; let (variadic, as_ref, ty) = Self::parse_typed(ty); result.typed.push(TypedArg { name: ident, @@ -570,18 +570,20 @@ impl<'a> Args<'a> { /// # Parameters /// /// * `optional` - The first optional argument. If [`None`], the optional - /// arguments will be from the first nullable argument after the last - /// non-nullable argument to the end of the arguments. + /// arguments will be from the first optional argument (nullable or has + /// default) after the last required argument to the end of the arguments. pub fn split_args(&self, optional: Option<&Ident>) -> (&[TypedArg<'a>], &[TypedArg<'a>]) { let mut mid = None; for (i, arg) in self.typed.iter().enumerate() { + // An argument is optional if it's nullable (Option) or has a default value. + let is_optional = arg.nullable || arg.default.is_some(); if let Some(optional) = optional { if optional == arg.name { mid.replace(i); } - } else if mid.is_none() && arg.nullable { + } else if mid.is_none() && is_optional { mid.replace(i); - } else if !arg.nullable { + } else if !is_optional { mid.take(); } } @@ -635,7 +637,7 @@ impl TypedArg<'_> { None }; let default = self.default.as_ref().map(|val| { - let val = val.to_token_stream().to_string(); + let val = expr_to_php_stub(val); quote! { .default(#val) } @@ -659,8 +661,49 @@ impl TypedArg<'_> { fn accessor(&self, bail_fn: impl Fn(TokenStream) -> TokenStream) -> TokenStream { let name = self.name; if let Some(default) = &self.default { - quote! { - #name.val().unwrap_or(#default.into()) + if self.nullable { + // For nullable types with defaults, null is acceptable + quote! { + #name.val().unwrap_or(#default.into()) + } + } else { + // For non-nullable types with defaults: + // - If argument was omitted: use default + // - If null was explicitly passed: throw TypeError + // - If a value was passed: try to convert it + let bail_null = bail_fn(quote! { + ::ext_php_rs::exception::PhpException::new( + concat!("Argument `$", stringify!(#name), "` must not be null").into(), + 0, + ::ext_php_rs::zend::ce::type_error(), + ) + }); + let bail_invalid = bail_fn(quote! { + ::ext_php_rs::exception::PhpException::default( + concat!("Invalid value given for argument `", stringify!(#name), "`.").into() + ) + }); + quote! { + match #name.zval() { + Some(zval) if zval.is_null() => { + // Null was explicitly passed to a non-nullable parameter + #bail_null + } + Some(_) => { + // A value was passed, try to convert it + match #name.val() { + Some(val) => val, + None => { + #bail_invalid + } + } + } + None => { + // Argument was omitted, use default + #default.into() + } + } + } } } else if self.variadic { let variadic_name = format_ident!("__variadic_{}", name); @@ -692,21 +735,132 @@ impl TypedArg<'_> { } } -/// Returns true of the given type is nullable in PHP. +/// Converts a Rust expression to a PHP stub-compatible default value string. +/// +/// This function handles common Rust patterns and converts them to valid PHP +/// syntax for use in generated stub files: +/// +/// - `None` → `"null"` +/// - `Some(expr)` → converts the inner expression +/// - `42`, `3.14` → numeric literals as-is +/// - `true`/`false` → as-is +/// - `"string"` → `"string"` +/// - `"string".to_string()` or `String::from("string")` → `"string"` +fn expr_to_php_stub(expr: &Expr) -> String { + match expr { + // Handle None -> null + Expr::Path(path) => { + let path_str = path.path.to_token_stream().to_string(); + if path_str == "None" { + "null".to_string() + } else if path_str == "true" || path_str == "false" { + path_str + } else { + // For other paths (constants, etc.), use the raw representation + path_str + } + } + + // Handle Some(expr) -> convert inner expression + Expr::Call(call) => { + if let Expr::Path(func_path) = &*call.func { + let func_name = func_path.path.to_token_stream().to_string(); + + // Some(value) -> convert inner value + if func_name == "Some" + && let Some(arg) = call.args.first() + { + return expr_to_php_stub(arg); + } + + // String::from("...") -> "..." + if (func_name == "String :: from" || func_name == "String::from") + && let Some(arg) = call.args.first() + { + return expr_to_php_stub(arg); + } + } + + // Default: use raw representation + expr.to_token_stream().to_string() + } + + // Handle method calls like "string".to_string() + Expr::MethodCall(method_call) => { + let method_name = method_call.method.to_string(); + + // "...".to_string() or "...".to_owned() or "...".into() -> "..." + if method_name == "to_string" || method_name == "to_owned" || method_name == "into" { + return expr_to_php_stub(&method_call.receiver); + } + + // Default: use raw representation + expr.to_token_stream().to_string() + } + + // String literals -> keep as-is (already valid PHP) + Expr::Lit(lit) => match &lit.lit { + syn::Lit::Str(s) => format!( + "\"{}\"", + s.value().replace('\\', "\\\\").replace('"', "\\\"") + ), + syn::Lit::Int(i) => i.to_string(), + syn::Lit::Float(f) => f.to_string(), + syn::Lit::Bool(b) => if b.value { "true" } else { "false" }.to_string(), + syn::Lit::Char(c) => format!("\"{}\"", c.value()), + _ => expr.to_token_stream().to_string(), + }, + + // Handle arrays: [] or vec![] + Expr::Array(arr) => { + if arr.elems.is_empty() { + "[]".to_string() + } else { + let elems: Vec = arr.elems.iter().map(expr_to_php_stub).collect(); + format!("[{}]", elems.join(", ")) + } + } + + // Handle vec![] macro + Expr::Macro(m) => { + let macro_name = m.mac.path.to_token_stream().to_string(); + if macro_name == "vec" { + let tokens = m.mac.tokens.to_string(); + if tokens.trim().is_empty() { + return "[]".to_string(); + } + } + // Default: use raw representation + expr.to_token_stream().to_string() + } + + // Handle unary expressions like -42 + Expr::Unary(unary) => { + let inner = expr_to_php_stub(&unary.expr); + format!("{}{}", unary.op.to_token_stream(), inner) + } + + // Default: use raw representation + _ => expr.to_token_stream().to_string(), + } +} + +/// Returns true if the given type is nullable in PHP (i.e., it's an `Option`). +/// +/// Note: Having a default value does NOT make a type nullable. A parameter with +/// a default value is optional (can be omitted), but passing `null` explicitly +/// should still be rejected unless the type is `Option`. // TODO(david): Eventually move to compile-time constants for this (similar to // FromZval::NULLABLE). -pub fn type_is_nullable(ty: &Type, has_default: bool) -> Result { +pub fn type_is_nullable(ty: &Type) -> Result { Ok(match ty { - syn::Type::Path(path) => { - has_default - || path - .path - .segments - .iter() - .next_back() - .is_some_and(|seg| seg.ident == "Option") - } - syn::Type::Reference(_) => false, /* Reference cannot be nullable unless */ + Type::Path(path) => path + .path + .segments + .iter() + .next_back() + .is_some_and(|seg| seg.ident == "Option"), + Type::Reference(_) => false, /* Reference cannot be nullable unless */ // wrapped in `Option` (in that case it'd be a Path). _ => bail!(ty => "Unsupported argument type."), }) diff --git a/src/builders/class.rs b/src/builders/class.rs index 665650641..cb4d111d1 100644 --- a/src/builders/class.rs +++ b/src/builders/class.rs @@ -17,7 +17,13 @@ use crate::{ zend_fastcall, }; -type ConstantEntry = (String, Box Result>, DocComments); +/// A constant entry: (name, `value_closure`, docs, `stub_value`) +type ConstantEntry = ( + String, + Box Result>, + DocComments, + String, +); type PropertyDefault = Option Result>>; /// Builder for registering a class in PHP. @@ -140,8 +146,11 @@ impl ClassBuilder { value: impl IntoZval + 'static, docs: DocComments, ) -> Result { + // Convert to Zval first to get stub value + let zval = value.into_zval(true)?; + let stub = crate::convert::zval_to_stub(&zval); self.constants - .push((name.into(), Box::new(|| value.into_zval(true)), docs)); + .push((name.into(), Box::new(|| Ok(zval)), docs, stub)); Ok(self) } @@ -166,9 +175,14 @@ impl ClassBuilder { value: &'static dyn IntoZvalDyn, docs: DocComments, ) -> Result { + let stub = value.stub_value(); let value = Rc::new(value); - self.constants - .push((name.into(), Box::new(move || value.as_zval(true)), docs)); + self.constants.push(( + name.into(), + Box::new(move || value.as_zval(true)), + docs, + stub, + )); Ok(self) } @@ -375,7 +389,7 @@ impl ClassBuilder { } } - for (name, value, _) in self.constants { + for (name, value, _, _) in self.constants { let value = Box::into_raw(Box::new(value()?)); unsafe { zend_declare_class_constant( diff --git a/src/constant.rs b/src/constant.rs index 56baeeca5..dc5d852e5 100644 --- a/src/constant.rs +++ b/src/constant.rs @@ -13,6 +13,13 @@ use crate::ffi::{ /// Implemented on types which can be registered as a constant in PHP. pub trait IntoConst: Debug { + /// Returns the PHP stub representation of this constant value. + /// + /// This is used when generating PHP stub files for IDE autocompletion. + /// The returned string should be a valid PHP literal (e.g., `"hello"`, + /// `42`, `true`). + fn stub_value(&self) -> String; + /// Registers a global module constant in PHP, with the value as the content /// of self. This function _must_ be called in the module startup /// function, which is called after the module is initialized. The @@ -89,6 +96,10 @@ pub trait IntoConst: Debug { } impl IntoConst for String { + fn stub_value(&self) -> String { + self.as_str().stub_value() + } + fn register_constant_flags( &self, name: &str, @@ -101,6 +112,17 @@ impl IntoConst for String { } impl IntoConst for &str { + fn stub_value(&self) -> String { + // Escape special characters for PHP string literal + let escaped = self + .replace('\\', "\\\\") + .replace('\'', "\\'") + .replace('\n', "\\n") + .replace('\r', "\\r") + .replace('\t', "\\t"); + format!("'{escaped}'") + } + fn register_constant_flags( &self, name: &str, @@ -133,6 +155,10 @@ impl IntoConst for &str { } impl IntoConst for bool { + fn stub_value(&self) -> String { + if *self { "true" } else { "false" }.to_string() + } + fn register_constant_flags( &self, name: &str, @@ -169,6 +195,10 @@ impl IntoConst for bool { macro_rules! into_const_num { ($type: ty, $fn: expr) => { impl IntoConst for $type { + fn stub_value(&self) -> String { + self.to_string() + } + fn register_constant_flags( &self, name: &str, diff --git a/src/convert.rs b/src/convert.rs index 69aa75f2f..57db0b430 100644 --- a/src/convert.rs +++ b/src/convert.rs @@ -253,6 +253,85 @@ pub trait IntoZvalDyn { /// Returns the PHP type of the type. fn get_type(&self) -> DataType; + + /// Returns the PHP stub representation of this value. + /// + /// This is used when generating PHP stub files for IDE autocompletion. + /// The returned string should be a valid PHP literal. + fn stub_value(&self) -> String { + // Default implementation - convert to zval and format + match self.as_zval(false) { + Ok(zval) => zval_to_stub(&zval), + Err(_) => "null".to_string(), + } + } +} + +/// Converts a Zval to its PHP stub representation. +#[must_use] +#[allow(clippy::match_same_arms)] +pub fn zval_to_stub(zval: &Zval) -> String { + use crate::flags::DataType; + + match zval.get_type() { + DataType::Null | DataType::Undef => "null".to_string(), + DataType::True => "true".to_string(), + DataType::False => "false".to_string(), + DataType::Long => zval + .long() + .map_or_else(|| "null".to_string(), |v| v.to_string()), + DataType::Double => zval + .double() + .map_or_else(|| "null".to_string(), |v| v.to_string()), + DataType::String => { + if let Some(s) = zval.str() { + let escaped = s + .replace('\\', "\\\\") + .replace('\'', "\\'") + .replace('\n', "\\n") + .replace('\r', "\\r") + .replace('\t', "\\t"); + format!("'{escaped}'") + } else { + "null".to_string() + } + } + DataType::Array => { + #[allow(clippy::explicit_iter_loop)] + if let Some(arr) = zval.array() { + // Check if array has sequential numeric keys starting from 0 + let is_sequential = arr.iter().enumerate().all(|(i, (key, _))| { + matches!(key, crate::types::ArrayKey::Long(idx) if i64::try_from(i).is_ok_and(|ii| idx == ii)) + }); + + let mut parts = Vec::new(); + for (key, val) in arr.iter() { + let val_str = zval_to_stub(val); + if is_sequential { + parts.push(val_str); + } else { + match key { + crate::types::ArrayKey::Long(idx) => { + parts.push(format!("{idx} => {val_str}")); + } + crate::types::ArrayKey::String(key) => { + let key_escaped = key.replace('\\', "\\\\").replace('\'', "\\'"); + parts.push(format!("'{key_escaped}' => {val_str}")); + } + crate::types::ArrayKey::Str(key) => { + let key_escaped = key.replace('\\', "\\\\").replace('\'', "\\'"); + parts.push(format!("'{key_escaped}' => {val_str}")); + } + } + } + } + format!("[{}]", parts.join(", ")) + } else { + "[]".to_string() + } + } + _ => "null".to_string(), + } } impl IntoZvalDyn for T { diff --git a/src/describe/mod.rs b/src/describe/mod.rs index d86cc5f20..b6e1b434c 100644 --- a/src/describe/mod.rs +++ b/src/describe/mod.rs @@ -270,8 +270,11 @@ impl From for Class { constants: val .constants .into_iter() - .map(|(name, _, docs)| (name, docs)) - .map(Constant::from) + .map(|(name, _, docs, stub)| Constant { + name: name.into(), + value: Option::Some(stub.into()), + docs: docs.into(), + }) .collect::>() .into(), flags, @@ -385,9 +388,9 @@ impl From<(String, PropertyFlags, D, DocComments)> for Property { let static_ = flags.contains(PropertyFlags::Static); let vis = Visibility::from(flags); // TODO: Implement ty #376 - let ty = abi::Option::None; + let ty = Option::None; // TODO: Implement default #376 - let default = abi::Option::::None; + let default = Option::::None; // TODO: Implement nullable #376 let nullable = false; let docs = docs.into(); @@ -552,7 +555,7 @@ impl From<(String, DocComments)> for Constant { let (name, docs) = val; Constant { name: name.into(), - value: abi::Option::None, + value: Option::None, docs: docs.into(), } } @@ -560,10 +563,10 @@ impl From<(String, DocComments)> for Constant { impl From<(String, Box, DocComments)> for Constant { fn from(val: (String, Box, DocComments)) -> Self { - let (name, _, docs) = val; + let (name, value, docs) = val; Constant { name: name.into(), - value: abi::Option::None, + value: Option::Some(value.stub_value().into()), docs: docs.into(), } } diff --git a/src/describe/stub.rs b/src/describe/stub.rs index b204145e0..dd174998c 100644 --- a/src/describe/stub.rs +++ b/src/describe/stub.rs @@ -171,7 +171,18 @@ impl ToStub for Parameter { write!(buf, "...")?; } - write!(buf, "${}", self.name) + write!(buf, "${}", self.name)?; + + // Add default value to stub + if let Option::Some(default) = &self.default { + write!(buf, " = {default}")?; + } else if self.nullable { + // For nullable parameters without explicit default, add = null + // This makes Option parameters truly optional in PHP + write!(buf, " = null")?; + } + + Ok(()) } } diff --git a/tests/src/integration/defaults/defaults.php b/tests/src/integration/defaults/defaults.php index bc711a3f8..66681dccd 100644 --- a/tests/src/integration/defaults/defaults.php +++ b/tests/src/integration/defaults/defaults.php @@ -7,3 +7,21 @@ assert(test_defaults_multiple_option_arguments() === 'Default'); assert(test_defaults_multiple_option_arguments(a: 'a') === 'a'); assert(test_defaults_multiple_option_arguments(b: 'b') === 'b'); + +// Test that passing null to a non-nullable parameter with a default value throws TypeError +// (fixes: https://github.com/extphprs/ext-php-rs/issues/538) +$threw = false; +try { + test_defaults_integer(null); +} catch (TypeError $e) { + $threw = true; +} +assert($threw, 'Expected TypeError when passing null to non-nullable parameter with default'); + +// But passing null to a nullable parameter should still work +assert(test_defaults_nullable_string(null) === null); + +// Test nullable parameter with Some() default value +assert(test_defaults_nullable_with_some_default() === 'fallback', 'Should return fallback when called without arguments'); +assert(test_defaults_nullable_with_some_default(null) === null, 'Should return null when null is passed'); +assert(test_defaults_nullable_with_some_default('custom') === 'custom', 'Should return custom value when provided'); diff --git a/tests/src/integration/defaults/mod.rs b/tests/src/integration/defaults/mod.rs index 5b1e2a5a5..7bd59be46 100644 --- a/tests/src/integration/defaults/mod.rs +++ b/tests/src/integration/defaults/mod.rs @@ -22,11 +22,18 @@ pub fn test_defaults_multiple_option_arguments( Ok(a.or(b).unwrap_or_else(|| "Default".to_string())) } +#[php_function] +#[php(defaults(a = Some("fallback".to_string())))] +pub fn test_defaults_nullable_with_some_default(a: Option) -> Option { + a +} + pub fn build_module(builder: ModuleBuilder) -> ModuleBuilder { builder .function(wrap_function!(test_defaults_integer)) .function(wrap_function!(test_defaults_nullable_string)) .function(wrap_function!(test_defaults_multiple_option_arguments)) + .function(wrap_function!(test_defaults_nullable_with_some_default)) } #[cfg(test)]