diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 64da56dbba..1a80756b7d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -68,7 +68,7 @@ jobs: - name: Run rustfmt run: cargo fmt --all -- --check - name: Run clippy - run: cargo clippy --all -- -D warnings + run: cargo clippy --workspace --all-targets --all-features -- -W clippy::pedantic -D warnings # Docs - name: Run rustdoc run: cargo rustdoc -- -D warnings diff --git a/build.rs b/build.rs index 4249964936..810c9065f3 100644 --- a/build.rs +++ b/build.rs @@ -2,6 +2,7 @@ //! This script is responsible for generating the bindings to the PHP Zend API. //! It also checks the PHP version for compatibility with ext-php-rs and sets //! configuration flags accordingly. +#![allow(clippy::inconsistent_digit_grouping)] #[cfg_attr(windows, path = "windows_build.rs")] #[cfg_attr(not(windows), path = "unix_build.rs")] mod impl_; @@ -19,21 +20,25 @@ use anyhow::{anyhow, bail, Context, Result}; use bindgen::RustTarget; use impl_::Provider; -const MIN_PHP_API_VER: u32 = 20200930; -const MAX_PHP_API_VER: u32 = 20240924; +const MIN_PHP_API_VER: u32 = 2020_09_30; +const MAX_PHP_API_VER: u32 = 2024_09_24; /// Provides information about the PHP installation. pub trait PHPProvider<'a>: Sized { /// Create a new PHP provider. + #[allow(clippy::missing_errors_doc)] fn new(info: &'a PHPInfo) -> Result; /// Retrieve a list of absolute include paths. + #[allow(clippy::missing_errors_doc)] fn get_includes(&self) -> Result>; /// Retrieve a list of macro definitions to pass to the compiler. + #[allow(clippy::missing_errors_doc)] fn get_defines(&self) -> Result>; /// Writes the bindings to a file. + #[allow(clippy::missing_errors_doc)] fn write_bindings(&self, bindings: String, writer: &mut impl Write) -> Result<()> { for line in bindings.lines() { writeln!(writer, "{line}")?; @@ -42,12 +47,14 @@ pub trait PHPProvider<'a>: Sized { } /// Prints any extra link arguments. + #[allow(clippy::missing_errors_doc)] fn print_extra_link_args(&self) -> Result<()> { Ok(()) } } /// Finds the location of an executable `name`. +#[must_use] pub fn find_executable(name: &str) -> Option { const WHICH: &str = if cfg!(windows) { "where" } else { "which" }; let cmd = Command::new(WHICH).arg(name).output().ok()?; @@ -59,7 +66,7 @@ pub fn find_executable(name: &str) -> Option { } } -/// Returns an environment variable's value as a PathBuf +/// Returns an environment variable's value as a `PathBuf` pub fn path_from_env(key: &str) -> Option { std::env::var_os(key).map(PathBuf::from) } @@ -85,6 +92,9 @@ pub struct PHPInfo(String); impl PHPInfo { /// Get the PHP info. + /// + /// # Errors + /// - `php -i` command failed to execute successfully pub fn get(php: &Path) -> Result { let cmd = Command::new(php) .arg("-i") @@ -108,6 +118,9 @@ impl PHPInfo { } /// Checks if thread safety is enabled. + /// + /// # Errors + /// - `PHPInfo` does not contain thread satety information pub fn thread_safety(&self) -> Result { Ok(self .get_key("Thread Safety") @@ -116,6 +129,9 @@ impl PHPInfo { } /// Checks if PHP was built with debug. + /// + /// # Errors + /// - `PHPInfo` does not contain debug build information pub fn debug(&self) -> Result { Ok(self .get_key("Debug Build") @@ -124,12 +140,18 @@ impl PHPInfo { } /// Get the php version. + /// + /// # Errors + /// - `PHPInfo` does not contain version number pub fn version(&self) -> Result<&str> { self.get_key("PHP Version") .context("Failed to get PHP version") } /// Get the zend version. + /// + /// # Errors + /// - `PHPInfo` does not contain php api version pub fn zend_version(&self) -> Result { self.get_key("PHP API") .context("Failed to get Zend version") @@ -202,7 +224,7 @@ fn generate_bindings(defines: &[(&str, &str)], includes: &[PathBuf]) -> Result Result Result<()> { + const PHP_81_API_VER: u32 = 2021_09_02; + const PHP_82_API_VER: u32 = 2022_08_29; + const PHP_83_API_VER: u32 = 2023_08_31; + const PHP_84_API_VER: u32 = 2024_09_24; + let version = info.zend_version()?; if !(MIN_PHP_API_VER..=MAX_PHP_API_VER).contains(&version) { @@ -245,14 +272,6 @@ fn check_php_version(info: &PHPInfo) -> Result<()> { // // The PHP version cfg flags should also stack - if you compile on PHP 8.2 you // should get both the `php81` and `php82` flags. - const PHP_81_API_VER: u32 = 20210902; - - const PHP_82_API_VER: u32 = 20220829; - - const PHP_83_API_VER: u32 = 20230831; - - const PHP_84_API_VER: u32 = 20240924; - println!( "cargo::rustc-check-cfg=cfg(php80, php81, php82, php83, php84, php_zts, php_debug, docs)" ); diff --git a/crates/cli/build.rs b/crates/cli/build.rs index a312acc09a..fd560f3ea7 100644 --- a/crates/cli/build.rs +++ b/crates/cli/build.rs @@ -4,5 +4,5 @@ //! time. fn main() { - println!("cargo:rustc-link-arg-bins=-rdynamic") + println!("cargo:rustc-link-arg-bins=-rdynamic"); } diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 5ca44ed638..2e592309e7 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -36,13 +36,17 @@ macro_rules! stub_symbols { pub type CrateResult = AResult<()>; /// Runs the CLI application. Returns nothing in a result on success. +/// +/// # Errors +/// +/// Returns an error if the application fails to run. pub fn run() -> CrateResult { let mut args: Vec<_> = std::env::args().collect(); // When called as a cargo subcommand, the second argument given will be the // subcommand, in this case `php`. We don't want this so we remove from args and // pass it to clap. - if args.get(1).map(|nth| nth == "php").unwrap_or(false) { + if args.get(1).is_some_and(|nth| nth == "php") { args.remove(1); } @@ -91,6 +95,7 @@ struct Install { /// Changes the path that the extension is copied to. This will not /// activate the extension unless `ini_path` is also passed. #[arg(long)] + #[allow(clippy::struct_field_names)] install_dir: Option, /// Path to the `php.ini` file to update with the new extension. #[arg(long)] @@ -166,7 +171,7 @@ impl Args { impl Install { pub fn handle(self) -> CrateResult { - let artifact = find_ext(&self.manifest)?; + let artifact = find_ext(self.manifest.as_ref())?; let ext_path = build_ext(&artifact, self.release)?; let (mut ext_dir, mut php_ini) = if let Some(install_dir) = self.install_dir { @@ -213,11 +218,11 @@ impl Install { let mut new_lines = vec![]; for line in BufReader::new(&file).lines() { let line = line.with_context(|| "Failed to read line from `php.ini`")?; - if !line.contains(&ext_line) { - new_lines.push(line); - } else { + if line.contains(&ext_line) { bail!("Extension already enabled."); } + + new_lines.push(line); } // Comment out extension if user specifies disable flag @@ -255,10 +260,14 @@ fn get_ext_dir() -> AResult { "Extension directory returned from PHP is not a valid directory: {:?}", ext_dir ); - } else { - std::fs::create_dir(&ext_dir) - .with_context(|| format!("Failed to create extension directory at {ext_dir:?}"))?; } + + std::fs::create_dir(&ext_dir).with_context(|| { + format!( + "Failed to create extension directory at {}", + ext_dir.display() + ) + })?; } Ok(ext_dir) } @@ -288,7 +297,7 @@ impl Remove { pub fn handle(self) -> CrateResult { use std::env::consts; - let artifact = find_ext(&self.manifest)?; + let artifact = find_ext(self.manifest.as_ref())?; let (mut ext_path, mut php_ini) = if let Some(install_dir) = self.install_dir { (install_dir, None) @@ -361,7 +370,7 @@ impl Stubs { let ext_path = if let Some(ext_path) = self.ext { ext_path } else { - let target = find_ext(&self.manifest)?; + let target = find_ext(self.manifest.as_ref())?; build_ext(&target, false)?.into() }; @@ -410,7 +419,7 @@ impl Stubs { } /// Attempts to find an extension in the target directory. -fn find_ext(manifest: &Option) -> AResult { +fn find_ext(manifest: Option<&PathBuf>) -> AResult { // TODO(david): Look for cargo manifest option or env let mut cmd = cargo_metadata::MetadataCommand::new(); if let Some(manifest) = manifest { @@ -493,13 +502,13 @@ fn build_ext(target: &Target, release: bool) -> AResult { } } cargo_metadata::Message::BuildFinished(b) => { - if !b.success { - bail!("Compilation failed, cancelling installation.") - } else { + if b.success { break; } + + bail!("Compilation failed, cancelling installation.") } - _ => continue, + _ => {} } } diff --git a/crates/macros/src/extern_.rs b/crates/macros/src/extern_.rs index 22c47499d7..4a55d4bdaf 100644 --- a/crates/macros/src/extern_.rs +++ b/crates/macros/src/extern_.rs @@ -1,8 +1,8 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{ - punctuated::Punctuated, spanned::Spanned as _, ForeignItemFn, ItemForeignMod, ReturnType, - Signature, Token, + punctuated::Punctuated, spanned::Spanned as _, token::Unsafe, ForeignItemFn, ItemForeignMod, + ReturnType, Signature, Token, }; use crate::prelude::*; @@ -23,7 +23,7 @@ fn parse_function(mut func: ForeignItemFn) -> Result { let ForeignItemFn { attrs, vis, sig, .. } = &mut func; - sig.unsafety = Some(Default::default()); // Function must be unsafe. + sig.unsafety = Some(Unsafe::default()); // Function must be unsafe. let Signature { ident, .. } = &sig; @@ -36,13 +36,13 @@ fn parse_function(mut func: ForeignItemFn) -> Result { let pat = &arg.pat; Some(quote! { &#pat }) } - _ => None, + syn::FnArg::Receiver(_) => None, }) .collect::>>() .ok_or_else(|| { err!(sig.span() => "`self` parameters are not permitted inside `#[php_extern]` blocks.") })?; - let ret = build_return(&name, &sig.output, params); + let ret = build_return(&name, &sig.output, ¶ms); Ok(quote! { #(#attrs)* #vis #sig { @@ -60,7 +60,7 @@ fn parse_function(mut func: ForeignItemFn) -> Result { fn build_return( name: &str, return_type: &ReturnType, - params: Punctuated, + params: &Punctuated, ) -> TokenStream { match return_type { ReturnType::Default => quote! { diff --git a/crates/macros/src/function.rs b/crates/macros/src/function.rs index 5148559259..d84363d9bf 100644 --- a/crates/macros/src/function.rs +++ b/crates/macros/src/function.rs @@ -132,27 +132,16 @@ impl<'a> Function<'a> { /// Generates the function builder for the function. pub fn function_builder(&self, call_type: CallType) -> TokenStream { - let ident = self.ident; let name = &self.name; let (required, not_required) = self.args.split_args(self.optional.as_ref()); // `handler` impl - let required_arg_names: Vec<_> = required.iter().map(|arg| arg.name).collect(); - let not_required_arg_names: Vec<_> = not_required.iter().map(|arg| arg.name).collect(); let arg_declarations = self .args .typed .iter() .map(TypedArg::arg_declaration) .collect::>(); - let arg_accessors = self.args.typed.iter().map(|arg| { - arg.accessor(|e| { - quote! { - #e.throw().expect("Failed to throw PHP exception."); - return; - } - }) - }); // `entry` impl let required_args = required @@ -163,7 +152,50 @@ impl<'a> Function<'a> { .iter() .map(TypedArg::arg_builder) .collect::>(); - let returns = self.output.as_ref().map(|output| { + + let returns = self.build_returns(); + let result = self.build_result(call_type, required, not_required); + let docs = if self.docs.is_empty() { + quote! {} + } else { + let docs = &self.docs; + quote! { + .docs(&[#(#docs),*]) + } + }; + + quote! { + ::ext_php_rs::builders::FunctionBuilder::new(#name, { + ::ext_php_rs::zend_fastcall! { + extern fn handler( + ex: &mut ::ext_php_rs::zend::ExecuteData, + retval: &mut ::ext_php_rs::types::Zval, + ) { + use ::ext_php_rs::convert::IntoZval; + + #(#arg_declarations)* + let result = { + #result + }; + + if let Err(e) = result.set_zval(retval, false) { + let e: ::ext_php_rs::exception::PhpException = e.into(); + e.throw().expect("Failed to throw PHP exception."); + } + } + } + handler + }) + #(.arg(#required_args))* + .not_required() + #(.arg(#not_required_args))* + #returns + #docs + } + } + + fn build_returns(&self) -> Option { + self.output.as_ref().map(|output| { quote! { .returns( <#output as ::ext_php_rs::convert::IntoZval>::TYPE, @@ -171,9 +203,29 @@ impl<'a> Function<'a> { <#output as ::ext_php_rs::convert::IntoZval>::NULLABLE, ) } + }) + } + + fn build_result( + &self, + call_type: CallType, + required: &[TypedArg<'_>], + not_required: &[TypedArg<'_>], + ) -> TokenStream { + let ident = self.ident; + let required_arg_names: Vec<_> = required.iter().map(|arg| arg.name).collect(); + let not_required_arg_names: Vec<_> = not_required.iter().map(|arg| arg.name).collect(); + + let arg_accessors = self.args.typed.iter().map(|arg| { + arg.accessor(|e| { + quote! { + #e.throw().expect("Failed to throw PHP exception."); + return; + } + }) }); - let result = match call_type { + match call_type { CallType::Function => quote! { let parse = ex.parser() #(.arg(&mut #required_arg_names))* @@ -227,44 +279,6 @@ impl<'a> Function<'a> { #call } } - }; - - let docs = if self.docs.is_empty() { - quote! {} - } else { - let docs = &self.docs; - quote! { - .docs(&[#(#docs),*]) - } - }; - - quote! { - ::ext_php_rs::builders::FunctionBuilder::new(#name, { - ::ext_php_rs::zend_fastcall! { - extern fn handler( - ex: &mut ::ext_php_rs::zend::ExecuteData, - retval: &mut ::ext_php_rs::types::Zval, - ) { - use ::ext_php_rs::convert::IntoZval; - - #(#arg_declarations)* - let result = { - #result - }; - - if let Err(e) = result.set_zval(retval, false) { - let e: ::ext_php_rs::exception::PhpException = e.into(); - e.throw().expect("Failed to throw PHP exception."); - } - } - } - handler - }) - #(.arg(#required_args))* - .not_required() - #(.arg(#not_required_args))* - #returns - #docs } } diff --git a/crates/macros/src/impl_.rs b/crates/macros/src/impl_.rs index 83303de29f..2de298da22 100644 --- a/crates/macros/src/impl_.rs +++ b/crates/macros/src/impl_.rs @@ -184,7 +184,7 @@ impl<'a> ParsedImpl<'a> { match items { syn::ImplItem::Const(c) => { let attr = PhpConstAttribute::from_attributes(&c.attrs)?; - let name = c.ident.rename(&self.rename_constants); + let name = c.ident.rename(self.rename_constants); let name = attr.rename.rename(name); let docs = get_docs(&attr.attrs)?; c.attrs.retain(|attr| !attr.path().is_ident("php")); @@ -197,7 +197,7 @@ impl<'a> ParsedImpl<'a> { } syn::ImplItem::Fn(method) => { let attr = PhpFunctionImplAttribute::from_attributes(&method.attrs)?; - let name = method.sig.ident.rename_method(&self.rename_methods); + let name = method.sig.ident.rename_method(self.rename_methods); let name = attr.rename.rename(name); let docs = get_docs(&attr.attrs)?; method.attrs.retain(|attr| !attr.path().is_ident("php")); @@ -309,7 +309,7 @@ impl quote::ToTokens for FnBuilder { Visibility::Private => quote! { ::ext_php_rs::flags::MethodFlags::Private }, }); for flag in &self.modifiers { - flags.push(quote! { #flag }) + flags.push(quote! { #flag }); } quote! { diff --git a/crates/macros/src/module.rs b/crates/macros/src/module.rs index ef1120d034..fb04998877 100644 --- a/crates/macros/src/module.rs +++ b/crates/macros/src/module.rs @@ -21,9 +21,10 @@ pub fn parser(args: TokenStream, input: ItemFn) -> Result { let ItemFn { sig, block, .. } = input; let Signature { output, inputs, .. } = sig; let stmts = &block.stmts; - let startup = match opts.startup { - Some(startup) => quote! { #startup(ty, mod_num) }, - None => quote! { 0i32 }, + let startup = if let Some(startup) = opts.startup { + quote! { #startup(ty, mod_num) } + } else { + quote! { 0i32 } }; Ok(quote! { diff --git a/crates/macros/src/parsing.rs b/crates/macros/src/parsing.rs index 6b8bdfd881..b3e1d21e65 100644 --- a/crates/macros/src/parsing.rs +++ b/crates/macros/src/parsing.rs @@ -31,11 +31,11 @@ pub enum Visibility { } pub trait Rename { - fn rename(&self, rule: &RenameRule) -> String; + fn rename(&self, rule: RenameRule) -> String; } pub trait MethodRename: Rename { - fn rename_method(&self, rule: &RenameRule) -> String; + fn rename_method(&self, rule: RenameRule) -> String; } #[derive(FromMeta, Debug, Default)] @@ -52,7 +52,7 @@ impl PhpRename { let name = name.as_ref(); self.rename .as_ref() - .map_or_else(|| name.to_string(), |r| name.rename(r)) + .map_or_else(|| name.to_string(), |r| name.rename(*r)) }, ToString::to_string, ) @@ -80,8 +80,8 @@ pub enum RenameRule { } impl RenameRule { - fn rename(&self, value: impl AsRef) -> String { - match *self { + fn rename(self, value: impl AsRef) -> String { + match self { Self::None => value.as_ref().to_string(), Self::Camel => ident_case::RenameRule::CamelCase.apply_to_field(value.as_ref()), Self::Pascal => ident_case::RenameRule::PascalCase.apply_to_field(value.as_ref()), @@ -94,35 +94,35 @@ impl RenameRule { } impl Rename for &str { - fn rename(&self, rule: &RenameRule) -> String { + fn rename(&self, rule: RenameRule) -> String { rule.rename(self) } } impl Rename for syn::Ident { - fn rename(&self, rule: &RenameRule) -> String { + fn rename(&self, rule: RenameRule) -> String { let s = self.to_string(); rule.rename(s) } } impl MethodRename for syn::Ident { - fn rename_method(&self, rule: &RenameRule) -> String { + fn rename_method(&self, rule: RenameRule) -> String { self.to_string().as_str().rename_method(rule) } } impl MethodRename for &str { - fn rename_method(&self, rule: &RenameRule) -> String { + fn rename_method(&self, rule: RenameRule) -> String { match rule { - RenameRule::None => self.to_string(), + RenameRule::None => (*self).to_string(), _ => { if MAGIC_METHOD.contains(self) { match *self { "__to_string" => "__toString".to_string(), "__debug_info" => "__debugInfo".to_string(), "__call_static" => "__callStatic".to_string(), - _ => self.to_string(), + _ => (*self).to_string(), } } else { self.rename(rule) @@ -136,7 +136,7 @@ impl MethodRename for &str { mod tests { use crate::parsing::{MethodRename, Rename}; - use super::{PhpRename, RenameRule, MAGIC_METHOD}; + use super::{PhpRename, RenameRule}; #[test] fn php_rename() { @@ -194,13 +194,13 @@ mod tests { ("__clone", "__clone"), ("__debug_info", "__debugInfo"), ] { - assert_eq!(magic, magic.rename_method(&RenameRule::None)); - assert_eq!(expected, magic.rename_method(&RenameRule::Camel)); - assert_eq!(expected, magic.rename_method(&RenameRule::Pascal)); - assert_eq!(expected, magic.rename_method(&RenameRule::Snake)); + assert_eq!(magic, magic.rename_method(RenameRule::None)); + assert_eq!(expected, magic.rename_method(RenameRule::Camel)); + assert_eq!(expected, magic.rename_method(RenameRule::Pascal)); + assert_eq!(expected, magic.rename_method(RenameRule::Snake)); assert_eq!( expected, - magic.rename_method(&RenameRule::ScreamingSnakeCase) + magic.rename_method(RenameRule::ScreamingSnakeCase) ); } } @@ -209,13 +209,13 @@ mod tests { fn rename_method() { let &(original, camel, snake, pascal, screaming_snake) = &("get_name", "getName", "get_name", "GetName", "GET_NAME"); - assert_eq!(original, original.rename_method(&RenameRule::None)); - assert_eq!(camel, original.rename_method(&RenameRule::Camel)); - assert_eq!(pascal, original.rename_method(&RenameRule::Pascal)); - assert_eq!(snake, original.rename_method(&RenameRule::Snake)); + assert_eq!(original, original.rename_method(RenameRule::None)); + assert_eq!(camel, original.rename_method(RenameRule::Camel)); + assert_eq!(pascal, original.rename_method(RenameRule::Pascal)); + assert_eq!(snake, original.rename_method(RenameRule::Snake)); assert_eq!( screaming_snake, - original.rename_method(&RenameRule::ScreamingSnakeCase) + original.rename_method(RenameRule::ScreamingSnakeCase) ); } @@ -223,13 +223,13 @@ mod tests { fn rename() { let &(original, camel, snake, pascal, screaming_snake) = &("get_name", "getName", "get_name", "GetName", "GET_NAME"); - assert_eq!(original, original.rename(&RenameRule::None)); - assert_eq!(camel, original.rename(&RenameRule::Camel)); - assert_eq!(pascal, original.rename(&RenameRule::Pascal)); - assert_eq!(snake, original.rename(&RenameRule::Snake)); + assert_eq!(original, original.rename(RenameRule::None)); + assert_eq!(camel, original.rename(RenameRule::Camel)); + assert_eq!(pascal, original.rename(RenameRule::Pascal)); + assert_eq!(snake, original.rename(RenameRule::Snake)); assert_eq!( screaming_snake, - original.rename(&RenameRule::ScreamingSnakeCase) + original.rename(RenameRule::ScreamingSnakeCase) ); } } diff --git a/crates/macros/src/syn_ext.rs b/crates/macros/src/syn_ext.rs index 226a8645ae..a5b1ea56ba 100644 --- a/crates/macros/src/syn_ext.rs +++ b/crates/macros/src/syn_ext.rs @@ -27,14 +27,16 @@ impl DropLifetimes for syn::Type { impl DropLifetimes for syn::TypeArray { fn drop_lifetimes(&mut self) { - self.elem.drop_lifetimes() + self.elem.drop_lifetimes(); } } impl DropLifetimes for syn::TypeBareFn { fn drop_lifetimes(&mut self) { self.lifetimes = None; - self.inputs.iter_mut().for_each(|i| i.drop_lifetimes()); + self.inputs + .iter_mut() + .for_each(DropLifetimes::drop_lifetimes); self.output.drop_lifetimes(); } } @@ -55,7 +57,7 @@ impl DropLifetimes for syn::ReturnType { impl DropLifetimes for syn::TypeGroup { fn drop_lifetimes(&mut self) { - self.elem.drop_lifetimes() + self.elem.drop_lifetimes(); } } @@ -89,7 +91,9 @@ impl DropLifetimes for syn::TraitBound { impl DropLifetimes for syn::Path { fn drop_lifetimes(&mut self) { - self.segments.iter_mut().for_each(|i| i.drop_lifetimes()); + self.segments + .iter_mut() + .for_each(DropLifetimes::drop_lifetimes); } } @@ -103,10 +107,9 @@ impl DropLifetimes for syn::PathSegment { syn::GenericArgument::Type(t) => t.drop_lifetimes(), syn::GenericArgument::AssocType(t) => t.drop_lifetimes(), syn::GenericArgument::Constraint(t) => t.drop_lifetimes(), - syn::GenericArgument::Const(_) => {} - syn::GenericArgument::AssocConst(_) => {} + syn::GenericArgument::Const(_) | syn::GenericArgument::AssocConst(_) => {} _ => return None, - }; + } Some(i) }) .collect(); @@ -168,7 +171,9 @@ impl DropLifetimes for syn::TypeTraitObject { impl DropLifetimes for syn::TypeTuple { fn drop_lifetimes(&mut self) { - self.elems.iter_mut().for_each(|i| i.drop_lifetimes()); + self.elems + .iter_mut() + .for_each(DropLifetimes::drop_lifetimes); } } diff --git a/crates/macros/src/zval.rs b/crates/macros/src/zval.rs index 96c96bf8be..7f95ef027e 100644 --- a/crates/macros/src/zval.rs +++ b/crates/macros/src/zval.rs @@ -2,8 +2,8 @@ use darling::ToTokens; use proc_macro2::{Span, TokenStream}; use quote::quote; use syn::{ - token::Where, DataEnum, DataStruct, DeriveInput, GenericParam, Generics, Ident, ImplGenerics, - Lifetime, LifetimeParam, TypeGenerics, Variant, WhereClause, + punctuated::Punctuated, token::Where, DataEnum, DataStruct, DeriveInput, GenericParam, + Generics, Ident, ImplGenerics, Lifetime, LifetimeParam, TypeGenerics, Variant, WhereClause, }; use crate::prelude::*; @@ -19,7 +19,7 @@ pub fn parser(input: DeriveInput) -> Result { where_token: Where { span: Span::call_site(), }, - predicates: Default::default(), + predicates: Punctuated::default(), }); let mut from_where_clause = into_where_clause.clone(); @@ -38,7 +38,7 @@ pub fn parser(input: DeriveInput) -> Result { parsed }; - for generic in generics.params.iter() { + for generic in &generics.params { match generic { GenericParam::Type(ty) => { let ident = &ty.ident; @@ -61,43 +61,43 @@ pub fn parser(input: DeriveInput) -> Result { }) .expect("couldn't parse where predicate"), ), - _ => continue, + GenericParam::Const(_) => {} } } match input.data { syn::Data::Struct(data) => parse_struct( - data, - ident, - into_impl_generics, - from_impl_generics, - into_where_clause, - from_where_clause, - ty_generics, + &data, + &ident, + &into_impl_generics, + &from_impl_generics, + &into_where_clause, + &from_where_clause, + &ty_generics, ), syn::Data::Enum(data) => parse_enum( - data, - ident, - into_impl_generics, - from_impl_generics, - into_where_clause, - from_where_clause, - ty_generics, + &data, + &ident, + &into_impl_generics, + &from_impl_generics, + &into_where_clause, + &from_where_clause, + &ty_generics, ), - _ => { + syn::Data::Union(_) => { bail!(ident.span() => "Only structs and enums are supported by the `#[derive(ZvalConvert)]` macro.") } } } fn parse_struct( - data: DataStruct, - ident: Ident, - into_impl_generics: ImplGenerics, - from_impl_generics: Generics, - into_where_clause: WhereClause, - from_where_clause: WhereClause, - ty_generics: TypeGenerics, + data: &DataStruct, + ident: &Ident, + into_impl_generics: &ImplGenerics, + from_impl_generics: &Generics, + into_where_clause: &WhereClause, + from_where_clause: &WhereClause, + ty_generics: &TypeGenerics, ) -> Result { let into_fields = data .fields @@ -176,13 +176,13 @@ fn parse_struct( } fn parse_enum( - data: DataEnum, - ident: Ident, - into_impl_generics: ImplGenerics, - from_impl_generics: Generics, - into_where_clause: WhereClause, - from_where_clause: WhereClause, - ty_generics: TypeGenerics, + data: &DataEnum, + ident: &Ident, + into_impl_generics: &ImplGenerics, + from_impl_generics: &Generics, + into_where_clause: &WhereClause, + from_where_clause: &WhereClause, + ty_generics: &TypeGenerics, ) -> Result { let into_variants = data.variants.iter().filter_map(|variant| { // can have default fields - in this case, return `null`. @@ -228,7 +228,7 @@ fn parse_enum( }); Ok(None) } - _ => bail!(fields => "Enum variants must be unnamed and have only one field inside the variant when using `#[derive(ZvalConvert)]`.") + syn::Fields::Named(_) => bail!(fields => "Enum variants must be unnamed and have only one field inside the variant when using `#[derive(ZvalConvert)]`.") } }).collect::>>()?; let default = default.unwrap_or_else(|| quote! { None }); diff --git a/examples/hello_world.rs b/examples/hello_world.rs index 6e513a3982..3b38cc684e 100644 --- a/examples/hello_world.rs +++ b/examples/hello_world.rs @@ -1,4 +1,4 @@ -#![allow(missing_docs)] +#![allow(missing_docs, clippy::must_use_candidate)] #![cfg_attr(windows, feature(abi_vectorcall))] use ext_php_rs::{constant::IntoConst, prelude::*, types::ZendClassObject}; @@ -26,10 +26,10 @@ impl TestClass { #[php(defaults(a = 5, test = 100))] pub fn test_camel_case(&self, a: i32, test: i32) { - println!("a: {} test: {}", a, test); + println!("a: {a} test: {test}"); } - fn x(&self) -> i32 { + fn x() -> i32 { 5 } diff --git a/guide/src/ini-settings.md b/guide/src/ini-settings.md index f56d72c947..7a572d9965 100644 --- a/guide/src/ini-settings.md +++ b/guide/src/ini-settings.md @@ -19,7 +19,7 @@ pub fn startup(ty: i32, mod_num: i32) -> i32 { IniEntryDef::new( "my_extension.display_emoji".to_owned(), "yes".to_owned(), - IniEntryPermission::All, + &IniEntryPermission::All, ), ]; IniEntryDef::register(ini_entries, mod_num); diff --git a/src/alloc.rs b/src/alloc.rs index 3bad5ee954..47517e1f17 100644 --- a/src/alloc.rs +++ b/src/alloc.rs @@ -1,6 +1,8 @@ //! Functions relating to the Zend Memory Manager, used to allocate //! request-bound memory. +use cfg_if::cfg_if; + use crate::ffi::{_efree, _emalloc}; use std::{alloc::Layout, ffi::c_void}; @@ -13,20 +15,23 @@ use std::{alloc::Layout, ffi::c_void}; /// # Returns /// /// A pointer to the memory allocated. +#[must_use] pub fn emalloc(layout: Layout) -> *mut u8 { // TODO account for alignment let size = layout.size(); (unsafe { - #[cfg(php_debug)] - { - _emalloc(size as _, std::ptr::null_mut(), 0, std::ptr::null_mut(), 0) - } - #[cfg(not(php_debug))] - { - _emalloc(size as _) + cfg_if! { + if #[cfg(php_debug)] { + #[allow(clippy::used_underscore_items)] + _emalloc(size as _, std::ptr::null_mut(), 0, std::ptr::null_mut(), 0) + } else { + #[allow(clippy::used_underscore_items)] + _emalloc(size as _) + } } - }) as *mut u8 + }) + .cast::() } /// Frees a given memory pointer which was allocated through the PHP memory @@ -41,18 +46,19 @@ pub fn emalloc(layout: Layout) -> *mut u8 { /// Caller must guarantee that the given pointer is valid (aligned and non-null) /// and was originally allocated through the Zend memory manager. pub unsafe fn efree(ptr: *mut u8) { - #[cfg(php_debug)] - { - _efree( - ptr as *mut c_void, - std::ptr::null_mut(), - 0, - std::ptr::null_mut(), - 0, - ) - } - #[cfg(not(php_debug))] - { - _efree(ptr as *mut c_void) + cfg_if! { + if #[cfg(php_debug)] { + #[allow(clippy::used_underscore_items)] + _efree( + ptr.cast::(), + std::ptr::null_mut(), + 0, + std::ptr::null_mut(), + 0, + ) + } else { + #[allow(clippy::used_underscore_items)] + _efree(ptr.cast::()); + } } } diff --git a/src/args.rs b/src/args.rs index 6c04724fc1..211c8937de 100644 --- a/src/args.rs +++ b/src/args.rs @@ -19,10 +19,11 @@ use crate::{ }; /// Represents an argument to a function. +#[must_use] #[derive(Debug)] pub struct Arg<'a> { name: String, - _type: DataType, + r#type: DataType, as_ref: bool, allow_null: bool, pub(crate) variadic: bool, @@ -38,10 +39,10 @@ impl<'a> Arg<'a> { /// /// * `name` - The name of the parameter. /// * `_type` - The type of the parameter. - pub fn new>(name: T, _type: DataType) -> Self { + pub fn new>(name: T, r#type: DataType) -> Self { Arg { name: name.into(), - _type, + r#type, as_ref: false, allow_null: false, variadic: false, @@ -79,11 +80,13 @@ impl<'a> Arg<'a> { /// Attempts to consume the argument, converting the inner type into `T`. /// Upon success, the result is returned in a [`Result`]. /// - /// If the conversion fails (or the argument contains no value), the - /// argument is returned in an [`Err`] variant. - /// /// As this function consumes, it cannot return a reference to the /// underlying zval. + /// + /// # Errors + /// + /// If the conversion fails (or the argument contains no value), the + /// argument is returned in an [`Err`] variant. pub fn consume(mut self) -> Result where for<'b> T: FromZvalMut<'b>, @@ -95,7 +98,7 @@ impl<'a> Arg<'a> { } /// Attempts to retrieve the value of the argument. - /// This will be None until the ArgParser is used to parse + /// This will be None until the [`ArgParser`] is used to parse /// the arguments. pub fn val(&'a mut self) -> Option where @@ -124,6 +127,8 @@ impl<'a> Arg<'a> { /// /// * `Some(&Zval)` - The internal zval. /// * `None` - The argument was empty. + // TODO: Figure out if we can change this + #[allow(clippy::mut_mut)] pub fn zval(&mut self) -> Option<&mut &'a mut Zval> { self.zval.as_mut() } @@ -140,6 +145,12 @@ impl<'a> Arg<'a> { /// # Parameters /// /// * `params` - A list of parameters to call the function with. + /// + /// # Errors + /// + /// * `Error::Callable` - The argument is not callable. + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] pub fn try_call(&self, params: Vec<&dyn IntoZvalDyn>) -> Result { self.zval.as_ref().ok_or(Error::Callable)?.try_call(params) @@ -150,7 +161,7 @@ impl<'a> Arg<'a> { Ok(ArgInfo { name: CString::new(self.name.as_str())?.into_raw(), type_: ZendType::empty_from_type( - self._type, + self.r#type, self.as_ref, self.variadic, self.allow_null, @@ -166,7 +177,7 @@ impl<'a> Arg<'a> { impl From> for _zend_expected_type { fn from(arg: Arg) -> Self { - let err = match arg._type { + let err = match arg.r#type { DataType::False | DataType::True => _zend_expected_type_Z_EXPECTED_BOOL, DataType::Long => _zend_expected_type_Z_EXPECTED_LONG, DataType::Double => _zend_expected_type_Z_EXPECTED_DOUBLE, @@ -189,7 +200,7 @@ impl From> for Parameter { fn from(val: Arg<'_>) -> Self { Parameter { name: val.name.into(), - ty: Some(val._type).into(), + ty: Some(val.r#type).into(), nullable: val.allow_null, default: val.default_value.map(abi::RString::from).into(), } @@ -200,6 +211,7 @@ impl From> for Parameter { pub type ArgInfo = zend_internal_arg_info; /// Parses the arguments of a function. +#[must_use] pub struct ArgParser<'a, 'b> { args: Vec<&'b mut Arg<'a>>, min_num_args: Option, @@ -248,6 +260,9 @@ impl<'a, 'b> ArgParser<'a, 'b> { /// Returns an [`Error`] type if there were too many or too little arguments /// passed to the function. The user has already been notified so you /// should break execution after seeing an error type. + /// + /// Also returns an error if the number of min/max arguments exceeds + /// `u32::MAX` pub fn parse(mut self) -> Result<()> { let max_num_args = self.args.len(); let mut min_num_args = self.min_num_args.unwrap_or(max_num_args); @@ -260,7 +275,12 @@ impl<'a, 'b> ArgParser<'a, 'b> { 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 // are copied. - unsafe { zend_wrong_parameters_count_error(min_num_args as _, max_num_args as _) }; + unsafe { + zend_wrong_parameters_count_error( + min_num_args.try_into()?, + max_num_args.try_into()?, + ); + }; return Err(Error::IncorrectArguments(num_args, min_num_args)); } diff --git a/src/binary.rs b/src/binary.rs index decb0e9703..1fd77688a4 100644 --- a/src/binary.rs +++ b/src/binary.rs @@ -154,16 +154,22 @@ macro_rules! pack_impl { } fn unpack_into(s: &zend_string) -> Vec { + #[allow(clippy::cast_lossless)] let bytes = ($d / 8) as u64; let len = (s.len as u64) / bytes; - let mut result = Vec::with_capacity(len as _); - let ptr = s.val.as_ptr() as *const $t; + let mut result = + Vec::with_capacity(len.try_into().expect("Capacity integer overflow")); + // TODO: Check alignment + #[allow(clippy::cast_ptr_alignment)] + let ptr = s.val.as_ptr().cast::<$t>(); // SAFETY: We calculate the length of memory that we can legally read based on // the side of the type, therefore we never read outside the memory we // should. for i in 0..len { - result.push(unsafe { *ptr.offset(i as _) }); + result.push(unsafe { + *ptr.offset(i.try_into().expect("Offset integer overflow")) + }); } result diff --git a/src/binary_slice.rs b/src/binary_slice.rs index 223f480025..9c62f1b6d5 100644 --- a/src/binary_slice.rs +++ b/src/binary_slice.rs @@ -115,7 +115,9 @@ macro_rules! pack_slice_impl { fn unpack_into(s: &zend_string) -> &[Self] { let bytes = ($d / 8) as usize; let len = (s.len as usize) / bytes; - let ptr = s.val.as_ptr() as *const $t; + // TODO: alignment needs fixing? + #[allow(clippy::cast_ptr_alignment)] + let ptr = s.val.as_ptr().cast::<$t>(); unsafe { from_raw_parts(ptr, len) } } } diff --git a/src/boxed.rs b/src/boxed.rs index fb61edc7af..5b2e1dac11 100644 --- a/src/boxed.rs +++ b/src/boxed.rs @@ -26,7 +26,7 @@ use std::{ fmt::Debug, mem::ManuallyDrop, ops::{Deref, DerefMut}, - ptr::NonNull, + ptr::{self, NonNull}, }; use super::alloc::efree; @@ -59,6 +59,7 @@ impl ZBox { /// /// The caller is responsible for managing the memory pointed to by the /// returned pointer, including freeing the memory. + #[must_use] pub fn into_raw(self) -> &'static mut T { let mut this = ManuallyDrop::new(self); // SAFETY: All constructors ensure the contained pointer is well-aligned and @@ -70,7 +71,7 @@ impl ZBox { impl Drop for ZBox { #[inline] fn drop(&mut self) { - self.deref_mut().free() + self.deref_mut().free(); } } @@ -133,6 +134,6 @@ pub unsafe trait ZBoxable { /// Frees the memory pointed to by `self`, calling any destructors required /// in the process. fn free(&mut self) { - unsafe { efree(self as *mut _ as *mut u8) }; + unsafe { efree(ptr::from_mut(self).cast::()) }; } } diff --git a/src/builders/class.rs b/src/builders/class.rs index c717b426d9..746c0bc1dc 100644 --- a/src/builders/class.rs +++ b/src/builders/class.rs @@ -1,4 +1,4 @@ -use std::{ffi::CString, mem::MaybeUninit, rc::Rc}; +use std::{ffi::CString, mem::MaybeUninit, ptr, rc::Rc}; use crate::{ builders::FunctionBuilder, @@ -20,6 +20,7 @@ use crate::{ type ConstantEntry = (String, Box Result>, DocComments); /// Builder for registering a class in PHP. +#[must_use] pub struct ClassBuilder { pub(crate) name: String, ce: ClassEntry, @@ -132,6 +133,10 @@ impl ClassBuilder { /// * `name` - The name of the constant to add to the class. /// * `value` - The value of the constant. /// * `docs` - Documentation comments for the constant. + /// + /// # Errors + /// + /// TODO: Never? pub fn constant>( mut self, name: T, @@ -154,6 +159,10 @@ impl ClassBuilder { /// * `name` - The name of the constant to add to the class. /// * `value` - The value of the constant. /// * `docs` - Documentation comments for the constant. + /// + /// # Errors + /// + /// TODO: Never? pub fn dyn_constant>( mut self, name: T, @@ -199,14 +208,11 @@ impl ClassBuilder { zend_fastcall! { extern fn constructor(ex: &mut ExecuteData, _: &mut Zval) { - let ConstructorMeta { constructor, .. } = match T::constructor() { - Some(c) => c, - None => { - PhpException::default("You cannot instantiate this class from PHP.".into()) - .throw() - .expect("Failed to throw exception when constructing class"); - return; - } + let Some(ConstructorMeta { constructor, .. }) = T::constructor() else { + PhpException::default("You cannot instantiate this class from PHP.".into()) + .throw() + .expect("Failed to throw exception when constructing class"); + return; }; let this = match constructor(ex) { @@ -218,15 +224,14 @@ impl ClassBuilder { } ConstructorResult::ArgError => return, }; - let this_obj = match ex.get_object::() { - Some(obj) => obj, - None => { - PhpException::default("Failed to retrieve reference to `this` object.".into()) - .throw() - .expect("Failed to throw exception while constructing class"); - return; - } + + let Some(this_obj) = ex.get_object::() else { + PhpException::default("Failed to retrieve reference to `this` object.".into()) + .throw() + .expect("Failed to throw exception while constructing class"); + return; }; + this_obj.initialize(this); } } @@ -274,7 +279,14 @@ impl ClassBuilder { /// /// # Errors /// - /// Returns an [`Error`] variant if the class could not be registered. + /// * [`Error::InvalidPointer`] - If the class could not be registered. + /// * [`Error::InvalidCString`] - If the class name is not a valid C string. + /// * [`Error::IntegerOverflow`] - If the property flags are not valid. + /// * If a method or property could not be built. + /// + /// # Panics + /// + /// If no registration function was provided. pub fn register(mut self) -> Result<()> { self.ce.name = ZendStr::new_interned(&self.name, true).into_raw(); @@ -297,7 +309,7 @@ impl ClassBuilder { zend_register_internal_class_ex( &mut self.ce, match self.extends { - Some(ptr) => (ptr as *const _) as *mut _, + Some(ptr) => ptr::from_ref(ptr).cast_mut(), None => std::ptr::null_mut(), }, ) @@ -318,13 +330,7 @@ impl ClassBuilder { } for iface in self.interfaces { - unsafe { - zend_do_implement_interface( - class, - iface as *const crate::ffi::_zend_class_entry - as *mut crate::ffi::_zend_class_entry, - ) - }; + unsafe { zend_do_implement_interface(class, ptr::from_ref(iface).cast_mut()) }; } for (name, flags, _) in self.properties { @@ -334,7 +340,7 @@ impl ClassBuilder { CString::new(name.as_str())?.as_ptr(), name.len() as _, &mut Zval::new(), - flags.bits() as _, + flags.bits().try_into()?, ); } } @@ -347,7 +353,7 @@ impl ClassBuilder { CString::new(name.as_str())?.as_ptr(), name.len(), value, - ) + ); }; } diff --git a/src/builders/function.rs b/src/builders/function.rs index 6a4f402dd0..8839a1d1a5 100644 --- a/src/builders/function.rs +++ b/src/builders/function.rs @@ -23,6 +23,7 @@ type FunctionPointerHandler = extern "vectorcall" fn(execute_data: *mut ExecuteData, retval: *mut Zval); /// Builder for registering a function in PHP. +#[must_use] #[derive(Debug)] pub struct FunctionBuilder<'a> { pub(crate) name: String, @@ -155,6 +156,14 @@ impl<'a> FunctionBuilder<'a> { /// Builds the function converting it into a Zend function entry. /// /// Returns a result containing the function entry if successful. + /// + /// # Errors + /// + /// * `Error::InvalidCString` - If the function name is not a valid C + /// string. + /// * `Error::IntegerOverflow` - If the number of arguments is too large. + /// * If arg info for an argument could not be created. + /// * If the function name contains NUL bytes. pub fn build(mut self) -> Result { let mut args = Vec::with_capacity(self.args.len() + 1); let mut n_req = self.n_req.unwrap_or(self.args.len()); @@ -186,12 +195,12 @@ impl<'a> FunctionBuilder<'a> { args.extend( self.args .iter() - .map(|arg| arg.as_arg_info()) + .map(Arg::as_arg_info) .collect::>>()?, ); self.function.fname = CString::new(self.name)?.into_raw(); - self.function.num_args = (args.len() - 1) as u32; + self.function.num_args = (args.len() - 1).try_into()?; self.function.arg_info = Box::into_raw(args.into_boxed_slice()) as *const ArgInfo; Ok(self.function) diff --git a/src/builders/module.rs b/src/builders/module.rs index f78630be09..78721f3a44 100644 --- a/src/builders/module.rs +++ b/src/builders/module.rs @@ -38,6 +38,7 @@ use crate::{ /// entry.into_raw() /// } /// ``` +#[must_use] #[derive(Debug, Default)] pub struct ModuleBuilder<'a> { pub(crate) name: String, @@ -150,8 +151,8 @@ impl ModuleBuilder<'_> { /// /// # Arguments /// - /// * `const` - Tuple containing the name, value and doc comments for the constant. This is - /// a tuple to support the [`wrap_constant`] macro. + /// * `const` - Tuple containing the name, value and doc comments for the + /// constant. This is a tuple to support the [`wrap_constant`] macro. /// /// [`wrap_constant`]: crate::wrap_constant pub fn constant( @@ -168,6 +169,10 @@ impl ModuleBuilder<'_> { } /// Adds a class to the extension. + /// + /// # Panics + /// + /// * Panics if a constant could not be registered. pub fn class(mut self) -> Self { self.classes.push(|| { let mut builder = ClassBuilder::new(T::CLASS_NAME); @@ -213,6 +218,14 @@ pub struct ModuleStartup { impl ModuleStartup { /// Completes startup of the module. Should only be called inside the module /// startup function. + /// + /// # Errors + /// + /// * Returns an error if a constant could not be registered. + /// + /// # Panics + /// + /// * Panics if a class could not be registered. pub fn startup(self, _ty: i32, mod_num: i32) -> Result<()> { for (name, val) in self.constants { val.register_constant(&name, mod_num)?; @@ -240,7 +253,7 @@ impl TryFrom> for (ModuleEntry, ModuleStartup) { let mut functions = builder .functions .into_iter() - .map(|f| f.build()) + .map(FunctionBuilder::build) .collect::>>()?; functions.push(FunctionEntry::end()); let functions = Box::into_raw(functions.into_boxed_slice()) as *const FunctionEntry; @@ -259,7 +272,7 @@ impl TryFrom> for (ModuleEntry, ModuleStartup) { Ok(( ModuleEntry { - size: mem::size_of::() as u16, + size: mem::size_of::().try_into()?, zend_api: ZEND_MODULE_API_NO, zend_debug: u8::from(PHP_DEBUG), zts: u8::from(PHP_ZTS), diff --git a/src/builders/sapi.rs b/src/builders/sapi.rs index 4916d3d0db..0bbf1605c5 100644 --- a/src/builders/sapi.rs +++ b/src/builders/sapi.rs @@ -4,6 +4,8 @@ use crate::{embed::SapiModule, error::Result}; use std::ffi::{c_char, c_void}; use std::{ffi::CString, ptr}; +/// Builder for `SapiModule`s +#[must_use] pub struct SapiBuilder { name: String, pretty_name: String, @@ -11,6 +13,7 @@ pub struct SapiBuilder { } impl SapiBuilder { + /// Creates a new [`SapiBuilder`] instance pub fn new, U: Into>(name: T, pretty_name: U) -> Self { Self { name: name.into(), @@ -56,6 +59,7 @@ impl SapiBuilder { } } + /// Sets the `ub_write` function for this SAPI pub fn ub_write_function(mut self, func: SapiUbWriteFunc) -> Self { self.module.ub_write = Some(func); self @@ -74,6 +78,10 @@ impl SapiBuilder { /// Builds the extension and returns a `SapiModule`. /// /// Returns a result containing the sapi module if successful. + /// + /// # Errors + /// + /// * If name or property name contain null bytes pub fn build(mut self) -> Result { self.module.name = CString::new(self.name)?.into_raw(); self.module.pretty_name = CString::new(self.pretty_name)?.into_raw(); diff --git a/src/class.rs b/src/class.rs index 67ead7d80f..ad7571ed31 100644 --- a/src/class.rs +++ b/src/class.rs @@ -123,6 +123,7 @@ pub struct ClassMetadata { impl ClassMetadata { /// Creates a new class metadata instance. + #[must_use] pub const fn new() -> Self { Self { handlers: OnceCell::new(), diff --git a/src/closure.rs b/src/closure.rs index 0bba5e74ce..b50205ae9d 100644 --- a/src/closure.rs +++ b/src/closure.rs @@ -119,9 +119,7 @@ impl Closure { /// /// Panics if the function is called more than once. pub fn build() { - if CLOSURE_META.has_ce() { - panic!("Closure has already been built."); - } + assert!(!CLOSURE_META.has_ce(), "Closure class already built."); ClassBuilder::new("RustClosure") .method( @@ -143,7 +141,7 @@ impl Closure { let (parser, this) = ex.parser_method::(); let this = this.expect("Internal closure function called on non-closure class"); - this.0.invoke(parser, ret) + this.0.invoke(parser, ret); } } } @@ -213,7 +211,7 @@ where { fn invoke(&mut self, _: ArgParser, ret: &mut Zval) { if let Err(e) = self().set_zval(ret, false) { - let _ = PhpException::default(format!("Failed to return closure result to PHP: {}", e)) + let _ = PhpException::default(format!("Failed to return closure result to PHP: {e}")) .throw(); } } @@ -225,7 +223,7 @@ where { fn invoke(&mut self, _: ArgParser, ret: &mut Zval) { if let Err(e) = self().set_zval(ret, false) { - let _ = PhpException::default(format!("Failed to return closure result to PHP: {}", e)) + let _ = PhpException::default(format!("Failed to return closure result to PHP: {e}")) .throw(); } } @@ -239,15 +237,12 @@ where let mut this = Some(self); Closure::wrap(Box::new(move || { - let this = match this.take() { - Some(this) => this, - None => { - let _ = PhpException::default( - "Attempted to call `FnOnce` closure more than once.".into(), - ) - .throw(); - return Option::::None; - } + let Some(this) = this.take() else { + let _ = PhpException::default( + "Attempted to call `FnOnce` closure more than once.".into(), + ) + .throw(); + return Option::::None; }; Some(this()) @@ -269,15 +264,12 @@ macro_rules! php_closure_impl { let mut this = Some(self); Closure::wrap(Box::new(move |$($gen),*| { - let this = match this.take() { - Some(this) => this, - None => { - let _ = PhpException::default( - "Attempted to call `FnOnce` closure more than once.".into(), - ) - .throw(); - return Option::::None; - } + let Some(this) = this.take() else { + let _ = PhpException::default( + "Attempted to call `FnOnce` closure more than once.".into(), + ) + .throw(); + return Option::::None; }; Some(this($($gen),*)) diff --git a/src/constant.rs b/src/constant.rs index 7a05774484..7e2aac1fcd 100644 --- a/src/constant.rs +++ b/src/constant.rs @@ -28,6 +28,10 @@ pub trait IntoConst: Debug { /// * `module_number` - The module number that we are registering the /// constant under. /// + /// # Errors + /// + /// Returns an error if the constant could not be registered. + /// /// # Examples /// /// ```no_run @@ -61,6 +65,10 @@ pub trait IntoConst: Debug { /// constant under. /// * `flags` - Flags to register the constant with. /// + /// # Errors + /// + /// Returns an error if the constant flags could not be registered. + /// /// # Examples /// /// ```no_run @@ -103,9 +111,9 @@ impl IntoConst for &str { CString::new(name)?.as_ptr(), name.len() as _, CString::new(*self)?.as_ptr(), - flags.bits() as _, + flags.bits().try_into()?, module_number, - ) + ); }; Ok(()) } @@ -123,9 +131,9 @@ impl IntoConst for bool { CString::new(name)?.as_ptr(), name.len() as _, *self, - flags.bits() as _, + flags.bits().try_into()?, module_number, - ) + ); }; Ok(()) } @@ -146,8 +154,8 @@ macro_rules! into_const_num { $fn( CString::new(name)?.as_ptr(), name.len() as _, - *self as _, - flags.bits() as _, + (*self).into(), + flags.bits().try_into()?, module_number, ) }) diff --git a/src/convert.rs b/src/convert.rs index a48d2d86b5..69aa75f2f6 100644 --- a/src/convert.rs +++ b/src/convert.rs @@ -79,6 +79,13 @@ where /// type. pub trait FromZendObject<'a>: Sized { /// Extracts `Self` from the source `ZendObject`. + /// + /// # Errors + /// + /// If the conversion fails, an [`Error`] is returned. + /// + /// [`Error`]: crate::error::Error + // TODO: Expand on error information fn from_zend_object(obj: &'a ZendObject) -> Result; } @@ -89,6 +96,13 @@ pub trait FromZendObject<'a>: Sized { /// any types that also implement [`FromZendObject`]. pub trait FromZendObjectMut<'a>: Sized { /// Extracts `Self` from the source `ZendObject`. + /// + /// # Errors + /// + /// If the conversion fails, an [`Error`] is returned. + /// + /// [`Error`]: crate::error::Error + // TODO: Expand on error information fn from_zend_object_mut(obj: &'a mut ZendObject) -> Result; } @@ -106,6 +120,13 @@ where /// the implementation to determine the type of object which is produced. pub trait IntoZendObject { /// Attempts to convert `self` into a Zend object. + /// + /// # Errors + /// + /// If the conversion fails, an [`Error`] is returned. + /// + /// [`Error`]: crate::error::Error + // TODO: Expand on error information fn into_zend_object(self) -> Result>; } @@ -129,6 +150,13 @@ pub trait IntoZval: Sized { /// /// * `persistent` - Whether the contents of the Zval will persist between /// requests. + /// + /// # Errors + /// + /// If the conversion fails, an [`Error`] is returned. + /// + /// [`Error`]: crate::error::Error + // TODO: Expand on error information fn into_zval(self, persistent: bool) -> Result { let mut zval = Zval::new(); self.set_zval(&mut zval, persistent)?; @@ -143,6 +171,13 @@ pub trait IntoZval: Sized { /// * `zv` - The Zval to set the content of. /// * `persistent` - Whether the contents of the Zval will persist between /// requests. + /// + /// # Errors + /// + /// If setting the content fails, an [`Error`] is returned. + /// + /// [`Error`]: crate::error::Error + // TODO: Expand on error information fn set_zval(self, zv: &mut Zval, persistent: bool) -> Result<()>; } @@ -166,12 +201,11 @@ where #[inline] fn set_zval(self, zv: &mut Zval, persistent: bool) -> Result<()> { - match self { - Some(val) => val.set_zval(zv, persistent), - None => { - zv.set_null(); - Ok(()) - } + if let Some(val) = self { + val.set_zval(zv, persistent) + } else { + zv.set_null(); + Ok(()) } } } @@ -209,6 +243,12 @@ pub trait IntoZvalDyn { /// /// * `persistent` - Whether the contents of the Zval will persist between /// requests. + /// + /// # Errors + /// + /// If the conversion fails, an [`Error`] is returned. + /// + /// [`Error`]: crate::error::Error fn as_zval(&self, persistent: bool) -> Result; /// Returns the PHP type of the type. diff --git a/src/describe/abi.rs b/src/describe/abi.rs index 4f2da62aad..dcf6519b1b 100644 --- a/src/describe/abi.rs +++ b/src/describe/abi.rs @@ -42,7 +42,7 @@ impl From> for Vec { fn from(vec: StdVec) -> Self { let vec = vec.into_boxed_slice(); let len = vec.len(); - let ptr = Box::into_raw(vec) as *mut T; + let ptr = Box::into_raw(vec).cast::(); Self { ptr, len } } @@ -60,6 +60,7 @@ impl Str { /// /// The lifetime is `'static` and can outlive the [`Str`] object, as you can /// only initialize a [`Str`] through a static reference. + #[must_use] pub fn str(&self) -> &'static str { unsafe { std::str::from_utf8_unchecked(std::slice::from_raw_parts(self.ptr, self.len)) } } @@ -93,6 +94,11 @@ pub struct RString { impl RString { /// Returns the string as a string slice. + /// + /// # Panics + /// + /// * If the string is not valid UTF-8 + #[must_use] pub fn as_str(&self) -> &str { std::str::from_utf8(&self.inner).expect("RString value is not valid UTF-8") } diff --git a/src/describe/mod.rs b/src/describe/mod.rs index 99f0056944..f0971fb53d 100644 --- a/src/describe/mod.rs +++ b/src/describe/mod.rs @@ -9,7 +9,7 @@ use crate::{ flags::{DataType, MethodFlags, PropertyFlags}, prelude::ModuleBuilder, }; -use abi::*; +use abi::{Option, RString, Str, Vec}; pub mod abi; mod stub; @@ -34,6 +34,7 @@ impl Description { /// # Parameters /// /// * `module` - The extension module representation. + #[must_use] pub fn new(module: Module) -> Self { Self { module, @@ -162,7 +163,8 @@ pub struct Class { pub docs: DocBlock, /// Name of the class the exported class extends. (Not implemented #326) pub extends: Option, - /// Names of the interfaces the exported class implements. (Not implemented #326) + /// Names of the interfaces the exported class implements. (Not implemented + /// #326) pub implements: Vec, /// Properties of the class. pub properties: Vec, @@ -239,7 +241,7 @@ impl From<(String, PropertyFlags, DocComments)> for Property { // TODO: Implement nullable #376 let nullable = false; let docs = docs.into(); - println!("Property: {:?}", name); + println!("Property: {name:?}"); Self { name: name.into(), docs, @@ -266,7 +268,7 @@ pub struct Method { /// Return value of the method. pub retval: Option, /// Whether the method is static. - pub _static: bool, + pub r#static: bool, /// Visibility of the method. pub visibility: Visibility, } @@ -295,11 +297,11 @@ impl From<(FunctionBuilder<'_>, MethodFlags)> for Method { params: builder .args .into_iter() - .map(|a| a.into()) + .map(Into::into) .collect::>() .into(), ty: flags.into(), - _static: flags.contains(MethodFlags::Static), + r#static: flags.contains(MethodFlags::Static), visibility: flags.into(), } } diff --git a/src/describe/stub.rs b/src/describe/stub.rs index 46daeb5596..58c7428743 100644 --- a/src/describe/stub.rs +++ b/src/describe/stub.rs @@ -4,7 +4,8 @@ use crate::flags::DataType; use std::{cmp::Ordering, collections::HashMap}; use super::{ - abi::*, Class, Constant, DocBlock, Function, Method, MethodType, Module, Parameter, Property, + abi::{Option, RString}, + Class, Constant, DocBlock, Function, Method, MethodType, Module, Parameter, Property, Visibility, }; use std::fmt::{Error as FmtError, Result as FmtResult, Write}; @@ -17,8 +18,11 @@ pub trait ToStub { /// /// # Returns /// - /// Returns a string on success. Returns an error if there was an error - /// writing into the string. + /// Returns a string on success. + /// + /// # Errors + /// + /// Returns an error if there was an error writing into the string. fn to_stub(&self) -> Result { let mut buf = String::new(); self.fmt_stub(&mut buf)?; @@ -33,8 +37,11 @@ pub trait ToStub { /// /// # Returns /// - /// Returns nothing on success. Returns an error if there was an error - /// writing into the buffer. + /// Returns nothing on success. + /// + /// # Errors + /// + /// Returns an error if there was an error writing into the buffer. fn fmt_stub(&self, buf: &mut String) -> FmtResult; } @@ -158,7 +165,7 @@ impl ToStub for DataType { buf, "{}", match self { - DataType::True | DataType::False => "bool", + DataType::Bool | DataType::True | DataType::False => "bool", DataType::Long => "int", DataType::Double => "float", DataType::String => "string", @@ -171,7 +178,6 @@ impl ToStub for DataType { DataType::Resource => "resource", DataType::Reference => "reference", DataType::Callable => "callable", - DataType::Bool => "bool", DataType::Iterable => "iterable", _ => "mixed", } @@ -194,6 +200,12 @@ impl ToStub for DocBlock { impl ToStub for Class { fn fmt_stub(&self, buf: &mut String) -> FmtResult { + fn stub(items: &[T]) -> impl Iterator> + '_ { + items + .iter() + .map(|item| item.to_stub().map(|stub| indent(&stub, 4))) + } + self.docs.fmt_stub(buf)?; let (_, name) = split_namespace(self.name.as_ref()); @@ -209,7 +221,7 @@ impl ToStub for Class { "implements {} ", self.implements .iter() - .map(|s| s.as_str()) + .map(RString::as_str) .collect::>() .join(", ") )?; @@ -217,12 +229,6 @@ impl ToStub for Class { writeln!(buf, "{{")?; - fn stub(items: &[T]) -> impl Iterator> + '_ { - items - .iter() - .map(|item| item.to_stub().map(|stub| indent(&stub, 4))) - } - buf.push_str( &stub(&self.constants) .chain(stub(&self.properties)) diff --git a/src/embed/mod.rs b/src/embed/mod.rs index 0a9cf18c15..1f52a60b52 100644 --- a/src/embed/mod.rs +++ b/src/embed/mod.rs @@ -25,19 +25,29 @@ use std::ptr::null_mut; pub use ffi::ext_php_rs_sapi_startup; pub use sapi::SapiModule; +/// The embed module provides a way to run php code from rust pub struct Embed; +/// Error type for the embed module #[derive(Debug)] pub enum EmbedError { + /// Failed to initialize InitError, + /// The script exited with a non-zero code ExecuteError(Option>), + /// The script exited with a non-zero code ExecuteScriptError, + /// The script is not a valid [`CString`] InvalidEvalString(NulError), + /// Failed to open the script file at the given path InvalidPath, + /// The script was executed but an exception was thrown CatchError, } impl EmbedError { + /// Check if the error is a bailout + #[must_use] pub fn is_bailout(&self) -> bool { matches!(self, EmbedError::CatchError) } @@ -54,6 +64,9 @@ impl Embed { /// # Returns /// /// * `Ok(())` - The script was executed successfully + /// + /// # Errors + /// /// * `Err(EmbedError)` - An error occurred during the execution of the /// script /// @@ -78,6 +91,7 @@ impl Embed { }; let mut file_handle = zend_file_handle { + #[allow(clippy::used_underscore_items)] handle: _zend_file_handle__bindgen_ty_1 { fp: null_mut() }, filename: null_mut(), opened_path: null_mut(), @@ -142,7 +156,7 @@ impl Embed { 0, null_mut(), panic_wrapper::, - &func as *const F as *const c_void, + (&raw const func).cast::(), ) }; @@ -151,7 +165,7 @@ impl Embed { return R::default(); } - match unsafe { *Box::from_raw(panic as *mut std::thread::Result) } { + match unsafe { *Box::from_raw(panic.cast::>()) } { Ok(r) => r, Err(err) => { // we resume the panic here so it can be caught correctly by the test framework @@ -168,6 +182,9 @@ impl Embed { /// # Returns /// /// * `Ok(Zval)` - The result of the evaluation + /// + /// # Errors + /// /// * `Err(EmbedError)` - An error occurred during the evaluation /// /// # Example @@ -190,9 +207,9 @@ impl Embed { let exec_result = try_catch(|| unsafe { zend_eval_string( - cstr.as_ptr() as *const c_char, + cstr.as_ptr().cast::(), &mut result, - b"run\0".as_ptr() as *const _, + c"run".as_ptr().cast(), ) }); @@ -206,6 +223,7 @@ impl Embed { #[cfg(test)] mod tests { + #![allow(clippy::unwrap_used)] use super::Embed; #[test] @@ -222,7 +240,7 @@ mod tests { Embed::run(|| { let result = Embed::eval("stupid code;"); - assert!(!result.is_ok()); + assert!(result.is_err()); }); } @@ -248,12 +266,12 @@ mod tests { Embed::run(|| { let result = Embed::run_script("src/embed/test-script-exception.php"); - assert!(!result.is_ok()); + assert!(result.is_err()); }); } #[test] - #[should_panic] + #[should_panic(expected = "test panic")] fn test_panic() { Embed::run::<(), _>(|| { panic!("test panic"); @@ -262,9 +280,7 @@ mod tests { #[test] fn test_return() { - let foo = Embed::run(|| { - return "foo"; - }); + let foo = Embed::run(|| "foo"); assert_eq!(foo, "foo"); } diff --git a/src/embed/sapi.rs b/src/embed/sapi.rs index ef6c359358..4053a7512a 100644 --- a/src/embed/sapi.rs +++ b/src/embed/sapi.rs @@ -9,6 +9,7 @@ pub type SapiModule = sapi_module_struct; impl SapiModule { /// Allocates the module entry on the heap, returning a pointer to the /// memory location. The caller is responsible for the memory pointed to. + #[must_use] pub fn into_raw(self) -> *mut Self { Box::into_raw(Box::new(self)) } diff --git a/src/error.rs b/src/error.rs index 071e997584..9bd2e178ef 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,6 +4,7 @@ use std::{ error::Error as ErrorTrait, ffi::{CString, NulError}, fmt::Display, + num::TryFromIntError, }; use crate::{ @@ -124,6 +125,12 @@ impl From for Error { } } +impl From for Error { + fn from(_value: TryFromIntError) -> Self { + Self::IntegerOverflow + } +} + impl From for PhpException { fn from(err: Error) -> Self { Self::default(err.to_string()) @@ -133,13 +140,20 @@ impl From for PhpException { /// Trigger an error that is reported in PHP the same way `trigger_error()` is. /// /// See specific error type descriptions at . -pub fn php_error(type_: ErrorType, message: &str) { - let c_string = match CString::new(message) { - Ok(string) => string, - Err(_) => { - return; - } +/// +/// # Panics +/// +/// * If the error type bits exceed `i32::MAX`. +pub fn php_error(type_: &ErrorType, message: &str) { + let Ok(c_string) = CString::new(message) else { + return; }; - unsafe { php_error_docref(std::ptr::null(), type_.bits() as _, c_string.as_ptr()) } + unsafe { + php_error_docref( + std::ptr::null(), + type_.bits().try_into().expect("Error type flags overflown"), + c_string.as_ptr(), + ); + } } diff --git a/src/exception.rs b/src/exception.rs index 1f2b201308..6ff7ab3763 100644 --- a/src/exception.rs +++ b/src/exception.rs @@ -1,6 +1,6 @@ //! Types and functions used for throwing exceptions from Rust to PHP. -use std::{ffi::CString, fmt::Debug}; +use std::{ffi::CString, fmt::Debug, ptr}; use crate::{ class::RegisteredClass, @@ -38,6 +38,7 @@ impl PhpException { /// * `message` - Message to contain in the exception. /// * `code` - Integer code to go inside the exception. /// * `ex` - Exception type to throw. + #[must_use] pub fn new(message: String, code: i32, ex: &'static ClassEntry) -> Self { Self { message, @@ -54,6 +55,7 @@ impl PhpException { /// # Parameters /// /// * `message` - Message to contain in the exception. + #[must_use] pub fn default(message: String) -> Self { Self::new(message, 0, ce::exception()) } @@ -63,6 +65,7 @@ impl PhpException { /// # Parameters /// /// * `message` - Message to contain in the exception. + #[must_use] pub fn from_class(message: String) -> Self { Self::new(message, 0, T::get_metadata().ce()) } @@ -81,6 +84,12 @@ impl PhpException { /// Throws the exception, returning nothing inside a result if successful /// and an error otherwise. + /// + /// # Errors + /// + /// * [`Error::InvalidException`] - If the exception type is an interface or + /// abstract class. + /// * If the message contains NUL bytes. pub fn throw(self) -> Result<()> { match self.object { Some(object) => throw_object(object), @@ -104,7 +113,7 @@ impl From<&str> for PhpException { #[cfg(feature = "anyhow")] impl From for PhpException { fn from(err: anyhow::Error) -> Self { - Self::new(format!("{:#}", err), 0, crate::zend::ce::exception()) + Self::new(format!("{err:#}"), 0, crate::zend::ce::exception()) } } @@ -119,6 +128,12 @@ impl From for PhpException { /// * `ex` - The exception type to throw. /// * `message` - The message to display when throwing the exception. /// +/// # Errors +/// +/// * [`Error::InvalidException`] - If the exception type is an interface or +/// abstract class. +/// * If the message contains NUL bytes. +/// /// # Examples /// /// ```no_run @@ -142,6 +157,12 @@ pub fn throw(ex: &ClassEntry, message: &str) -> Result<()> { /// * `code` - The status code to use when throwing the exception. /// * `message` - The message to display when throwing the exception. /// +/// # Errors +/// +/// * [`Error::InvalidException`] - If the exception type is an interface or +/// abstract class. +/// * If the message contains NUL bytes. +/// /// # Examples /// /// ```no_run @@ -161,8 +182,8 @@ pub fn throw_with_code(ex: &ClassEntry, code: i32, message: &str) -> Result<()> // to a pointer it will be valid. unsafe { zend_throw_exception_ex( - (ex as *const _) as *mut _, - code as _, + ptr::from_ref(ex).cast_mut(), + code.into(), CString::new("%s")?.as_ptr(), CString::new(message)?.as_ptr(), ) @@ -179,6 +200,11 @@ pub fn throw_with_code(ex: &ClassEntry, code: i32, message: &str) -> Result<()> /// /// * `object` - The zval of type object /// +/// # Errors +/// +/// *shrug* +/// TODO: does this error? +/// /// # Examples /// /// ```no_run diff --git a/src/ffi.rs b/src/ffi.rs index ca299af254..52004638c0 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -5,8 +5,8 @@ use std::{ffi::c_void, os::raw::c_char}; -pub const ZEND_MM_ALIGNMENT: u32 = 8; -pub const ZEND_MM_ALIGNMENT_MASK: i32 = -8; +pub const ZEND_MM_ALIGNMENT: isize = 8; +pub const ZEND_MM_ALIGNMENT_MASK: isize = -8; // These are not generated by Bindgen as everything in `bindings.rs` will have // the `#[link(name = "php")]` attribute appended. This will cause the wrapper diff --git a/src/flags.rs b/src/flags.rs index 2e50663839..f56520b7a8 100644 --- a/src/flags.rs +++ b/src/flags.rs @@ -338,11 +338,11 @@ pub enum FunctionType { impl From for FunctionType { #[allow(clippy::bad_bit_mask)] fn from(value: u8) -> Self { - match value as _ { + match value.into() { ZEND_INTERNAL_FUNCTION => Self::Internal, ZEND_USER_FUNCTION => Self::User, ZEND_EVAL_CODE => Self::Eval, - _ => panic!("Unknown function type: {}", value), + _ => panic!("Unknown function type: {value}"), } } } @@ -399,6 +399,7 @@ impl Default for DataType { impl DataType { /// Returns the integer representation of the data type. + #[must_use] pub const fn as_u32(&self) -> u32 { match self { DataType::Undef => IS_UNDEF, @@ -410,8 +411,7 @@ impl DataType { DataType::String => IS_STRING, DataType::Array => IS_ARRAY, DataType::Object(_) => IS_OBJECT, - DataType::Resource => IS_RESOURCE, - DataType::Reference => IS_RESOURCE, + DataType::Resource | DataType::Reference => IS_RESOURCE, DataType::Indirect => IS_INDIRECT, DataType::Callable => IS_CALLABLE, DataType::ConstantExpression => IS_CONSTANT_AST, @@ -533,6 +533,7 @@ impl Display for DataType { #[cfg(test)] mod tests { + #![allow(clippy::unnecessary_fallible_conversions)] use super::DataType; use crate::ffi::{ IS_ARRAY, IS_ARRAY_EX, IS_CONSTANT_AST, IS_CONSTANT_AST_EX, IS_DOUBLE, IS_FALSE, diff --git a/src/internal/class.rs b/src/internal/class.rs index d03540a1ae..e126a56f8d 100644 --- a/src/internal/class.rs +++ b/src/internal/class.rs @@ -26,24 +26,24 @@ pub trait PhpClassImpl { fn get_constants(self) -> &'static [(&'static str, &'static dyn IntoZvalDyn, DocComments)]; } -// Default implementation for classes without an `impl` block. Classes that do -// have an `impl` block will override this by implementing `PhpClassImpl` for -// `PhpClassImplCollector` (note the missing reference). This is -// `dtolnay` specialisation: https://github.com/dtolnay/case-studies/blob/master/autoref-specialization/README.md +/// Default implementation for classes without an `impl` block. Classes that do +/// have an `impl` block will override this by implementing `PhpClassImpl` for +/// `PhpClassImplCollector` (note the missing reference). This is +/// `dtolnay` specialisation: impl PhpClassImpl for &'_ PhpClassImplCollector { #[inline] fn get_methods(self) -> Vec<(FunctionBuilder<'static>, MethodFlags)> { - Default::default() + Vec::default() } #[inline] fn get_method_props<'a>(self) -> HashMap<&'static str, Property<'a, T>> { - Default::default() + HashMap::default() } #[inline] fn get_constructor(self) -> Option> { - Default::default() + Option::default() } #[inline] diff --git a/src/internal/function.rs b/src/internal/function.rs index 4f953a206d..2f4767aada 100644 --- a/src/internal/function.rs +++ b/src/internal/function.rs @@ -2,7 +2,7 @@ use crate::builders::FunctionBuilder; /// Implemented on ZSTs that represent PHP functions. pub trait PhpFunction { - /// Function used to 'build' the PHP function, returning a [`FunctionBuilder`] - /// used to build the function. + /// Function used to 'build' the PHP function, returning a + /// [`FunctionBuilder`] used to build the function. const FUNCTION_ENTRY: fn() -> FunctionBuilder<'static>; } diff --git a/src/internal/mod.rs b/src/internal/mod.rs index fe297df41e..bceae5a811 100644 --- a/src/internal/mod.rs +++ b/src/internal/mod.rs @@ -18,7 +18,9 @@ pub const MODULE_STARTUP_INIT: ModuleStartupMutex = const_mutex(None); /// Called by startup functions registered with the [`#[php_startup]`] macro. /// Initializes all classes that are defined by ext-php-rs (i.e. `Closure`). /// -/// [`#[php_startup]`]: crate::php_startup +/// [`#[php_startup]`]: `crate::php_startup` +// TODO: Measure this +#[allow(clippy::inline_always)] #[inline(always)] pub fn ext_php_rs_startup() { #[cfg(feature = "closure")] diff --git a/src/props.rs b/src/props.rs index 6c5386cf91..974eb197e8 100644 --- a/src/props.rs +++ b/src/props.rs @@ -37,6 +37,10 @@ pub trait Prop<'a> { /// # Parameters /// /// * `zv` - The zval to set the value of. + /// + /// # Errors + /// + /// If the property could not be read. fn get(&self, zv: &mut Zval) -> Result<()>; /// Sets the value of `self` with the contents of a given zval `zv`. @@ -44,6 +48,10 @@ pub trait Prop<'a> { /// # Parameters /// /// * `zv` - The zval containing the new value of `self`. + /// + /// # Errors + /// + /// If the property could not be set fn set(&mut self, zv: &'a Zval) -> Result<()>; } @@ -132,6 +140,7 @@ impl<'a, T: 'a> Property<'a, T> { /// /// let prop: Property = Property::method(Some(Test::get_prop), Some(Test::set_prop)); /// ``` + #[must_use] pub fn method(get: Option V>, set: Option) -> Self where for<'b> V: IntoZval + FromZval<'b> + 'a, @@ -172,8 +181,12 @@ impl<'a, T: 'a> Property<'a, T> { /// /// # Returns /// - /// Nothing upon success, a [`PhpException`] inside an error variant when - /// the property could not be retrieved. + /// Nothing upon success + /// + /// # Errors + /// + /// A [`PhpException`] inside an error variant when the property could not + /// be retrieved. /// /// # Examples /// @@ -219,8 +232,12 @@ impl<'a, T: 'a> Property<'a, T> { /// /// # Returns /// - /// Nothing upon success, a [`PhpException`] inside an error variant when - /// the property could not be set. + /// Nothing upon success + /// + /// # Errors + /// + /// A [`PhpException`] inside an error variant when the property could not + /// be set. /// /// # Examples /// diff --git a/src/rc.rs b/src/rc.rs index ecbb6f1de5..0104930d3c 100644 --- a/src/rc.rs +++ b/src/rc.rs @@ -33,7 +33,7 @@ pub trait PhpRc { /// Increments the reference counter by 1. fn inc_count(&mut self) { - self.get_rc_mut().refcount += 1 + self.get_rc_mut().refcount += 1; } /// Decrements the reference counter by 1. diff --git a/src/types/array.rs b/src/types/array.rs index db2fd15f66..2e51c62f82 100644 --- a/src/types/array.rs +++ b/src/types/array.rs @@ -7,6 +7,7 @@ use std::{ ffi::CString, fmt::{Debug, Display}, iter::FromIterator, + ptr, }; use crate::{ @@ -67,6 +68,7 @@ impl ZendHashTable { /// # Panics /// /// Panics if memory for the hashtable could not be allocated. + #[must_use] pub fn new() -> ZBox { Self::with_capacity(HT_MIN_SIZE) } @@ -89,9 +91,11 @@ impl ZendHashTable { /// # Panics /// /// Panics if memory for the hashtable could not be allocated. + #[must_use] pub fn with_capacity(size: u32) -> ZBox { unsafe { // SAFETY: PHP allocator handles the creation of the array. + #[allow(clippy::used_underscore_items)] let ptr = _zend_new_array(size); // SAFETY: `as_mut()` checks if the pointer is null, and panics if it is not. @@ -116,8 +120,9 @@ impl ZendHashTable { /// /// assert_eq!(ht.len(), 2); /// ``` + #[must_use] pub fn len(&self) -> usize { - unsafe { zend_array_count(self as *const ZendHashTable as *mut ZendHashTable) as usize } + unsafe { zend_array_count(ptr::from_ref(self).cast_mut()) as usize } } /// Returns whether the hash table is empty. @@ -136,6 +141,7 @@ impl ZendHashTable { /// /// assert_eq!(ht.is_empty(), false); /// ``` + #[must_use] pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -181,6 +187,7 @@ impl ZendHashTable { /// ht.insert("test", "hello world"); /// assert_eq!(ht.get("test").and_then(|zv| zv.str()), Some("hello world")); /// ``` + #[must_use] pub fn get(&self, key: &'_ str) -> Option<&Zval> { let str = CString::new(key).ok()?; unsafe { zend_hash_str_find(self, str.as_ptr(), key.len() as _).as_ref() } @@ -208,6 +215,7 @@ impl ZendHashTable { /// ht.insert("test", "hello world"); /// assert_eq!(ht.get("test").and_then(|zv| zv.str()), Some("hello world")); /// ``` + #[must_use] pub fn get_mut(&self, key: &'_ str) -> Option<&mut Zval> { let str = CString::new(key).ok()?; unsafe { zend_hash_str_find(self, str.as_ptr(), key.len() as _).as_mut() } @@ -235,6 +243,7 @@ impl ZendHashTable { /// ht.push(100); /// assert_eq!(ht.get_index(0).and_then(|zv| zv.long()), Some(100)); /// ``` + #[must_use] pub fn get_index(&self, key: u64) -> Option<&Zval> { unsafe { zend_hash_index_find(self, key).as_ref() } } @@ -261,6 +270,7 @@ impl ZendHashTable { /// ht.push(100); /// assert_eq!(ht.get_index(0).and_then(|zv| zv.long()), Some(100)); /// ``` + #[must_use] pub fn get_index_mut(&self, key: u64) -> Option<&mut Zval> { unsafe { zend_hash_index_find(self, key).as_mut() } } @@ -344,9 +354,12 @@ impl ZendHashTable { /// /// # Returns /// - /// Returns nothing in a result on success. Returns an error if the key - /// could not be converted into a [`CString`], or converting the value into - /// a [`Zval`] failed. + /// Returns nothing in a result on success. + /// + /// # Errors + /// + /// Returns an error if the key could not be converted into a [`CString`], + /// or converting the value into a [`Zval`] failed. /// /// # Example /// @@ -380,8 +393,11 @@ impl ZendHashTable { /// /// # Returns /// - /// Returns nothing in a result on success. Returns an error if converting - /// the value into a [`Zval`] failed. + /// Returns nothing in a result on success. + /// + /// # Errors + /// + /// Returns an error if converting the value into a [`Zval`] failed. /// /// # Example /// @@ -414,8 +430,11 @@ impl ZendHashTable { /// /// # Returns /// - /// Returns nothing in a result on success. Returns an error if converting - /// the value into a [`Zval`] failed. + /// Returns nothing in a result on success. + /// + /// # Errors + /// + /// Returns an error if converting the value into a [`Zval`] failed. /// /// # Example /// @@ -461,6 +480,7 @@ impl ZendHashTable { /// ht.insert("obviously not numerical", 10); /// assert!(!ht.has_numerical_keys()); /// ``` + #[must_use] pub fn has_numerical_keys(&self) -> bool { !self.into_iter().any(|(k, _)| !k.is_long()) } @@ -472,6 +492,10 @@ impl ZendHashTable { /// True if all keys on the hashtable are numerical and are in sequential /// order (i.e. starting at 0 and not skipping any keys). /// + /// # Panics + /// + /// Panics if the number of elements in the hashtable exceeds `i64::MAX`. + /// /// # Example /// /// ```no_run @@ -487,11 +511,12 @@ impl ZendHashTable { /// ht.insert_at_index(90, 10); /// assert!(!ht.has_sequential_keys()); /// ``` + #[must_use] pub fn has_sequential_keys(&self) -> bool { !self .into_iter() .enumerate() - .any(|(i, (k, _))| ArrayKey::Long(i as i64) != k) + .any(|(i, (k, _))| ArrayKey::Long(i64::try_from(i).expect("Integer overflow")) != k) } /// Returns an iterator over the values contained inside the hashtable, as @@ -508,6 +533,7 @@ impl ZendHashTable { /// dbg!(val); /// } #[inline] + #[must_use] pub fn values(&self) -> Values { Values::new(self) } @@ -532,6 +558,7 @@ impl ZendHashTable { /// dbg!(key, val); /// } #[inline] + #[must_use] pub fn iter(&self) -> Iter { self.into_iter() } @@ -558,7 +585,7 @@ impl ToOwned for ZendHashTable { fn to_owned(&self) -> Self::Owned { unsafe { // SAFETY: FFI call does not modify `self`, returns a new hashtable. - let ptr = zend_array_dup(self as *const ZendHashTable as *mut ZendHashTable); + let ptr = zend_array_dup(ptr::from_ref(self).cast_mut()); // SAFETY: `as_mut()` checks if the pointer is null, and panics if it is not. ZBox::from_raw( @@ -593,6 +620,7 @@ impl ArrayKey { /// # Returns /// /// Returns true if the key is an integer, false otherwise. + #[must_use] pub fn is_long(&self) -> bool { match self { ArrayKey::Long(_) => true, @@ -604,8 +632,8 @@ impl ArrayKey { impl Display for ArrayKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - ArrayKey::Long(key) => write!(f, "{}", key), - ArrayKey::String(key) => write!(f, "{}", key), + ArrayKey::Long(key) => write!(f, "{key}"), + ArrayKey::String(key) => write!(f, "{key}"), } } } @@ -707,10 +735,7 @@ impl DoubleEndedIterator for Iter<'_> { } let key_type = unsafe { - zend_hash_get_current_key_type_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &mut self.pos as *mut HashPosition, - ) + zend_hash_get_current_key_type_ex(ptr::from_ref(self.ht).cast_mut(), &raw mut self.pos) }; if key_type == -1 { @@ -721,15 +746,15 @@ impl DoubleEndedIterator for Iter<'_> { unsafe { zend_hash_get_current_key_zval_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &key as *const Zval as *mut Zval, - &mut self.end_pos as *mut HashPosition, + ptr::from_ref(self.ht).cast_mut(), + (&raw const key).cast_mut(), + &raw mut self.end_pos, ); } let value = unsafe { &*zend_hash_get_current_data_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &mut self.end_pos as *mut HashPosition, + ptr::from_ref(self.ht).cast_mut(), + &raw mut self.end_pos, ) }; @@ -739,10 +764,7 @@ impl DoubleEndedIterator for Iter<'_> { }; unsafe { - zend_hash_move_backwards_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &mut self.end_pos as *mut HashPosition, - ) + zend_hash_move_backwards_ex(ptr::from_ref(self.ht).cast_mut(), &raw mut self.end_pos) }; self.end_num -= 1; @@ -757,10 +779,7 @@ impl<'a> Iter<'a> { } let key_type = unsafe { - zend_hash_get_current_key_type_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &mut self.pos as *mut HashPosition, - ) + zend_hash_get_current_key_type_ex(ptr::from_ref(self.ht).cast_mut(), &raw mut self.pos) }; // Key type `-1` is ??? @@ -775,16 +794,14 @@ impl<'a> Iter<'a> { unsafe { zend_hash_get_current_key_zval_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &key as *const Zval as *mut Zval, - &mut self.pos as *mut HashPosition, + ptr::from_ref(self.ht).cast_mut(), + (&raw const key).cast_mut(), + &raw mut self.pos, ); } let value = unsafe { - let val_ptr = zend_hash_get_current_data_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &mut self.pos as *mut HashPosition, - ); + let val_ptr = + zend_hash_get_current_data_ex(ptr::from_ref(self.ht).cast_mut(), &raw mut self.pos); if val_ptr.is_null() { return None; @@ -794,15 +811,10 @@ impl<'a> Iter<'a> { }; if !key.is_long() && !key.is_string() { - key.set_long(self.current_num) + key.set_long(self.current_num); } - unsafe { - zend_hash_move_forward_ex( - self.ht as *const ZendHashTable as *mut ZendHashTable, - &mut self.pos as *mut HashPosition, - ) - }; + unsafe { zend_hash_move_forward_ex(ptr::from_ref(self.ht).cast_mut(), &raw mut self.pos) }; self.current_num += 1; Some((key, value)) @@ -885,6 +897,8 @@ impl<'a> FromZval<'a> for &'a ZendHashTable { // HashMap /////////////////////////////////////////// +// TODO: Generalize hasher +#[allow(clippy::implicit_hasher)] impl<'a, V> TryFrom<&'a ZendHashTable> for HashMap where V: FromZval<'a>, @@ -917,7 +931,7 @@ where value.len().try_into().map_err(|_| Error::IntegerOverflow)?, ); - for (k, v) in value.into_iter() { + for (k, v) in value { ht.insert(k.as_ref(), v)?; } @@ -925,6 +939,8 @@ where } } +// TODO: Generalize hasher +#[allow(clippy::implicit_hasher)] impl IntoZval for HashMap where K: AsRef, @@ -940,6 +956,8 @@ where } } +// TODO: Generalize hasher +#[allow(clippy::implicit_hasher)] impl<'a, T> FromZval<'a> for HashMap where T: FromZval<'a>, @@ -983,7 +1001,7 @@ where value.len().try_into().map_err(|_| Error::IntegerOverflow)?, ); - for val in value.into_iter() { + for val in value { ht.push(val)?; } @@ -1019,7 +1037,7 @@ where impl FromIterator for ZBox { fn from_iter>(iter: T) -> Self { let mut ht = ZendHashTable::new(); - for item in iter.into_iter() { + for item in iter { // Inserting a zval cannot fail, as `push` only returns `Err` if converting // `val` to a zval fails. let _ = ht.push(item); @@ -1031,7 +1049,7 @@ impl FromIterator for ZBox { impl FromIterator<(u64, Zval)> for ZBox { fn from_iter>(iter: T) -> Self { let mut ht = ZendHashTable::new(); - for (key, val) in iter.into_iter() { + for (key, val) in iter { // Inserting a zval cannot fail, as `push` only returns `Err` if converting // `val` to a zval fails. let _ = ht.insert_at_index(key, val); @@ -1043,7 +1061,7 @@ impl FromIterator<(u64, Zval)> for ZBox { impl<'a> FromIterator<(&'a str, Zval)> for ZBox { fn from_iter>(iter: T) -> Self { let mut ht = ZendHashTable::new(); - for (key, val) in iter.into_iter() { + for (key, val) in iter { // Inserting a zval cannot fail, as `push` only returns `Err` if converting // `val` to a zval fails. let _ = ht.insert(key, val); diff --git a/src/types/callable.rs b/src/types/callable.rs index dfb65b3b35..a30038d510 100644 --- a/src/types/callable.rs +++ b/src/types/callable.rs @@ -1,6 +1,6 @@ //! Types related to callables in PHP (anonymous functions, functions, etc). -use std::{convert::TryFrom, ops::Deref}; +use std::{convert::TryFrom, ops::Deref, ptr}; use crate::{ convert::{FromZval, IntoZvalDyn}, @@ -43,6 +43,10 @@ impl<'a> ZendCallable<'a> { /// # Parameters /// /// * `callable` - The underlying [`Zval`] that is callable. + /// + /// # Errors + /// + /// * [`Error::Callable`] - If the zval was not callable. pub fn new_owned(callable: Zval) -> Result { if callable.is_callable() { Ok(Self(OwnedZval::Owned(callable))) @@ -59,6 +63,10 @@ impl<'a> ZendCallable<'a> { /// /// * `name` - Name of the callable function. /// + /// # Errors + /// + /// Returns an error if the function does not exist or is not callable. + /// /// # Example /// /// ```no_run @@ -87,8 +95,13 @@ impl<'a> ZendCallable<'a> { /// /// # Returns /// - /// Returns the result wrapped in [`Ok`] upon success. If calling the - /// callable fails, or an exception is thrown, an [`Err`] is returned. + /// Returns the result wrapped in [`Ok`] upon success. + /// + /// # Errors + /// + /// * If calling the callable fails, or an exception is thrown, an [`Err`] + /// is returned. + /// * If the number of parameters exceeds `u32::MAX`. /// /// # Example /// @@ -99,6 +112,8 @@ impl<'a> ZendCallable<'a> { /// let result = strpos.try_call(vec![&"hello", &"e"]).unwrap(); /// assert_eq!(result.long(), Some(1)); /// ``` + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] pub fn try_call(&self, params: Vec<&dyn IntoZvalDyn>) -> Result { if !self.0.is_callable() { @@ -114,12 +129,13 @@ impl<'a> ZendCallable<'a> { let packed = params.into_boxed_slice(); let result = unsafe { + #[allow(clippy::used_underscore_items)] _call_user_function_impl( std::ptr::null_mut(), - self.0.as_ref() as *const crate::ffi::_zval_struct as *mut crate::ffi::_zval_struct, + ptr::from_ref(self.0.as_ref()).cast_mut(), &mut retval, - len as _, - packed.as_ptr() as *mut _, + len.try_into()?, + packed.as_ptr().cast_mut(), std::ptr::null_mut(), ) }; diff --git a/src/types/class_object.rs b/src/types/class_object.rs index ecf68aafad..04f3eaeab3 100644 --- a/src/types/class_object.rs +++ b/src/types/class_object.rs @@ -109,8 +109,8 @@ impl ZendClassObject { unsafe fn internal_new(val: Option, ce: Option<&'static ClassEntry>) -> ZBox { let size = mem::size_of::>(); let meta = T::get_metadata(); - let ce = ce.unwrap_or_else(|| meta.ce()) as *const _ as *mut _; - let obj = ext_php_rs_zend_object_alloc(size as _, ce) as *mut ZendClassObject; + let ce = ptr::from_ref(ce.unwrap_or_else(|| meta.ce())).cast_mut(); + let obj = ext_php_rs_zend_object_alloc(size as _, ce).cast::>(); let obj = obj .as_mut() .expect("Failed to allocate for new Zend object"); @@ -148,8 +148,13 @@ impl ZendClassObject { /// # Parameters /// /// * `obj` - The zend object to get the [`ZendClassObject`] for. + /// + /// # Panics + /// + /// * If the std offset over/underflows `isize`. + #[must_use] pub fn from_zend_obj(std: &zend_object) -> Option<&Self> { - Some(Self::_from_zend_obj(std)?) + Some(Self::internal_from_zend_obj(std)?) } /// Returns a mutable reference to the [`ZendClassObject`] of a given zend @@ -159,16 +164,21 @@ impl ZendClassObject { /// # Parameters /// /// * `obj` - The zend object to get the [`ZendClassObject`] for. + /// + /// # Panics + /// + /// * If the std offset over/underflows `isize`. #[allow(clippy::needless_pass_by_ref_mut)] pub fn from_zend_obj_mut(std: &mut zend_object) -> Option<&mut Self> { - Self::_from_zend_obj(std) + Self::internal_from_zend_obj(std) } - fn _from_zend_obj(std: &zend_object) -> Option<&mut Self> { - let std = std as *const zend_object as *const c_char; + fn internal_from_zend_obj(std: &zend_object) -> Option<&mut Self> { + let std = ptr::from_ref(std).cast::(); let ptr = unsafe { - let ptr = std.offset(0 - Self::std_offset() as isize) as *const Self; - (ptr as *mut Self).as_mut()? + let offset = isize::try_from(Self::std_offset()).expect("Offset overflow"); + let ptr = std.offset(0 - offset).cast::(); + ptr.cast_mut().as_mut()? }; if ptr.std.instance_of(T::get_metadata().ce()) { @@ -188,7 +198,7 @@ impl ZendClassObject { unsafe { let null = NonNull::::dangling(); let base = null.as_ref() as *const Self; - let std = &null.as_ref().std as *const zend_object; + let std = &raw const null.as_ref().std; (std as usize) - (base as usize) } @@ -266,7 +276,7 @@ impl Clone for ZBox> { // therefore we can dereference both safely. unsafe { let mut new = ZendClassObject::new((***self).clone()); - zend_objects_clone_members(&mut new.std, &self.std as *const _ as *mut _); + zend_objects_clone_members(&mut new.std, (&raw const self.std).cast_mut()); new } } diff --git a/src/types/iterable.rs b/src/types/iterable.rs index 3d99babbc7..fd011e680c 100644 --- a/src/types/iterable.rs +++ b/src/types/iterable.rs @@ -17,6 +17,8 @@ pub enum Iterable<'a> { impl Iterable<'_> { /// Creates a new rust iterator from a PHP iterable. /// May return None if a Traversable cannot be rewound. + // TODO: Check iter not returning iterator + #[allow(clippy::iter_not_returning_iterator)] pub fn iter(&mut self) -> Option { match self { Iterable::Array(array) => Some(Iter::Array(array.iter())), @@ -25,6 +27,8 @@ impl Iterable<'_> { } } +// TODO: Implement `iter_mut` +#[allow(clippy::into_iter_without_iter)] impl<'a> IntoIterator for &'a mut Iterable<'a> { type Item = (Zval, &'a Zval); type IntoIter = Iter<'a>; diff --git a/src/types/iterator.rs b/src/types/iterator.rs index b6591ab348..ef6ddeaea2 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -7,17 +7,22 @@ use std::fmt::{Debug, Formatter}; /// A PHP Iterator. /// -/// In PHP, iterators are represented as zend_object_iterator. This allows user -/// to iterate over objects implementing Traversable interface using foreach. +/// In PHP, iterators are represented as `zend_object_iterator`. This allows +/// user to iterate over objects implementing Traversable interface using +/// foreach. /// -/// Use ZendIterable to iterate over both iterators and arrays. +/// Use [`Iterable`] to iterate over both iterators and arrays. +/// +/// [`Iterable`]: crate::types::iterable::Iterable pub type ZendIterator = zend_object_iterator; impl ZendIterator { - /// Creates a new rust iterator from a zend_object_iterator. + /// Creates a new rust iterator from a `zend_object_iterator`. /// - /// Returns a iterator over the zend_object_iterator, or None if the + /// Returns a iterator over the `zend_object_iterator`, or [`None`] if the /// iterator cannot be rewound. + // TODO: Check iter not returning iterator + #[allow(clippy::iter_not_returning_iterator)] pub fn iter(&mut self) -> Option { self.index = 0; @@ -31,7 +36,7 @@ impl ZendIterator { /// Check if the current position of the iterator is valid. /// /// As an example this will call the user defined valid method of the - /// ['\Iterator'] interface. see + /// `\Iterator` interface. see pub fn valid(&mut self) -> bool { if let Some(valid) = unsafe { (*self.funcs).valid } { let valid = unsafe { valid(&mut *self) == ZEND_RESULT_CODE_SUCCESS }; @@ -49,7 +54,7 @@ impl ZendIterator { /// Rewind the iterator to the first element. /// /// As an example this will call the user defined rewind method of the - /// ['\Iterator'] interface. see + /// `\Iterator` interface. see /// /// # Returns /// @@ -68,7 +73,7 @@ impl ZendIterator { /// Move the iterator forward to the next element. /// /// As an example this will call the user defined next method of the - /// ['\Iterator'] interface. see + /// `\Iterator` interface. see /// /// # Returns /// @@ -89,7 +94,7 @@ impl ZendIterator { /// # Returns /// /// Returns a reference to the current data of the iterator if available - /// , ['None'] otherwise. + /// , [`None`] otherwise. pub fn get_current_data<'a>(&mut self) -> Option<&'a Zval> { let get_current_data = unsafe { (*self.funcs).get_current_data }?; let value = unsafe { &*get_current_data(&mut *self) }; @@ -105,8 +110,8 @@ impl ZendIterator { /// /// # Returns /// - /// Returns a new ['Zval'] containing the current key of the iterator if - /// available , ['None'] otherwise. + /// Returns a new [`Zval`] containing the current key of the iterator if + /// available , [`None`] otherwise. pub fn get_current_key(&mut self) -> Option { let get_current_key = unsafe { (*self.funcs).get_current_key? }; let mut key = Zval::new(); @@ -123,6 +128,8 @@ impl ZendIterator { } } +// TODO: Implement `iter_mut` +#[allow(clippy::into_iter_without_iter)] impl<'a> IntoIterator for &'a mut ZendIterator { type Item = (Zval, &'a Zval); type IntoIter = Iter<'a>; @@ -159,12 +166,12 @@ impl<'a> Iterator for Iter<'a> { self.zi.index += 1; - let real_index = self.zi.index - 1; + let real_index = i64::try_from(self.zi.index - 1).expect("index out of bounds"); let key = match self.zi.get_current_key() { None => { let mut z = Zval::new(); - z.set_long(real_index as i64); + z.set_long(real_index); z } Some(key) => key, @@ -185,6 +192,7 @@ impl<'a> FromZvalMut<'a> for &'a mut ZendIterator { #[cfg(test)] #[cfg(feature = "embed")] mod tests { + #![allow(clippy::unwrap_used)] use crate::embed::Embed; #[test] diff --git a/src/types/long.rs b/src/types/long.rs index cc056b26ce..6d11f7e033 100644 --- a/src/types/long.rs +++ b/src/types/long.rs @@ -1,6 +1,6 @@ //! Represents an integer introduced in PHP. Note that the size of this integer -//! differs. On a 32-bit system, a ZendLong is 32-bits, while on a 64-bit system -//! it is 64-bits. +//! differs. On a 32-bit system, a [`ZendLong`] is 32-bits, while on a 64-bit +//! system it is 64-bits. use crate::{ convert::IntoZval, diff --git a/src/types/mod.rs b/src/types/mod.rs index a3db468c88..3d209dc5a0 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -36,6 +36,7 @@ impl FromZval<'_> for f32 { const TYPE: DataType = DataType::Double; fn from_zval(zval: &Zval) -> Option { + #[allow(clippy::cast_possible_truncation)] zval.double().map(|v| v as f32) } } diff --git a/src/types/object.rs b/src/types/object.rs index 4ef55846b4..6c5fad3930 100644 --- a/src/types/object.rs +++ b/src/types/object.rs @@ -1,7 +1,7 @@ //! Represents an object in PHP. Allows for overriding the internal object used //! by classes, allowing users to store Rust data inside a PHP object. -use std::{convert::TryInto, fmt::Debug, ops::DerefMut, os::raw::c_char}; +use std::{convert::TryInto, fmt::Debug, os::raw::c_char, ptr}; use crate::{ boxed::{ZBox, ZBoxable}, @@ -38,20 +38,20 @@ impl ZendObject { /// # Panics /// /// Panics when allocating memory for the new object fails. + #[must_use] pub fn new(ce: &ClassEntry) -> ZBox { // SAFETY: Using emalloc to allocate memory inside Zend arena. Casting `ce` to // `*mut` is valid as the function will not mutate `ce`. unsafe { let ptr = match ce.__bindgen_anon_2.create_object { None => { - let ptr = zend_objects_new(ce as *const _ as *mut _); - if ptr.is_null() { - panic!("Failed to allocate memory for Zend object") - } - object_properties_init(ptr, ce as *const _ as *mut _); + let ptr = zend_objects_new(ptr::from_ref(ce).cast_mut()); + assert!(!ptr.is_null(), "Failed to allocate memory for Zend object"); + + object_properties_init(ptr, ptr::from_ref(ce).cast_mut()); ptr } - Some(v) => v(ce as *const _ as *mut _), + Some(v) => v(ptr::from_ref(ce).cast_mut()), }; ZBox::from_raw( @@ -78,6 +78,7 @@ impl ZendObject { /// /// obj.set_property("hello", "world"); /// ``` + #[must_use] pub fn new_stdclass() -> ZBox { // SAFETY: This will be `NULL` until it is initialized. `as_ref()` checks for // null, so we can panic if it's null. @@ -86,6 +87,7 @@ impl ZendObject { /// Converts a class object into an owned [`ZendObject`]. This removes any /// possibility of accessing the underlying attached Rust struct. + #[must_use] pub fn from_class_object(obj: ZBox>) -> ZBox { let this = obj.into_raw(); // SAFETY: Consumed box must produce a well-aligned non-null pointer. @@ -97,6 +99,7 @@ impl ZendObject { /// # Panics /// /// Panics if the class entry is invalid. + #[must_use] pub fn get_class_entry(&self) -> &'static ClassEntry { // SAFETY: it is OK to panic here since PHP would segfault anyway // when encountering an object with no class entry. @@ -104,13 +107,18 @@ impl ZendObject { } /// Attempts to retrieve the class name of the object. + /// + /// # Errors + /// + /// * `Error::InvalidScope` - If the object handlers or the class name + /// cannot be retrieved. pub fn get_class_name(&self) -> Result { unsafe { self.handlers()? .get_class_name .and_then(|f| f(self).as_ref()) .ok_or(Error::InvalidScope) - .and_then(|s| s.try_into()) + .and_then(TryInto::try_into) } } @@ -121,6 +129,7 @@ impl ZendObject { /// # Panics /// /// Panics if the class entry is invalid. + #[must_use] pub fn instance_of(&self, ce: &ClassEntry) -> bool { self.get_class_entry().instance_of(ce) } @@ -129,8 +138,9 @@ impl ZendObject { /// Rust type `T`. /// /// This method doesn't check the class and interface inheritance chain. + #[must_use] pub fn is_instance(&self) -> bool { - (self.ce as *const ClassEntry).eq(&(T::get_metadata().ce() as *const _)) + (self.ce.cast_const()).eq(&ptr::from_ref(T::get_metadata().ce())) } /// Returns whether this object is an instance of \Traversable @@ -138,6 +148,7 @@ impl ZendObject { /// # Panics /// /// Panics if the class entry is invalid. + #[must_use] pub fn is_traversable(&self) -> bool { self.instance_of(ce::traversable()) } @@ -145,8 +156,17 @@ impl ZendObject { /// Tries to call a method on the object. /// /// # Returns + /// /// Returns the return value of the method, or an error if the method /// could not be found or called. + /// + /// # Errors + /// + /// * `Error::Callable` - If the method could not be found. + /// * If a parameter could not be converted to a zval. + /// * If the parameter count is bigger than `u32::MAX`. + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] pub fn try_call_method(&self, name: &str, params: Vec<&dyn IntoZvalDyn>) -> Result { let mut retval = Zval::new(); @@ -160,21 +180,24 @@ impl ZendObject { unsafe { let res = zend_hash_str_find_ptr_lc( &(*self.ce).function_table, - name.as_ptr() as *const c_char, + name.as_ptr().cast::(), name.len(), - ) as *mut zend_function; + ) + .cast::(); + if res.is_null() { return Err(Error::Callable); } + zend_call_known_function( res, - self as *const _ as *mut _, + ptr::from_ref(self).cast_mut(), self.ce, &mut retval, - len as _, - packed.as_ptr() as *mut _, + len.try_into()?, + packed.as_ptr().cast_mut(), std::ptr::null_mut(), - ) + ); }; Ok(retval) @@ -188,6 +211,11 @@ impl ZendObject { /// /// * `name` - The name of the property. /// * `query` - The type of query to use when attempting to get a property. + /// + /// # Errors + /// + /// * `Error::InvalidScope` - If the object handlers or the properties + /// cannot be retrieved. pub fn get_property<'a, T>(&'a self, name: &str) -> Result where T: FromZval<'a>, @@ -202,7 +230,7 @@ impl ZendObject { let zv = unsafe { self.handlers()?.read_property.ok_or(Error::InvalidScope)?( self.mut_ptr(), - name.deref_mut(), + &mut *name, 1, std::ptr::null_mut(), &mut rv, @@ -220,6 +248,11 @@ impl ZendObject { /// /// * `name` - The name of the property. /// * `value` - The value to set the property to. + /// + /// # Errors + /// + /// * `Error::InvalidScope` - If the object handlers or the properties + /// cannot be retrieved. pub fn set_property(&mut self, name: &str, value: impl IntoZval) -> Result<()> { let mut name = ZendStr::new(name, false); let mut value = value.into_zval(false)?; @@ -227,7 +260,7 @@ impl ZendObject { unsafe { self.handlers()?.write_property.ok_or(Error::InvalidScope)?( self, - name.deref_mut(), + &mut *name, &mut value, std::ptr::null_mut(), ) @@ -245,13 +278,18 @@ impl ZendObject { /// /// * `name` - The name of the property. /// * `query` - The 'query' to classify if a property exists. + /// + /// # Errors + /// + /// * `Error::InvalidScope` - If the object handlers or the properties + /// cannot be retrieved. pub fn has_property(&self, name: &str, query: PropertyQuery) -> Result { let mut name = ZendStr::new(name, false); Ok(unsafe { self.handlers()?.has_property.ok_or(Error::InvalidScope)?( self.mut_ptr(), - name.deref_mut(), + &mut *name, query as _, std::ptr::null_mut(), ) @@ -260,6 +298,11 @@ impl ZendObject { /// Attempts to retrieve the properties of the object. Returned inside a /// Zend Hashtable. + /// + /// # Errors + /// + /// * `Error::InvalidScope` - If the object handlers or the properties + /// cannot be retrieved. pub fn get_properties(&self) -> Result<&HashTable> { unsafe { self.handlers()? @@ -272,6 +315,10 @@ impl ZendObject { /// Extracts some type from a Zend object. /// /// This is a wrapper function around `FromZendObject::extract()`. + /// + /// # Errors + /// + /// Returns an error if the conversion fails. pub fn extract<'a, T>(&'a self) -> Result where T: FromZendObject<'a>, @@ -287,6 +334,7 @@ impl ZendObject { /// /// [`spl_object_id`]: https://www.php.net/manual/function.spl-object-id #[inline] + #[must_use] pub fn get_id(&self) -> u32 { self.handle } @@ -298,6 +346,7 @@ impl ZendObject { /// This is equivalent to calling the [`spl_object_hash`] PHP function. /// /// [`spl_object_hash`]: https://www.php.net/manual/function.spl-object-hash.php + #[must_use] pub fn hash(&self) -> String { format!("{:016x}0000000000000000", self.handle) } @@ -313,7 +362,7 @@ impl ZendObject { /// a mutable pointer but does not modify the underlying data. #[inline] fn mut_ptr(&self) -> *mut Self { - (self as *const Self) as *mut Self + ptr::from_ref(self).cast_mut() } } @@ -332,7 +381,7 @@ impl Debug for ZendObject { ); if let Ok(props) = self.get_properties() { - for (key, val) in props.iter() { + for (key, val) in props { dbg.field(key.to_string().as_str(), val); } } @@ -389,7 +438,7 @@ impl FromZendObject<'_> for String { unsafe { zend_call_known_function( (*obj.ce).__tostring, - obj as *const _ as *mut _, + ptr::from_ref(obj).cast_mut(), obj.ce, &mut ret, 0, diff --git a/src/types/string.rs b/src/types/string.rs index 84d93a164b..5b54cb6f5c 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -6,7 +6,7 @@ use std::{ convert::TryFrom, ffi::{CStr, CString}, fmt::Debug, - slice, + ptr, slice, }; use parking_lot::{const_mutex, Mutex}; @@ -118,6 +118,7 @@ impl ZendStr { /// let c_s = CString::new("Hello world!").unwrap(); /// let s = ZendStr::from_c_str(&c_s, false); /// ``` + #[must_use] pub fn from_c_str(str: &CStr, persistent: bool) -> ZBox { unsafe { let ptr = @@ -250,6 +251,7 @@ impl ZendStr { /// let s = ZendStr::new("hello, world!", false); /// assert_eq!(s.len(), 13); /// ``` + #[must_use] pub fn len(&self) -> usize { self.len } @@ -264,6 +266,7 @@ impl ZendStr { /// let s = ZendStr::new("hello, world!", false); /// assert_eq!(s.is_empty(), false); /// ``` + #[must_use] pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -271,7 +274,9 @@ impl ZendStr { /// Attempts to return a reference to the underlying bytes inside the Zend /// string as a [`CStr`]. /// - /// Returns an [Error::InvalidCString] variant if the string contains null + /// # Errors + /// + /// Returns an [`Error::InvalidCString`] variant if the string contains null /// bytes. pub fn as_c_str(&self) -> Result<&CStr> { let bytes_with_null = @@ -282,7 +287,9 @@ impl ZendStr { /// Attempts to return a reference to the underlying bytes inside the Zend /// string. /// - /// Returns an [Error::InvalidUtf8] variant if the [`str`] contains + /// # Errors + /// + /// Returns an [`Error::InvalidUtf8`] variant if the [`str`] contains /// non-UTF-8 characters. /// /// # Example @@ -299,23 +306,25 @@ impl ZendStr { return Ok(str); } let str = std::str::from_utf8(self.as_bytes()).map_err(|_| Error::InvalidUtf8)?; - unsafe { ext_php_rs_set_known_valid_utf8(self.as_ptr() as *mut _) }; + unsafe { ext_php_rs_set_known_valid_utf8(self.as_ptr().cast_mut()) }; Ok(str) } /// Returns a reference to the underlying bytes inside the Zend string. + #[must_use] pub fn as_bytes(&self) -> &[u8] { unsafe { slice::from_raw_parts(self.val.as_ptr().cast(), self.len()) } } /// Returns a raw pointer to this object + #[must_use] pub fn as_ptr(&self) -> *const ZendStr { - self as *const _ + ptr::from_ref(self) } /// Returns a mutable pointer to this object pub fn as_mut_ptr(&mut self) -> *mut ZendStr { - self as *mut _ + ptr::from_mut(self) } } @@ -467,10 +476,10 @@ mod tests { assert!(result.is_ok()); - let zval = result.as_ref().unwrap(); + let zval = result.as_ref().expect("Unreachable"); assert!(zval.is_string()); - assert_eq!(zval.string().unwrap(), "foo"); + assert_eq!(zval.string(), Some("foo".to_string())); }); } } diff --git a/src/types/zval.rs b/src/types/zval.rs index a1ef08bf88..a1c0a080f6 100644 --- a/src/types/zval.rs +++ b/src/types/zval.rs @@ -39,24 +39,32 @@ pub type Zval = zval; impl Zval { /// Creates a new, empty zval. + #[must_use] pub const fn new() -> Self { Self { value: zend_value { ptr: ptr::null_mut(), }, + #[allow(clippy::used_underscore_items)] u1: _zval_struct__bindgen_ty_1 { type_info: DataType::Null.as_u32(), }, + #[allow(clippy::used_underscore_items)] u2: _zval_struct__bindgen_ty_2 { next: 0 }, } } /// Dereference the zval, if it is a reference. + #[must_use] pub fn dereference(&self) -> &Self { self.reference().or_else(|| self.indirect()).unwrap_or(self) } /// Dereference the zval mutable, if it is a reference. + /// + /// # Panics + /// + /// Panics if a mutable reference to the zval is not possible. pub fn dereference_mut(&mut self) -> &mut Self { // TODO: probably more ZTS work is needed here if self.is_reference() { @@ -71,6 +79,7 @@ impl Zval { } /// Returns the value of the zval if it is a long. + #[must_use] pub fn long(&self) -> Option { if self.is_long() { Some(unsafe { self.value.lval }) @@ -80,6 +89,7 @@ impl Zval { } /// Returns the value of the zval if it is a bool. + #[must_use] pub fn bool(&self) -> Option { if self.is_true() { Some(true) @@ -91,6 +101,7 @@ impl Zval { } /// Returns the value of the zval if it is a double. + #[must_use] pub fn double(&self) -> Option { if self.is_double() { Some(unsafe { self.value.dval }) @@ -104,6 +115,7 @@ impl Zval { /// Note that this functions output will not be the same as /// [`string()`](#method.string), as this function does not attempt to /// convert other types into a [`String`]. + #[must_use] pub fn zend_str(&self) -> Option<&ZendStr> { if self.is_string() { unsafe { self.value.str_.as_ref() } @@ -116,7 +128,7 @@ impl Zval { /// /// [`str()`]: #method.str pub fn string(&self) -> Option { - self.str().map(|s| s.to_string()) + self.str().map(ToString::to_string) } /// Returns the value of the zval if it is a string. @@ -125,6 +137,7 @@ impl Zval { /// [`string()`](#method.string), as this function does not attempt to /// convert other types into a [`String`], as it could not pass back a /// [`&str`] in those cases. + #[must_use] pub fn str(&self) -> Option<&str> { self.zend_str().and_then(|zs| zs.as_str().ok()) } @@ -170,6 +183,7 @@ impl Zval { } /// Returns the value of the zval if it is a resource. + #[must_use] pub fn resource(&self) -> Option<*mut zend_resource> { // TODO: Can we improve this function? I haven't done much research into // resources so I don't know if this is the optimal way to return this. @@ -182,6 +196,7 @@ impl Zval { /// Returns an immutable reference to the underlying zval hashtable if the /// zval contains an array. + #[must_use] pub fn array(&self) -> Option<&ZendHashTable> { if self.is_array() { unsafe { self.value.arr.as_ref() } @@ -201,6 +216,7 @@ impl Zval { } /// Returns the value of the zval if it is an object. + #[must_use] pub fn object(&self) -> Option<&ZendObject> { if self.is_object() { unsafe { self.value.obj.as_ref() } @@ -220,6 +236,12 @@ impl Zval { } /// Attempts to call a method on the object contained in the zval. + /// + /// # Errors + /// + /// * Returns an error if the [`Zval`] is not an object. + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] pub fn try_call_method(&self, name: &str, params: Vec<&dyn IntoZvalDyn>) -> Result { self.object() @@ -228,9 +250,10 @@ impl Zval { } /// Returns the value of the zval if it is an internal indirect reference. + #[must_use] pub fn indirect(&self) -> Option<&Zval> { if self.is_indirect() { - Some(unsafe { &*(self.value.zv as *mut Zval) }) + Some(unsafe { &*(self.value.zv.cast::()) }) } else { None } @@ -238,15 +261,17 @@ impl Zval { /// Returns a mutable reference to the zval if it is an internal indirect /// reference. + #[must_use] pub fn indirect_mut(&self) -> Option<&mut Zval> { if self.is_indirect() { - Some(unsafe { &mut *(self.value.zv as *mut Zval) }) + Some(unsafe { &mut *(self.value.zv.cast::()) }) } else { None } } /// Returns the value of the zval if it is a reference. + #[must_use] pub fn reference(&self) -> Option<&Zval> { if self.is_reference() { Some(&unsafe { self.value.ref_.as_ref() }?.val) @@ -265,12 +290,14 @@ impl Zval { } /// Returns the value of the zval if it is callable. + #[must_use] pub fn callable(&self) -> Option { // The Zval is checked if it is callable in the `new` function. ZendCallable::new(self).ok() } /// Returns an iterator over the zval if it is traversable. + #[must_use] pub fn traversable(&self) -> Option<&mut ZendIterator> { if self.is_traversable() { self.object()?.get_class_entry().get_iterator(self, false) @@ -281,6 +308,7 @@ impl Zval { /// Returns an iterable over the zval if it is an array or traversable. (is /// iterable) + #[must_use] pub fn iterable(&self) -> Option { if self.is_iterable() { Iterable::from_zval(self) @@ -296,9 +324,10 @@ impl Zval { /// The caller must ensure that the pointer contained in the zval is in fact /// a pointer to an instance of `T`, as the zval has no way of defining /// the type of pointer. + #[must_use] pub unsafe fn ptr(&self) -> Option<*mut T> { if self.is_ptr() { - Some(self.value.ptr as *mut T) + Some(self.value.ptr.cast::()) } else { None } @@ -316,80 +345,100 @@ impl Zval { /// # Parameters /// /// * `params` - A list of parameters to call the function with. + /// + /// # Errors + /// + /// * Returns an error if the [`Zval`] is not callable. + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] pub fn try_call(&self, params: Vec<&dyn IntoZvalDyn>) -> Result { self.callable().ok_or(Error::Callable)?.try_call(params) } /// Returns the type of the Zval. + #[must_use] pub fn get_type(&self) -> DataType { - DataType::from(unsafe { self.u1.v.type_ } as u32) + DataType::from(u32::from(unsafe { self.u1.v.type_ })) } /// Returns true if the zval is a long, false otherwise. + #[must_use] pub fn is_long(&self) -> bool { self.get_type() == DataType::Long } /// Returns true if the zval is null, false otherwise. + #[must_use] pub fn is_null(&self) -> bool { self.get_type() == DataType::Null } /// Returns true if the zval is true, false otherwise. + #[must_use] pub fn is_true(&self) -> bool { self.get_type() == DataType::True } /// Returns true if the zval is false, false otherwise. + #[must_use] pub fn is_false(&self) -> bool { self.get_type() == DataType::False } /// Returns true if the zval is a bool, false otherwise. + #[must_use] pub fn is_bool(&self) -> bool { self.is_true() || self.is_false() } /// Returns true if the zval is a double, false otherwise. + #[must_use] pub fn is_double(&self) -> bool { self.get_type() == DataType::Double } /// Returns true if the zval is a string, false otherwise. + #[must_use] pub fn is_string(&self) -> bool { self.get_type() == DataType::String } /// Returns true if the zval is a resource, false otherwise. + #[must_use] pub fn is_resource(&self) -> bool { self.get_type() == DataType::Resource } /// Returns true if the zval is an array, false otherwise. + #[must_use] pub fn is_array(&self) -> bool { self.get_type() == DataType::Array } /// Returns true if the zval is an object, false otherwise. + #[must_use] pub fn is_object(&self) -> bool { matches!(self.get_type(), DataType::Object(_)) } /// Returns true if the zval is a reference, false otherwise. + #[must_use] pub fn is_reference(&self) -> bool { self.get_type() == DataType::Reference } /// Returns true if the zval is a reference, false otherwise. + #[must_use] pub fn is_indirect(&self) -> bool { self.get_type() == DataType::Indirect } /// Returns true if the zval is callable, false otherwise. + #[must_use] pub fn is_callable(&self) -> bool { let ptr: *const Self = self; - unsafe { zend_is_callable(ptr as *mut Self, 0, std::ptr::null_mut()) } + unsafe { zend_is_callable(ptr.cast_mut(), 0, std::ptr::null_mut()) } } /// Checks if the zval is identical to another one. @@ -398,13 +447,15 @@ impl Zval { /// # Parameters /// /// * `other` - The the zval to check identity against. + #[must_use] pub fn is_identical(&self, other: &Self) -> bool { let self_p: *const Self = self; let other_p: *const Self = other; - unsafe { zend_is_identical(self_p as *mut Self, other_p as *mut Self) } + unsafe { zend_is_identical(self_p.cast_mut(), other_p.cast_mut()) } } /// Returns true if the zval is traversable, false otherwise. + #[must_use] pub fn is_traversable(&self) -> bool { match self.object() { None => false, @@ -414,12 +465,14 @@ impl Zval { /// Returns true if the zval is iterable (array or traversable), false /// otherwise. + #[must_use] pub fn is_iterable(&self) -> bool { let ptr: *const Self = self; - unsafe { zend_is_iterable(ptr as *mut Self) } + unsafe { zend_is_iterable(ptr.cast_mut()) } } /// Returns true if the zval contains a pointer, false otherwise. + #[must_use] pub fn is_ptr(&self) -> bool { self.get_type() == DataType::Ptr } @@ -431,6 +484,11 @@ impl Zval { /// /// * `val` - The value to set the zval as. /// * `persistent` - Whether the string should persist between requests. + /// + /// # Errors + /// + /// Never returns an error. + // TODO: Check if we can drop the result here. pub fn set_string(&mut self, val: &str, persistent: bool) -> Result<()> { self.set_zend_string(ZendStr::new(val, persistent)); Ok(()) @@ -465,6 +523,11 @@ impl Zval { /// /// * `val` - The value to set the zval as. /// * `persistent` - Whether the string should persist between requests. + /// + /// # Errors + /// + /// Never returns an error. + // TODO: Check if we can drop the result here. pub fn set_interned_string(&mut self, val: &str, persistent: bool) -> Result<()> { self.set_zend_string(ZendStr::new_interned(val, persistent)); Ok(()) @@ -476,10 +539,10 @@ impl Zval { /// /// * `val` - The value to set the zval as. pub fn set_long>(&mut self, val: T) { - self._set_long(val.into()) + self.internal_set_long(val.into()); } - fn _set_long(&mut self, val: ZendLong) { + fn internal_set_long(&mut self, val: ZendLong) { self.change_type(ZvalTypeFlags::Long); self.value.lval = val; } @@ -490,10 +553,10 @@ impl Zval { /// /// * `val` - The value to set the zval as. pub fn set_double>(&mut self, val: T) { - self._set_double(val.into()) + self.internal_set_double(val.into()); } - fn _set_double(&mut self, val: f64) { + fn internal_set_double(&mut self, val: f64) { self.change_type(ZvalTypeFlags::Double); self.value.dval = val; } @@ -504,10 +567,10 @@ impl Zval { /// /// * `val` - The value to set the zval as. pub fn set_bool>(&mut self, val: T) { - self._set_bool(val.into()) + self.internal_set_bool(val.into()); } - fn _set_bool(&mut self, val: bool) { + fn internal_set_bool(&mut self, val: bool) { self.change_type(if val { ZvalTypeFlags::True } else { @@ -540,7 +603,7 @@ impl Zval { pub fn set_object(&mut self, val: &mut ZendObject) { self.change_type(ZvalTypeFlags::ObjectEx); val.inc_count(); // TODO(david): not sure if this is needed :/ - self.value.obj = (val as *const ZendObject) as *mut ZendObject; + self.value.obj = ptr::from_ref(val).cast_mut(); } /// Sets the value of the zval as an array. Returns nothing in a result on @@ -549,6 +612,10 @@ impl Zval { /// # Parameters /// /// * `val` - The value to set the zval as. + /// + /// # Errors + /// + /// * Returns an error if the conversion to a hashtable fails. pub fn set_array, Error = Error>>( &mut self, val: T, @@ -575,7 +642,7 @@ impl Zval { /// * `ptr` - The pointer to set the zval as. pub fn set_ptr(&mut self, ptr: *mut T) { self.u1.type_info = ZvalTypeFlags::Ptr.bits(); - self.value.ptr = ptr as *mut c_void; + self.value.ptr = ptr.cast::(); } /// Used to drop the Zval but keep the value of the zval intact. @@ -605,6 +672,7 @@ impl Zval { /// Extracts some type from a `Zval`. /// /// This is a wrapper function around `TryFrom`. + #[must_use] pub fn extract<'a, T>(&'a self) -> Option where T: FromZval<'a>, @@ -624,6 +692,7 @@ impl Zval { /// # Returns /// /// The cloned zval. + #[must_use] pub fn shallow_clone(&self) -> Zval { let mut new = Zval::new(); new.u1 = self.u1; @@ -658,20 +727,18 @@ impl Debug for Zval { } match ty { - DataType::Undef => field!(Option::<()>::None), - DataType::Null => field!(Option::<()>::None), + DataType::Undef | DataType::Null | DataType::ConstantExpression | DataType::Void => { + field!(Option::<()>::None) + } DataType::False => field!(false), DataType::True => field!(true), DataType::Long => field!(self.long()), DataType::Double => field!(self.double()), - DataType::String | DataType::Mixed => field!(self.string()), + DataType::String | DataType::Mixed | DataType::Callable => field!(self.string()), DataType::Array => field!(self.array()), DataType::Object(_) => field!(self.object()), DataType::Resource => field!(self.resource()), DataType::Reference => field!(self.reference()), - DataType::Callable => field!(self.string()), - DataType::ConstantExpression => field!(Option::<()>::None), - DataType::Void => field!(Option::<()>::None), DataType::Bool => field!(self.bool()), DataType::Indirect => field!(self.indirect()), DataType::Iterable => field!(self.iterable()), diff --git a/src/zend/_type.rs b/src/zend/_type.rs index 7718514497..a683385cc0 100644 --- a/src/zend/_type.rs +++ b/src/zend/_type.rs @@ -18,9 +18,10 @@ impl ZendType { /// /// * `pass_by_ref` - Whether the value should be passed by reference. /// * `is_variadic` - Whether this type represents a variadic argument. + #[must_use] pub fn empty(pass_by_ref: bool, is_variadic: bool) -> Self { Self { - ptr: ptr::null::() as *mut c_void, + ptr: ptr::null_mut::(), type_mask: Self::arg_info_flags(pass_by_ref, is_variadic), } } @@ -39,6 +40,7 @@ impl ZendType { /// * `is_variadic` - Whether the type is for a variadic argument. /// * `allow_null` - Whether the type should allow null to be passed in /// place. + #[must_use] pub fn empty_from_type( type_: DataType, pass_by_ref: bool, @@ -80,7 +82,7 @@ impl ZendType { ) -> Option { let mut flags = Self::arg_info_flags(pass_by_ref, is_variadic); if allow_null { - flags |= _ZEND_TYPE_NULLABLE_BIT + flags |= _ZEND_TYPE_NULLABLE_BIT; } cfg_if::cfg_if! { if #[cfg(php83)] { @@ -91,7 +93,10 @@ impl ZendType { } Some(Self { - ptr: std::ffi::CString::new(class_name).ok()?.into_raw() as *mut c_void, + ptr: std::ffi::CString::new(class_name) + .ok()? + .into_raw() + .cast::(), type_mask: flags, }) } @@ -117,20 +122,21 @@ impl ZendType { ) -> Self { assert!(!matches!(type_, DataType::Object(Some(_)))); Self { - ptr: ptr::null::() as *mut c_void, + ptr: ptr::null_mut::(), type_mask: Self::type_init_code(type_, pass_by_ref, is_variadic, allow_null), } } /// Calculates the internal flags of the type. - /// Translation of of the `_ZEND_ARG_INFO_FLAGS` macro from zend_API.h:110. + /// Translation of of the `_ZEND_ARG_INFO_FLAGS` macro from + /// `zend_API.h:110`. /// /// # Parameters /// /// * `pass_by_ref` - Whether the value should be passed by reference. /// * `is_variadic` - Whether this type represents a variadic argument. pub(crate) fn arg_info_flags(pass_by_ref: bool, is_variadic: bool) -> u32 { - ((pass_by_ref as u32) << _ZEND_SEND_MODE_SHIFT) + (u32::from(pass_by_ref) << _ZEND_SEND_MODE_SHIFT) | (if is_variadic { _ZEND_IS_VARIADIC_BIT } else { @@ -139,7 +145,7 @@ impl ZendType { } /// Calculates the internal flags of the type. - /// Translation of the `ZEND_TYPE_INIT_CODE` macro from zend_API.h:163. + /// Translation of the `ZEND_TYPE_INIT_CODE` macro from `zend_API.h:163`. /// /// # Parameters /// diff --git a/src/zend/ce.rs b/src/zend/ce.rs index 57b1877f92..655819f3d1 100644 --- a/src/zend/ce.rs +++ b/src/zend/ce.rs @@ -14,96 +14,172 @@ use crate::ffi::{ use super::ClassEntry; /// Returns the base [`stdClass`](https://www.php.net/manual/en/class.stdclass.php) class. +/// +/// # Panics +/// +/// If stdclass [`ClassEntry`] is not available pub fn stdclass() -> &'static ClassEntry { unsafe { zend_standard_class_def.as_ref() }.unwrap() } /// Returns the base [`Throwable`](https://www.php.net/manual/en/class.throwable.php) class. +/// +/// # Panics +/// +/// If throwable [`ClassEntry`] is not available pub fn throwable() -> &'static ClassEntry { unsafe { zend_ce_throwable.as_ref() }.unwrap() } /// Returns the base [`Exception`](https://www.php.net/manual/en/class.exception.php) class. +/// +/// # Panics +/// +/// If exception [`ClassEntry`] is not available pub fn exception() -> &'static ClassEntry { unsafe { zend_ce_exception.as_ref() }.unwrap() } /// Returns the base [`ErrorException`](https://www.php.net/manual/en/class.errorexception.php) class. +/// +/// # Panics +/// +/// If error exception [`ClassEntry`] is not available pub fn error_exception() -> &'static ClassEntry { unsafe { zend_ce_error_exception.as_ref() }.unwrap() } /// Returns the base [`CompileError`](https://www.php.net/manual/en/class.compileerror.php) class. +/// +/// # Panics +/// +/// If compile error [`ClassEntry`] is not available pub fn compile_error() -> &'static ClassEntry { unsafe { zend_ce_compile_error.as_ref() }.unwrap() } /// Returns the base [`ParseError`](https://www.php.net/manual/en/class.parseerror.php) class. +/// +/// # Panics +/// +/// If parse error [`ClassEntry`] is not available pub fn parse_error() -> &'static ClassEntry { unsafe { zend_ce_parse_error.as_ref() }.unwrap() } /// Returns the base [`TypeError`](https://www.php.net/manual/en/class.typeerror.php) class. +/// +/// # Panics +/// +/// If type error [`ClassEntry`] is not available pub fn type_error() -> &'static ClassEntry { unsafe { zend_ce_type_error.as_ref() }.unwrap() } /// Returns the base [`ArgumentCountError`](https://www.php.net/manual/en/class.argumentcounterror.php) class. +/// +/// # Panics +/// +/// If argument count error [`ClassEntry`] is not available pub fn argument_count_error() -> &'static ClassEntry { unsafe { zend_ce_argument_count_error.as_ref() }.unwrap() } /// Returns the base [`ValueError`](https://www.php.net/manual/en/class.valueerror.php) class. +/// +/// # Panics +/// +/// If value error [`ClassEntry`] is not available pub fn value_error() -> &'static ClassEntry { unsafe { zend_ce_value_error.as_ref() }.unwrap() } /// Returns the base [`ArithmeticError`](https://www.php.net/manual/en/class.arithmeticerror.php) class. +/// +/// # Panics +/// +/// If arithmetic error [`ClassEntry`] is not available pub fn arithmetic_error() -> &'static ClassEntry { unsafe { zend_ce_arithmetic_error.as_ref() }.unwrap() } /// Returns the base [`DivisionByZeroError`](https://www.php.net/manual/en/class.divisionbyzeroerror.php) class. +/// +/// # Panics +/// +/// If division by zero error [`ClassEntry`] is not available pub fn division_by_zero_error() -> &'static ClassEntry { unsafe { zend_ce_division_by_zero_error.as_ref() }.unwrap() } /// Returns the base [`UnhandledMatchError`](https://www.php.net/manual/en/class.unhandledmatcherror.php) class. +/// +/// # Panics +/// +/// If unhandled match error [`ClassEntry`] is not available pub fn unhandled_match_error() -> &'static ClassEntry { unsafe { zend_ce_unhandled_match_error.as_ref() }.unwrap() } /// Returns the [`Traversable`](https://www.php.net/manual/en/class.traversable.php) interface. +/// +/// # Panics +/// +/// If traversable [`ClassEntry`] is not available pub fn traversable() -> &'static ClassEntry { unsafe { zend_ce_traversable.as_ref() }.unwrap() } /// Returns the [`IteratorAggregate`](https://www.php.net/manual/en/class.iteratoraggregate.php) interface. +/// +/// # Panics +/// +/// If aggregate [`ClassEntry`] is not available pub fn aggregate() -> &'static ClassEntry { unsafe { zend_ce_aggregate.as_ref() }.unwrap() } /// Returns the [`Iterator`](https://www.php.net/manual/en/class.iterator.php) interface. +/// +/// # Panics +/// +/// If iterator [`ClassEntry`] is not available pub fn iterator() -> &'static ClassEntry { unsafe { zend_ce_iterator.as_ref() }.unwrap() } /// Returns the [`ArrayAccess`](https://www.php.net/manual/en/class.arrayaccess.php) interface. +/// +/// # Panics +/// +/// If arrayaccess [`ClassEntry`] is not available pub fn arrayaccess() -> &'static ClassEntry { unsafe { zend_ce_arrayaccess.as_ref() }.unwrap() } /// Returns the [`Serializable`](https://www.php.net/manual/en/class.serializable.php) interface. +/// +/// # Panics +/// +/// If serializable [`ClassEntry`] is not available pub fn serializable() -> &'static ClassEntry { unsafe { zend_ce_serializable.as_ref() }.unwrap() } /// Returns the [`Countable`](https://www.php.net/manual/en/class.countable.php) interface. +/// +/// # Panics +/// +/// If countable [`ClassEntry`] is not available pub fn countable() -> &'static ClassEntry { unsafe { zend_ce_countable.as_ref() }.unwrap() } /// Returns the [`Stringable`](https://www.php.net/manual/en/class.stringable.php) interface. +/// +/// # Panics +/// +/// If stringable [`ClassEntry`] is not available pub fn stringable() -> &'static ClassEntry { unsafe { zend_ce_stringable.as_ref() }.unwrap() } diff --git a/src/zend/class.rs b/src/zend/class.rs index bd2035f886..a2440e25cc 100644 --- a/src/zend/class.rs +++ b/src/zend/class.rs @@ -9,7 +9,8 @@ use crate::{ types::{ZendObject, ZendStr}, zend::ExecutorGlobals, }; -use std::{convert::TryInto, fmt::Debug, ops::DerefMut}; +use std::ptr; +use std::{convert::TryInto, fmt::Debug}; /// A PHP class entry. /// @@ -21,13 +22,12 @@ impl ClassEntry { /// /// Returns a reference to the class if found, or [`None`] if the class /// could not be found or the class table has not been initialized. + #[must_use] pub fn try_find(name: &str) -> Option<&'static Self> { ExecutorGlobals::get().class_table()?; let mut name = ZendStr::new(name, false); - unsafe { - crate::ffi::zend_lookup_class_ex(name.deref_mut(), std::ptr::null_mut(), 0).as_ref() - } + unsafe { crate::ffi::zend_lookup_class_ex(&mut *name, std::ptr::null_mut(), 0).as_ref() } } /// Creates a new [`ZendObject`], returned inside an [`ZBox`] @@ -37,17 +37,20 @@ impl ClassEntry { /// /// Panics when allocating memory for the new object fails. #[allow(clippy::new_ret_no_self)] + #[must_use] pub fn new(&self) -> ZBox { ZendObject::new(self) } /// Returns the class flags. + #[must_use] pub fn flags(&self) -> ClassFlags { ClassFlags::from_bits_truncate(self.ce_flags) } /// Returns `true` if the class entry is an interface, and `false` /// otherwise. + #[must_use] pub fn is_interface(&self) -> bool { self.flags().contains(ClassFlags::Interface) } @@ -57,24 +60,35 @@ impl ClassEntry { /// # Parameters /// /// * `other` - The inherited class entry to check. + #[must_use] pub fn instance_of(&self, other: &ClassEntry) -> bool { if self == other { return true; } - unsafe { instanceof_function_slow(self as _, other as _) } + unsafe { instanceof_function_slow(ptr::from_ref(self), ptr::from_ref(other)) } } /// Returns an iterator of all the interfaces that the class implements. /// /// Returns [`None`] if the interfaces have not been resolved on the /// class. + /// + /// # Panics + /// + /// Panics if the number of interfaces exceeds `isize::MAX`. + #[must_use] pub fn interfaces(&self) -> Option> { self.flags() .contains(ClassFlags::ResolvedInterfaces) .then(|| unsafe { (0..self.num_interfaces) - .map(move |i| *self.__bindgen_anon_3.interfaces.offset(i as _)) + .map(move |i| { + *self + .__bindgen_anon_3 + .interfaces + .offset(isize::try_from(i).expect("index exceeds isize")) + }) .filter_map(|ptr| ptr.as_ref()) }) } @@ -84,6 +98,7 @@ impl ClassEntry { /// If the parent of the class has not been resolved, it attempts to find /// the parent by name. Returns [`None`] if the parent was not resolved /// and the parent was not able to be found by name. + #[must_use] pub fn parent(&self) -> Option<&Self> { if self.flags().contains(ClassFlags::ResolvedParent) { unsafe { self.__bindgen_anon_1.parent.as_ref() } @@ -96,22 +111,19 @@ impl ClassEntry { /// Returns the iterator for the class for a specific instance /// /// Returns [`None`] if there is no associated iterator for the class. + #[must_use] pub fn get_iterator<'a>(&self, zval: &'a Zval, by_ref: bool) -> Option<&'a mut ZendIterator> { let ptr: *const Self = self; let zval_ptr: *const Zval = zval; - let iterator = unsafe { - (*ptr).get_iterator?( - ptr as *mut ClassEntry, - zval_ptr as *mut Zval, - if by_ref { 1 } else { 0 }, - ) - }; + let iterator = + unsafe { (*ptr).get_iterator?(ptr.cast_mut(), zval_ptr.cast_mut(), i32::from(by_ref)) }; unsafe { iterator.as_mut() } } /// Gets the name of the class. + #[must_use] pub fn name(&self) -> Option<&str> { unsafe { self.name.as_ref().and_then(|s| s.as_str().ok()) } } @@ -135,7 +147,7 @@ impl Debug for ClassEntry { .field("is_interface", &self.is_interface()) .field( "interfaces", - &self.interfaces().map(|iter| iter.collect::>()), + &self.interfaces().map(Iterator::collect::>), ) .field("parent", &self.parent()) .finish() diff --git a/src/zend/ex.rs b/src/zend/ex.rs index cb389cb2e7..c57f34e668 100644 --- a/src/zend/ex.rs +++ b/src/zend/ex.rs @@ -197,11 +197,13 @@ impl ExecuteData { } /// Attempt to retrieve the function that is being called. + #[must_use] pub fn function(&self) -> Option<&Function> { unsafe { self.func.as_ref() } } /// Attempt to retrieve the previous execute data on the call stack. + #[must_use] pub fn previous(&self) -> Option<&Self> { unsafe { self.prev_execute_data.as_ref() } } @@ -209,14 +211,15 @@ impl ExecuteData { /// Translation of macro `ZEND_CALL_ARG(call, n)` /// zend_compile.h:578 /// - /// The resultant zval reference has a lifetime equal to the lifetime of + /// The resultant [`Zval`] reference has a lifetime equal to the lifetime of /// `self`. This isn't specified because when you attempt to get a /// reference to args and the `$this` object, Rust doesn't let you. /// Since this is a private method it's up to the caller to ensure the /// lifetime isn't exceeded. #[doc(hidden)] unsafe fn zend_call_arg<'a>(&self, n: usize) -> Option<&'a mut Zval> { - let ptr = self.zend_call_var_num(n as isize); + let n = isize::try_from(n).expect("n is too large"); + let ptr = self.zend_call_var_num(n); ptr.as_mut() } @@ -224,7 +227,7 @@ impl ExecuteData { /// zend_compile.h: 575 #[doc(hidden)] unsafe fn zend_call_var_num(&self, n: isize) -> *mut Zval { - let ptr = self as *const Self as *mut Zval; + let ptr = std::ptr::from_ref(self) as *mut Zval; ptr.offset(Self::zend_call_frame_slot() + n) } @@ -240,8 +243,8 @@ impl ExecuteData { /// zend_alloc.h:41 #[doc(hidden)] fn zend_mm_aligned_size() -> isize { - let size = std::mem::size_of::(); - ((size as isize) + ZEND_MM_ALIGNMENT as isize - 1) & ZEND_MM_ALIGNMENT_MASK as isize + let size = isize::try_from(std::mem::size_of::()).expect("size of T is too large"); + (size + ZEND_MM_ALIGNMENT - 1) & ZEND_MM_ALIGNMENT_MASK } } diff --git a/src/zend/function.rs b/src/zend/function.rs index 656a40e590..94a3d9ba03 100644 --- a/src/zend/function.rs +++ b/src/zend/function.rs @@ -31,9 +31,10 @@ impl Debug for FunctionEntry { impl FunctionEntry { /// Returns an empty function entry, signifing the end of a function list. + #[must_use] pub fn end() -> Self { Self { - fname: ptr::null() as *const c_char, + fname: ptr::null::(), handler: None, arg_info: ptr::null(), num_args: 0, @@ -47,6 +48,7 @@ impl FunctionEntry { /// Converts the function entry into a raw and pointer, releasing it to the /// C world. + #[must_use] pub fn into_raw(self) -> *mut Self { Box::into_raw(Box::new(self)) } @@ -57,14 +59,16 @@ pub type Function = zend_function; impl Function { /// Returns the function type. + #[must_use] pub fn function_type(&self) -> FunctionType { FunctionType::from(unsafe { self.type_ }) } /// Attempts to fetch a [`Function`] from the function name. + #[must_use] pub fn try_from_function(name: &str) -> Option { unsafe { - let res = zend_fetch_function_str(name.as_ptr() as *const c_char, name.len()); + let res = zend_fetch_function_str(name.as_ptr().cast::(), name.len()); if res.is_null() { return None; } @@ -73,15 +77,17 @@ impl Function { } /// Attempts to fetch a [`Function`] from the class and method name. + #[must_use] pub fn try_from_method(class: &str, name: &str) -> Option { match ClassEntry::try_find(class) { None => None, Some(ce) => unsafe { let res = zend_hash_str_find_ptr_lc( &ce.function_table, - name.as_ptr() as *const c_char, + name.as_ptr().cast::(), name.len(), - ) as *mut zend_function; + ) + .cast::(); if res.is_null() { return None; } @@ -102,8 +108,12 @@ impl Function { /// /// # Returns /// - /// Returns the result wrapped in [`Ok`] upon success. If calling the - /// callable fails, or an exception is thrown, an [`Err`] is returned. + /// Returns the result wrapped in [`Ok`] upon success. + /// + /// # Errors + /// + /// * If the function call fails, an [`Err`] is returned. + /// * If the number of parameters is not a valid `u32` value. /// /// # Example /// @@ -114,6 +124,8 @@ impl Function { /// let result = strpos.try_call(vec![&"hello", &"e"]).unwrap(); /// assert_eq!(result.long(), Some(1)); /// ``` + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] pub fn try_call(&self, params: Vec<&dyn IntoZvalDyn>) -> Result { let mut retval = Zval::new(); @@ -126,14 +138,14 @@ impl Function { unsafe { zend_call_known_function( - self as *const _ as *mut _, + ptr::from_ref(self).cast_mut(), std::ptr::null_mut(), std::ptr::null_mut(), &mut retval, - len as _, - packed.as_ptr() as *mut _, + len.try_into()?, + packed.as_ptr().cast_mut(), std::ptr::null_mut(), - ) + ); }; Ok(retval) diff --git a/src/zend/globals.rs b/src/zend/globals.rs index 932d0a42a7..3c98827007 100644 --- a/src/zend/globals.rs +++ b/src/zend/globals.rs @@ -41,11 +41,15 @@ pub type SapiModule = _sapi_module_struct; impl ExecutorGlobals { /// Returns a reference to the PHP executor globals. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release /// the lock. + /// + /// # Panics + /// + /// * If static executor globals are not set pub fn get() -> GlobalReadGuard { // SAFETY: PHP executor globals are statically declared therefore should never // return an invalid pointer. @@ -65,11 +69,15 @@ impl ExecutorGlobals { /// Returns a mutable reference to the PHP executor globals. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release /// the lock. + /// + /// # Panics + /// + /// * If static executor globals are not set pub fn get_mut() -> GlobalWriteGuard { // SAFETY: PHP executor globals are statically declared therefore should never // return an invalid pointer. @@ -88,26 +96,35 @@ impl ExecutorGlobals { } /// Attempts to retrieve the global class hash table. + #[must_use] pub fn class_table(&self) -> Option<&ZendHashTable> { unsafe { self.class_table.as_ref() } } /// Attempts to retrieve the global functions hash table. + #[must_use] pub fn function_table(&self) -> Option<&ZendHashTable> { unsafe { self.function_table.as_ref() } } /// Attempts to retrieve the global functions hash table as mutable. + #[must_use] pub fn function_table_mut(&self) -> Option<&mut ZendHashTable> { unsafe { self.function_table.as_mut() } } /// Retrieves the ini values for all ini directives in the current executor /// context.. + /// + /// # Panics + /// + /// * If the ini directives are not a valid hash table. + /// * If the ini entry is not a string. + #[must_use] pub fn ini_values(&self) -> HashMap> { let hash_table = unsafe { &*self.ini_directives }; let mut ini_hash_map: HashMap> = HashMap::new(); - for (key, value) in hash_table.iter() { + for (key, value) in hash_table { ini_hash_map.insert(key.to_string(), unsafe { let ini_entry = &*value.ptr::().expect("Invalid ini entry"); if ini_entry.value.is_null() { @@ -126,6 +143,7 @@ impl ExecutorGlobals { } /// Attempts to retrieve the global constants table. + #[must_use] pub fn constants(&self) -> Option<&ZendHashTable> { unsafe { self.zend_constants.as_ref() } } @@ -136,6 +154,7 @@ impl ExecutorGlobals { /// This function requires the executor globals to be mutably held, which /// could lead to a deadlock if the globals are already borrowed immutably /// or mutably. + #[must_use] pub fn take_exception() -> Option> { { // This avoid a write lock if there is no exception. @@ -154,6 +173,7 @@ impl ExecutorGlobals { } /// Checks if the executor globals contain an exception. + #[must_use] pub fn has_exception() -> bool { !Self::get().exception.is_null() } @@ -164,6 +184,11 @@ impl ExecutorGlobals { /// This function requires the executor globals to be mutably held, which /// could lead to a deadlock if the globals are already borrowed immutably /// or mutably. + /// + /// # Errors + /// + /// If an exception is present, it will be returned as `Err` value inside a + /// [`PhpResult`]. pub fn throw_if_exception() -> PhpResult<()> { if let Some(e) = Self::take_exception() { Err(crate::error::Error::Exception(e).into()) @@ -204,11 +229,15 @@ impl ExecutorGlobals { impl SapiModule { /// Returns a reference to the PHP SAPI module. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release /// the lock. + /// + /// # Panics + /// + /// * If static executor globals are not set pub fn get() -> GlobalReadGuard { // SAFETY: PHP executor globals are statically declared therefore should never // return an invalid pointer. @@ -220,11 +249,15 @@ impl SapiModule { /// Returns a mutable reference to the PHP executor globals. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release /// the lock. + /// + /// # Panics + /// + /// * If static executor globals are not set pub fn get_mut() -> GlobalWriteGuard { // SAFETY: PHP executor globals are statically declared therefore should never // return an invalid pointer. @@ -241,7 +274,7 @@ pub type ProcessGlobals = php_core_globals; impl ProcessGlobals { /// Returns a reference to the PHP process globals. /// - /// The process globals are guarded by a RwLock. There can be multiple + /// The process globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release @@ -264,7 +297,7 @@ impl ProcessGlobals { /// Returns a mutable reference to the PHP executor globals. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release @@ -286,6 +319,7 @@ impl ProcessGlobals { } /// Get the HTTP Server variables. Equivalent of $_SERVER. + #[must_use] pub fn http_server_vars(&self) -> Option<&ZendHashTable> { // $_SERVER is lazy-initted, we need to call zend_is_auto_global // if it's not already populated. @@ -301,6 +335,11 @@ impl ProcessGlobals { } /// Get the HTTP POST variables. Equivalent of $_POST. + /// + /// # Panics + /// + /// * If the post global is not found or fails to be populated. + #[must_use] pub fn http_post_vars(&self) -> &ZendHashTable { self.http_globals[TRACK_VARS_POST as usize] .array() @@ -308,6 +347,11 @@ impl ProcessGlobals { } /// Get the HTTP GET variables. Equivalent of $_GET. + /// + /// # Panics + /// + /// * If the get global is not found or fails to be populated. + #[must_use] pub fn http_get_vars(&self) -> &ZendHashTable { self.http_globals[TRACK_VARS_GET as usize] .array() @@ -315,6 +359,11 @@ impl ProcessGlobals { } /// Get the HTTP Cookie variables. Equivalent of $_COOKIE. + /// + /// # Panics + /// + /// * If the cookie global is not found or fails to be populated. + #[must_use] pub fn http_cookie_vars(&self) -> &ZendHashTable { self.http_globals[TRACK_VARS_COOKIE as usize] .array() @@ -324,8 +373,9 @@ impl ProcessGlobals { /// Get the HTTP Request variables. Equivalent of $_REQUEST. /// /// # Panics - /// - If the request global is not found or fails to be populated. - /// - If the request global is not a ZendArray. + /// + /// * If the request global is not found or fails to be populated. + /// * If the request global is not a [`ZendHashTable`]. pub fn http_request_vars(&self) -> Option<&ZendHashTable> { cfg_if::cfg_if! { if #[cfg(php81)] { @@ -339,9 +389,10 @@ impl ProcessGlobals { // `$_REQUEST` is lazy-initted, we need to call `zend_is_auto_global` to make // sure it's populated. - if !unsafe { zend_is_auto_global(key) } { - panic!("Failed to get request global"); - } + assert!( + unsafe { zend_is_auto_global(key) }, + "Failed to get request global" + ); let symbol_table = &ExecutorGlobals::get().symbol_table; cfg_if::cfg_if! { @@ -360,6 +411,11 @@ impl ProcessGlobals { } /// Get the HTTP Environment variables. Equivalent of $_ENV. + /// + /// # Panics + /// + /// * If the environment global is not found or fails to be populated. + #[must_use] pub fn http_env_vars(&self) -> &ZendHashTable { self.http_globals[TRACK_VARS_ENV as usize] .array() @@ -367,6 +423,11 @@ impl ProcessGlobals { } /// Get the HTTP Files variables. Equivalent of $_FILES. + /// + /// # Panics + /// + /// * If the files global is not found or fails to be populated. + #[must_use] pub fn http_files_vars(&self) -> &ZendHashTable { self.http_globals[TRACK_VARS_FILES as usize] .array() @@ -380,7 +441,7 @@ pub type SapiGlobals = sapi_globals_struct; impl SapiGlobals { /// Returns a reference to the PHP process globals. /// - /// The process globals are guarded by a RwLock. There can be multiple + /// The process globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release @@ -403,7 +464,7 @@ impl SapiGlobals { /// Returns a mutable reference to the PHP executor globals. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release @@ -425,11 +486,13 @@ impl SapiGlobals { } /// Get the request info for the Sapi. + #[must_use] pub fn request_info(&self) -> &SapiRequestInfo { &self.request_info } /// Get the sapi headers for the Sapi. + #[must_use] pub fn sapi_headers(&self) -> &SapiHeaders { &self.sapi_headers } @@ -450,7 +513,9 @@ impl<'a> SapiHeader { /// Get the header as a string. /// /// # Panics - /// - If the header is not a valid UTF-8 string. + /// + /// * If the header is not a valid UTF-8 string. + #[must_use] pub fn as_str(&'a self) -> &'a str { unsafe { let slice = slice::from_raw_parts(self.header as *const u8, self.header_len); @@ -459,13 +524,15 @@ impl<'a> SapiHeader { } /// Returns the header name (key). + #[must_use] pub fn name(&'a self) -> &'a str { self.as_str().split(':').next().unwrap_or("").trim() } /// Returns the header value. + #[must_use] pub fn value(&'a self) -> Option<&'a str> { - self.as_str().split(':').nth(1).map(|s| s.trim()) + self.as_str().split(':').nth(1).map(str::trim) } } @@ -473,6 +540,7 @@ pub type SapiRequestInfo = sapi_request_info; impl SapiRequestInfo { /// Get the request method. + #[must_use] pub fn request_method(&self) -> Option<&str> { if self.request_method.is_null() { return None; @@ -481,6 +549,7 @@ impl SapiRequestInfo { } /// Get the query string. + #[must_use] pub fn query_string(&self) -> Option<&str> { if self.query_string.is_null() { return None; @@ -489,6 +558,7 @@ impl SapiRequestInfo { } /// Get the cookie data. + #[must_use] pub fn cookie_data(&self) -> Option<&str> { if self.cookie_data.is_null() { return None; @@ -497,11 +567,13 @@ impl SapiRequestInfo { } /// Get the content length. + #[must_use] pub fn content_length(&self) -> i64 { self.content_length } /// Get the path info. + #[must_use] pub fn path_translated(&self) -> Option<&str> { if self.path_translated.is_null() { return None; @@ -510,6 +582,7 @@ impl SapiRequestInfo { } /// Get the request uri. + #[must_use] pub fn request_uri(&self) -> Option<&str> { if self.request_uri.is_null() { return None; @@ -520,6 +593,7 @@ impl SapiRequestInfo { // Todo: request_body _php_stream /// Get the content type. + #[must_use] pub fn content_type(&self) -> Option<&str> { if self.content_type.is_null() { return None; @@ -528,16 +602,19 @@ impl SapiRequestInfo { } /// Whether the request consists of headers only. + #[must_use] pub fn headers_only(&self) -> bool { self.headers_only } /// Whether the request has no headers. + #[must_use] pub fn no_headers(&self) -> bool { self.no_headers } /// Whether the request headers have been read. + #[must_use] pub fn headers_read(&self) -> bool { self.headers_read } @@ -545,6 +622,7 @@ impl SapiRequestInfo { // Todo: post_entry sapi_post_entry /// Get the auth user. + #[must_use] pub fn auth_user(&self) -> Option<&str> { if self.auth_user.is_null() { return None; @@ -553,6 +631,7 @@ impl SapiRequestInfo { } /// Get the auth password. + #[must_use] pub fn auth_password(&self) -> Option<&str> { if self.auth_password.is_null() { return None; @@ -561,6 +640,7 @@ impl SapiRequestInfo { } /// Get the auth digest. + #[must_use] pub fn auth_digest(&self) -> Option<&str> { if self.auth_digest.is_null() { return None; @@ -569,6 +649,7 @@ impl SapiRequestInfo { } /// Get argv0. + #[must_use] pub fn argv0(&self) -> Option<&str> { if self.argv0.is_null() { return None; @@ -577,6 +658,7 @@ impl SapiRequestInfo { } /// Get the current user. + #[must_use] pub fn current_user(&self) -> Option<&str> { if self.current_user.is_null() { return None; @@ -585,16 +667,19 @@ impl SapiRequestInfo { } /// Get the current user length. + #[must_use] pub fn current_user_length(&self) -> i32 { self.current_user_length } /// Get argvc. + #[must_use] pub fn argvc(&self) -> i32 { self.argc } /// Get argv. + #[must_use] pub fn argv(&self) -> Option<&str> { if self.argv.is_null() { return None; @@ -603,6 +688,7 @@ impl SapiRequestInfo { } /// Get the protocol number. + #[must_use] pub fn proto_num(&self) -> i32 { self.proto_num } @@ -614,11 +700,15 @@ pub type FileGlobals = php_file_globals; impl FileGlobals { /// Returns a reference to the PHP process globals. /// - /// The process globals are guarded by a RwLock. There can be multiple + /// The process globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release /// the lock. + /// + /// # Panics + /// + /// * If static file globals are not set pub fn get() -> GlobalReadGuard { // SAFETY: PHP executor globals are statically declared therefore should never // return an invalid pointer. @@ -638,7 +728,7 @@ impl FileGlobals { /// Returns a mutable reference to the PHP executor globals. /// - /// The executor globals are guarded by a RwLock. There can be multiple + /// The executor globals are guarded by a [`RwLock`]. There can be multiple /// immutable references at one time but only ever one mutable reference. /// Attempting to retrieve the globals while already holding the global /// guard will lead to a deadlock. Dropping the globals guard will release @@ -660,6 +750,7 @@ impl FileGlobals { } /// Returns the stream wrappers + #[must_use] pub fn stream_wrappers(&self) -> Option<&'static ZendHashTable> { unsafe { self.stream_wrappers.as_ref() } } diff --git a/src/zend/handlers.rs b/src/zend/handlers.rs index fe31a438a0..c788145efd 100644 --- a/src/zend/handlers.rs +++ b/src/zend/handlers.rs @@ -18,6 +18,7 @@ pub type ZendObjectHandlers = zend_object_handlers; impl ZendObjectHandlers { /// Creates a new set of object handlers based on the standard object /// handlers. + #[must_use] pub fn new() -> ZendObjectHandlers { let mut this = MaybeUninit::uninit(); @@ -40,10 +41,14 @@ impl ZendObjectHandlers { /// # Safety /// /// Caller must guarantee that the `ptr` given is a valid memory location. + /// + /// # Panics + /// + /// * If the offset of the `T` type is not a valid `i32` value. pub unsafe fn init(ptr: *mut ZendObjectHandlers) { std::ptr::copy_nonoverlapping(&std_object_handlers, ptr, 1); let offset = ZendClassObject::::std_offset(); - (*ptr).offset = offset as _; + (*ptr).offset = offset.try_into().expect("Invalid offset"); (*ptr).free_obj = Some(Self::free_obj::); (*ptr).read_property = Some(Self::read_property::); (*ptr).write_property = Some(Self::write_property::); @@ -60,7 +65,7 @@ impl ZendObjectHandlers { // Manually drop the object as we don't want to free the underlying memory. ptr::drop_in_place(&mut obj.obj); - zend_object_std_dtor(object) + zend_object_std_dtor(object); } unsafe extern "C" fn read_property( @@ -70,6 +75,8 @@ impl ZendObjectHandlers { cache_slot: *mut *mut c_void, rv: *mut Zval, ) -> *mut Zval { + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] unsafe fn internal( object: *mut ZendObject, @@ -118,6 +125,8 @@ impl ZendObjectHandlers { value: *mut Zval, cache_slot: *mut *mut c_void, ) -> *mut Zval { + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] unsafe fn internal( object: *mut ZendObject, @@ -158,6 +167,8 @@ impl ZendObjectHandlers { unsafe extern "C" fn get_properties( object: *mut ZendObject, ) -> *mut ZendHashTable { + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] unsafe fn internal( object: *mut ZendObject, @@ -201,6 +212,8 @@ impl ZendObjectHandlers { has_set_exists: c_int, cache_slot: *mut *mut c_void, ) -> c_int { + // TODO: Measure this + #[allow(clippy::inline_always)] #[inline(always)] unsafe fn internal( object: *mut ZendObject, @@ -264,7 +277,7 @@ impl ZendObjectHandlers { "Invalid value given for `has_set_exists` in struct `has_property` function." .into(), ), - }; + } Ok(zend_std_has_property( object, diff --git a/src/zend/ini_entry_def.rs b/src/zend/ini_entry_def.rs index 838d6076f3..049f37fabd 100644 --- a/src/zend/ini_entry_def.rs +++ b/src/zend/ini_entry_def.rs @@ -7,30 +7,53 @@ use crate::{ffi::zend_ini_entry_def, ffi::zend_register_ini_entries, flags::IniE /// A Zend ini entry definition. /// -/// To register ini definitions for extensions, the IniEntryDef builder should -/// be used. Ini entries should be registered in your module's startup_function -/// via `IniEntryDef::register(Vec)`. +/// To register ini definitions for extensions, the [`IniEntryDef`] builder +/// should be used. Ini entries should be registered in your module's +/// `startup_function` via `IniEntryDef::register(Vec)`. pub type IniEntryDef = zend_ini_entry_def; impl IniEntryDef { - /// Returns an empty ini entry, signifying the end of a ini list. - pub fn new(name: String, default_value: String, permission: IniEntryPermission) -> Self { + /// Creates a new ini entry definition. + /// + /// # Panics + /// + /// * If the name or value cannot be converted to a C string + /// * If the name or value length is > `65_535` + /// * If the permission bits are invalid + #[must_use] + pub fn new(name: String, default_value: String, permission: &IniEntryPermission) -> Self { let mut template = Self::end(); let name = CString::new(name).expect("Unable to create CString from name"); let value = CString::new(default_value).expect("Unable to create CString from value"); - template.name_length = name.as_bytes().len() as _; + template.name_length = name + .as_bytes() + .len() + .try_into() + .expect("Invalid name length"); template.name = name.into_raw(); - template.value_length = value.as_bytes().len() as _; + template.value_length = value + .as_bytes() + .len() + .try_into() + .expect("Invalid value length"); template.value = value.into_raw(); - template.modifiable = IniEntryPermission::PerDir.bits() as _; - template.modifiable = permission.bits() as _; + // FIXME: Double assignment of modifiable + template.modifiable = IniEntryPermission::PerDir + .bits() + .try_into() + .expect("Invalid permission bits"); + template.modifiable = permission + .bits() + .try_into() + .expect("Invalid permission bits"); template } /// Returns an empty ini entry def, signifying the end of a ini list. + #[must_use] pub fn end() -> Self { Self { - name: ptr::null() as *const c_char, + name: ptr::null::(), on_modify: None, mh_arg1: std::ptr::null_mut(), mh_arg2: std::ptr::null_mut(), @@ -45,6 +68,7 @@ impl IniEntryDef { /// Converts the ini entry into a raw and pointer, releasing it to the /// C world. + #[must_use] pub fn into_raw(self) -> *mut Self { Box::into_raw(Box::new(self)) } diff --git a/src/zend/linked_list.rs b/src/zend/linked_list.rs index cc2ccb1133..b01ffce06f 100644 --- a/src/zend/linked_list.rs +++ b/src/zend/linked_list.rs @@ -1,4 +1,4 @@ -use std::marker::PhantomData; +use std::{marker::PhantomData, ptr}; use crate::ffi::{zend_llist, zend_llist_element, zend_llist_get_next_ex}; @@ -7,6 +7,7 @@ pub type ZendLinkedList = zend_llist; impl ZendLinkedList { /// Create an iterator over the linked list + #[must_use] pub fn iter(&self) -> ZendLinkedListIterator { ZendLinkedListIterator::new(self) } @@ -36,10 +37,10 @@ impl<'a, T: 'a> Iterator for ZendLinkedListIterator<'a, T> { return None; } let ptr = unsafe { (*self.position).data.as_mut_ptr() }; - let value = unsafe { &*(ptr as *const T as *mut T) }; + let value = unsafe { &*((ptr as *const T).cast_mut()) }; unsafe { zend_llist_get_next_ex( - self.list as *const ZendLinkedList as *mut ZendLinkedList, + ptr::from_ref::(self.list).cast_mut(), &mut self.position, ) }; diff --git a/src/zend/mod.rs b/src/zend/mod.rs index c7a74ac273..5ee46ebae5 100644 --- a/src/zend/mod.rs +++ b/src/zend/mod.rs @@ -36,7 +36,7 @@ pub use module::ModuleEntry; pub use streams::*; #[cfg(feature = "embed")] pub(crate) use try_catch::panic_wrapper; -pub use try_catch::{bailout, try_catch, try_catch_first}; +pub use try_catch::{bailout, try_catch, try_catch_first, CatchError}; // Used as the format string for `php_printf`. const FORMAT_STR: &[u8] = b"%s\0"; @@ -49,10 +49,9 @@ const FORMAT_STR: &[u8] = b"%s\0"; /// /// * message - The message to print to stdout. /// -/// # Returns +/// # Errors /// -/// Nothing on success, error if the message could not be converted to a -/// [`CString`]. +/// * If the message could not be converted to a [`CString`]. pub fn printf(message: &str) -> Result<()> { let message = CString::new(message)?; unsafe { @@ -62,6 +61,12 @@ pub fn printf(message: &str) -> Result<()> { } /// Get the name of the SAPI module. +/// +/// # Panics +/// +/// * If the module name is not a valid [`CStr`] +/// +/// [`CStr`]: std::ffi::CStr pub fn php_sapi_name() -> String { let c_str = unsafe { std::ffi::CStr::from_ptr(sapi_module.name) }; c_str.to_str().expect("Unable to parse CStr").to_string() diff --git a/src/zend/module.rs b/src/zend/module.rs index d86d50375a..4db6c303e9 100644 --- a/src/zend/module.rs +++ b/src/zend/module.rs @@ -9,6 +9,7 @@ pub type ModuleEntry = zend_module_entry; impl ModuleEntry { /// Allocates the module entry on the heap, returning a pointer to the /// memory location. The caller is responsible for the memory pointed to. + #[must_use] pub fn into_raw(self) -> *mut Self { Box::into_raw(Box::new(self)) } diff --git a/src/zend/streams.rs b/src/zend/streams.rs index 153f667491..d9fe342fac 100644 --- a/src/zend/streams.rs +++ b/src/zend/streams.rs @@ -31,6 +31,7 @@ pub type StreamOpener = unsafe extern "C" fn( impl StreamWrapper { /// Get wrapped stream by name + #[must_use] pub fn get(name: &str) -> Option<&Self> { unsafe { let result = php_stream_locate_url_wrapper(name.as_ptr().cast(), ptr::null_mut(), 0); @@ -39,6 +40,7 @@ impl StreamWrapper { } /// Get mutable wrapped stream by name + #[must_use] pub fn get_mut(name: &str) -> Option<&mut Self> { unsafe { let result = php_stream_locate_url_wrapper(name.as_ptr().cast(), ptr::null_mut(), 0); @@ -48,8 +50,14 @@ impl StreamWrapper { /// Register stream wrapper for name /// + /// # Errors + /// + /// * `Error::StreamWrapperRegistrationFailure` - If the stream wrapper + /// could not be registered + /// /// # Panics - /// - If the name cannot be converted to a C string + /// + /// * If the name cannot be converted to a C string pub fn register(self, name: &str) -> Result { // We have to convert it to a static so owned streamwrapper doesn't get dropped. let copy = Box::new(self); @@ -64,13 +72,18 @@ impl StreamWrapper { } /// Register volatile stream wrapper for name + /// + /// # Errors + /// + /// * `Error::StreamWrapperRegistrationFailure` - If the stream wrapper + /// could not be registered pub fn register_volatile(self, name: &str) -> Result { // We have to convert it to a static so owned streamwrapper doesn't get dropped. let copy = Box::new(self); let copy = Box::leak(copy); let name = ZendStr::new(name, false); let result = - unsafe { php_register_url_stream_wrapper_volatile((*name).as_ptr() as _, copy) }; + unsafe { php_register_url_stream_wrapper_volatile((*name).as_ptr().cast_mut(), copy) }; if result == 0 { Ok(*copy) } else { @@ -80,8 +93,14 @@ impl StreamWrapper { /// Unregister stream wrapper by name /// + /// # Errors + /// + /// * `Error::StreamWrapperUnregistrationFailure` - If the stream wrapper + /// could not be unregistered + /// /// # Panics - /// - If the name cannot be converted to a C string + /// + /// * If the name cannot be converted to a C string pub fn unregister(name: &str) -> Result<(), Error> { let name = std::ffi::CString::new(name).expect("Could not create C string for name!"); match unsafe { php_unregister_url_stream_wrapper(name.as_ptr()) } { @@ -91,22 +110,28 @@ impl StreamWrapper { } /// Unregister volatile stream wrapper by name + /// + /// # Errors + /// + /// * `Error::StreamWrapperUnregistrationFailure` - If the stream wrapper + /// could not be unregistered pub fn unregister_volatile(name: &str) -> Result<(), Error> { let name = ZendStr::new(name, false); - match unsafe { php_unregister_url_stream_wrapper_volatile((*name).as_ptr() as _) } { + match unsafe { php_unregister_url_stream_wrapper_volatile((*name).as_ptr().cast_mut()) } { 0 => Ok(()), _ => Err(Error::StreamWrapperUnregistrationFailure), } } /// Get the operations the stream wrapper can perform + #[must_use] pub fn wops(&self) -> &php_stream_wrapper_ops { unsafe { &*self.wops } } /// Get the mutable operations the stream can perform pub fn wops_mut(&mut self) -> &mut php_stream_wrapper_ops { - unsafe { &mut *(self.wops as *mut php_stream_wrapper_ops) } + unsafe { &mut *(self.wops.cast_mut()) } } } diff --git a/src/zend/try_catch.rs b/src/zend/try_catch.rs index 3af511b0e7..2871ed9987 100644 --- a/src/zend/try_catch.rs +++ b/src/zend/try_catch.rs @@ -5,6 +5,7 @@ use std::ffi::c_void; use std::panic::{catch_unwind, resume_unwind, RefUnwindSafe}; use std::ptr::null_mut; +/// Error returned when a bailout occurs #[derive(Debug)] pub struct CatchError; @@ -15,38 +16,44 @@ pub(crate) unsafe extern "C" fn panic_wrapper R + RefUnwindSafe // mandatory when we do assert on test as other test would not run correctly let panic = catch_unwind(|| (*(ctx as *mut F))()); - Box::into_raw(Box::new(panic)) as *mut c_void + Box::into_raw(Box::new(panic)).cast::() } -/// PHP propose a try catch mechanism in C using setjmp and longjmp (bailout) -/// It store the arg of setjmp into the bailout field of the global executor +/// PHP proposes a try catch mechanism in C using setjmp and longjmp (bailout) +/// It stores the arg of setjmp into the bailout field of the global executor /// If a bailout is triggered, the executor will jump to the setjmp and restore /// the previous setjmp /// -/// try_catch allow to use this mechanism +/// [`try_catch`] allows to use this mechanism /// /// # Returns /// -/// * `Ok(R)` - The result of the function -/// * `Err(CatchError)` - A bailout occurred during the execution +/// * The result of the function +/// +/// # Errors +/// +/// * [`CatchError`] - A bailout occurred during the execution pub fn try_catch R + RefUnwindSafe>(func: F) -> Result { do_try_catch(func, false) } -/// PHP propose a try catch mechanism in C using setjmp and longjmp (bailout) -/// It store the arg of setjmp into the bailout field of the global executor +/// PHP proposes a try catch mechanism in C using setjmp and longjmp (bailout) +/// It stores the arg of setjmp into the bailout field of the global executor /// If a bailout is triggered, the executor will jump to the setjmp and restore /// the previous setjmp /// -/// try_catch_first allow to use this mechanism +/// [`try_catch_first`] allows to use this mechanism /// -/// This functions differs from ['try_catch'] as it also initialize the bailout +/// This functions differs from [`try_catch`] as it also initialize the bailout /// mechanism for the first time /// /// # Returns /// -/// * `Ok(R)` - The result of the function -/// * `Err(CatchError)` - A bailout occurred during the execution +/// * The result of the function +/// +/// # Errors +/// +/// * [`CatchError`] - A bailout occurred during the execution pub fn try_catch_first R + RefUnwindSafe>(func: F) -> Result { do_try_catch(func, true) } @@ -57,26 +64,26 @@ fn do_try_catch R + RefUnwindSafe>(func: F, first: bool) -> Res if first { ext_php_rs_zend_first_try_catch( panic_wrapper::, - &func as *const F as *const c_void, - (&mut panic_ptr) as *mut *mut c_void, + (&raw const func).cast::(), + &raw mut panic_ptr, ) } else { ext_php_rs_zend_try_catch( panic_wrapper::, - &func as *const F as *const c_void, - (&mut panic_ptr) as *mut *mut c_void, + (&raw const func).cast::(), + &raw mut panic_ptr, ) } }; - let panic = panic_ptr as *mut std::thread::Result; + let panic = panic_ptr.cast::>(); // can be null if there is a bailout if panic.is_null() || has_bailout { return Err(CatchError); } - match unsafe { *Box::from_raw(panic as *mut std::thread::Result) } { + match unsafe { *Box::from_raw(panic.cast::>()) } { Ok(r) => Ok(r), Err(err) => { // we resume the panic here so it can be caught correctly by the test framework @@ -118,6 +125,7 @@ mod tests { } #[allow(unreachable_code)] + #[allow(clippy::assertions_on_constants)] { assert!(false); } @@ -131,7 +139,10 @@ mod tests { fn test_no_catch() { Embed::run(|| { let catch = try_catch(|| { - assert!(true); + #[allow(clippy::assertions_on_constants)] + { + assert!(true); + } }); assert!(catch.is_ok()); @@ -146,6 +157,7 @@ mod tests { } #[allow(unreachable_code)] + #[allow(clippy::assertions_on_constants)] { assert!(false); } @@ -153,7 +165,7 @@ mod tests { } #[test] - #[should_panic] + #[should_panic(expected = "should panic")] fn test_panic() { Embed::run(|| { let _ = try_catch(|| { @@ -165,12 +177,11 @@ mod tests { #[test] fn test_return() { let foo = Embed::run(|| { - let result = try_catch(|| { - return "foo"; - }); + let result = try_catch(|| "foo"); assert!(result.is_ok()); + #[allow(clippy::unwrap_used)] result.unwrap() }); diff --git a/tests/module.rs b/tests/module.rs index 7131ef6fe8..95c2935aaf 100644 --- a/tests/module.rs +++ b/tests/module.rs @@ -1,5 +1,11 @@ +//! Module tests #![cfg_attr(windows, feature(abi_vectorcall))] #![cfg(feature = "embed")] +#![allow( + missing_docs, + clippy::needless_pass_by_value, + clippy::must_use_candidate +)] extern crate ext_php_rs; use cfg_if::cfg_if; @@ -42,7 +48,7 @@ fn test_module() { /// @return string Nice greeting! #[php_function] pub fn hello_world(name: String) -> String { - format!("Hello, {}!", name) + format!("Hello, {name}!") } #[php_module] diff --git a/tests/sapi.rs b/tests/sapi.rs index def0bc7578..d74accb894 100644 --- a/tests/sapi.rs +++ b/tests/sapi.rs @@ -1,5 +1,11 @@ +//! Sapi Tests #![cfg_attr(windows, feature(abi_vectorcall))] #![cfg(feature = "embed")] +#![allow( + missing_docs, + clippy::needless_pass_by_value, + clippy::must_use_candidate +)] extern crate ext_php_rs; use ext_php_rs::builders::SapiBuilder; @@ -15,10 +21,10 @@ use std::ffi::c_char; static mut LAST_OUTPUT: String = String::new(); extern "C" fn output_tester(str: *const c_char, str_length: usize) -> usize { - let char = unsafe { std::slice::from_raw_parts(str as *const u8, str_length) }; + let char = unsafe { std::slice::from_raw_parts(str.cast::(), str_length) }; let string = String::from_utf8_lossy(char); - println!("{}", string); + println!("{string}"); unsafe { LAST_OUTPUT = string.to_string(); @@ -89,7 +95,7 @@ fn test_sapi() { /// @return string Nice greeting! #[php_function] pub fn hello_world(name: String) -> String { - format!("Hello, {}!", name) + format!("Hello, {name}!") } #[php_module] diff --git a/tests/src/lib.rs b/tests/src/lib.rs index ea783476f1..b0b6084edc 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -1,4 +1,10 @@ #![cfg_attr(windows, feature(abi_vectorcall))] +#![allow( + clippy::must_use_candidate, + clippy::missing_panics_doc, + clippy::needless_pass_by_value, + clippy::implicit_hasher +)] use ext_php_rs::{ binary::Binary, boxed::ZBox, @@ -177,7 +183,7 @@ pub fn test_variadic_add_required(number: u32, numbers: &[&Zval]) -> u32 { number + numbers .iter() - .map(|x| x.long().unwrap() as u32) + .map(|x| u32::try_from(x.long().unwrap()).unwrap()) .sum::() } @@ -283,7 +289,7 @@ impl MagicMethod { pub fn __set(&mut self, prop_name: String, val: &Zval) { if val.is_long() && prop_name == "count" { - self.0 = val.long().unwrap() + self.0 = val.long().unwrap(); } } @@ -385,7 +391,7 @@ mod integration { .arg("-dassert.active=1") .arg("-dassert.exception=1") .arg("-dzend.assertions=1") - .arg(format!("src/integration/{}", file)) + .arg(format!("src/integration/{file}")) .output() .expect("failed to run php file"); if output.status.success() { diff --git a/unix_build.rs b/unix_build.rs index 8be6fd8be9..c41afb1dc7 100644 --- a/unix_build.rs +++ b/unix_build.rs @@ -8,8 +8,8 @@ pub struct Provider {} impl Provider { /// Runs `php-config` with one argument, returning the stdout. - fn php_config(&self, arg: &str) -> Result { - let cmd = Command::new(self.find_bin()?) + fn php_config(arg: &str) -> Result { + let cmd = Command::new(Self::find_bin()?) .arg(arg) .output() .context("Failed to run `php-config`")?; @@ -21,7 +21,7 @@ impl Provider { Ok(stdout.to_string()) } - fn find_bin(&self) -> Result { + fn find_bin() -> Result { // If path is given via env, it takes priority. if let Some(path) = path_from_env("PHP_CONFIG") { if !path.try_exists()? { @@ -44,8 +44,7 @@ impl<'a> PHPProvider<'a> for Provider { } fn get_includes(&self) -> Result> { - Ok(self - .php_config("--includes")? + Ok(Self::php_config("--includes")? .split(' ') .map(|s| s.trim_start_matches("-I")) .map(PathBuf::from)