style(clippy): apply pedantic rules

Refs: #418

Author: Xenira

.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

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<Self>;
 
     /// Retrieve a list of absolute include paths.
+    #[allow(clippy::missing_errors_doc)]
     fn get_includes(&self) -> Result<Vec<PathBuf>>;
 
     /// Retrieve a list of macro definitions to pass to the compiler.
+    #[allow(clippy::missing_errors_doc)]
     fn get_defines(&self) -> Result<Vec<(&'static str, &'static str)>>;
 
     /// 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<PathBuf> {
     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<PathBuf> {
     }
 }
 
-/// 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<PathBuf> {
     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<Self> {
         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<bool> {
         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<bool> {
         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<u32> {
         self.get_key("PHP API")
             .context("Failed to get Zend version")
@@ -202,7 +224,7 @@ fn generate_bindings(defines: &[(&str, &str)], includes: &[PathBuf]) -> Result<S
         .layout_tests(env::var("EXT_PHP_RS_TEST").is_ok())
         .rust_target(RustTarget::Nightly);
 
-    for binding in ALLOWED_BINDINGS.iter() {
+    for binding in ALLOWED_BINDINGS {
         bindgen = bindgen
             .allowlist_function(binding)
             .allowlist_type(binding)
@@ -230,6 +252,11 @@ fn generate_bindings(defines: &[(&str, &str)], includes: &[PathBuf]) -> Result<S
 /// Checks the PHP Zend API version for compatibility with ext-php-rs, setting
 /// any configuration flags required.
 fn check_php_version(info: &PHPInfo) -> 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)"
     );

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");
 }

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<PathBuf>,
     /// 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<PathBuf> {
                 "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<PathBuf>) -> AResult<cargo_metadata::Target> {
+fn find_ext(manifest: Option<&PathBuf>) -> AResult<cargo_metadata::Target> {
     // 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<Utf8PathBuf> {
                 }
             }
             cargo_metadata::Message::BuildFinished(b) => {
-                if !b.success {
-                    bail!("Compilation failed, cancelling installation.")
-                } else {
+                if b.success {
                     break;
                 }
+
+                bail!("Compilation failed, cancelling installation.")
             }
-            _ => continue,
+            _ => {}
         }
     }
 

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<TokenStream> {
     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<TokenStream> {
                 let pat = &arg.pat;
                 Some(quote! { &#pat })
             }
-            _ => None,
+            syn::FnArg::Receiver(_) => None,
         })
         .collect::<Option<Punctuated<_, Token![,]>>>()
         .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, &params);
 
     Ok(quote! {
         #(#attrs)* #vis #sig {
@@ -60,7 +60,7 @@ fn parse_function(mut func: ForeignItemFn) -> Result<TokenStream> {
 fn build_return(
     name: &str,
     return_type: &ReturnType,
-    params: Punctuated<TokenStream, Token![,]>,
+    params: &Punctuated<TokenStream, Token![,]>,
 ) -> TokenStream {
     match return_type {
         ReturnType::Default => quote! {