fix(path): propagate PathBuf

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

7 files changed, 88 insertions, 82 deletions

diff --git a/crates/shirabe/src/cache.rs b/crates/shirabe/src/cache.rs
index 532de41..315ba2c 100644
--- a/crates/shirabe/src/cache.rs
+++ b/crates/shirabe/src/cache.rs
@@ -325,14 +325,14 @@ impl Cache {
                 expire.format(date_format_to_strftime("Y-m-d H:i:s"))
             ));
             for file in &mut finder {
-                let _ = self.filesystem.borrow_mut().unlink(&file.get_pathname());
+                let _ = self.filesystem.borrow_mut().unlink(&file);
             }
 
             let mut total_size = self.filesystem.borrow_mut().size(&self.root).unwrap_or(0);
             if total_size > max_size {
                 let mut iterator = self.get_finder().sort_by_accessed_time().get_iterator();
                 while total_size > max_size && iterator.valid() {
-                    let filepath = iterator.current().get_pathname();
+                    let filepath = iterator.current();
                     total_size -= self.filesystem.borrow_mut().size(&filepath).unwrap_or(0);
                     let _ = self.filesystem.borrow_mut().unlink(&filepath);
                     iterator.next();
@@ -361,10 +361,7 @@ impl Cache {
                     expire.format(date_format_to_strftime("Y-m-d H:i:s"))
                 ));
             for file in &mut finder {
-                let _ = self
-                    .filesystem
-                    .borrow_mut()
-                    .remove_directory(&file.get_pathname());
+                let _ = self.filesystem.borrow_mut().remove_directory(&file);
             }
 
             *CACHE_COLLECTED.lock().unwrap() = Some(true);
diff --git a/crates/shirabe/src/command/create_project_command.rs b/crates/shirabe/src/command/create_project_command.rs
index c3c82d4..8432040 100644
--- a/crates/shirabe/src/command/create_project_command.rs
+++ b/crates/shirabe/src/command/create_project_command.rs
@@ -14,6 +14,7 @@ use shirabe_php_shim::{
     is_dir, is_file, mkdir, realpath, rtrim, strtolower, unlink,
 };
 use std::cell::RefCell;
+use std::path::PathBuf;
 use std::rc::Rc;
 
 use crate::advisory::AuditConfig;
@@ -559,14 +560,14 @@ impl CreateProjectCommand {
             }
 
             // PHP: try { $dirs = iterator_to_array($finder); ... } catch (\Exception $e) { ... }
-            let dirs: Vec<String> = finder.iter().map(|f| f.get_pathname()).collect();
+            let dirs: Vec<PathBuf> = finder.iter().collect();
             drop(finder);
             let mut had_error: Option<anyhow::Error> = None;
             for dir in &dirs {
                 if !fs.remove_directory(dir)? {
                     had_error = Some(
                         RuntimeException {
-                            message: format!("Could not remove {}", dir),
+                            message: format!("Could not remove {}", dir.display()),
                             code: 0,
                         }
                         .into(),
diff --git a/crates/shirabe/src/downloader/archive_downloader.rs b/crates/shirabe/src/downloader/archive_downloader.rs
index 9003e63..e8ce9b3 100644
--- a/crates/shirabe/src/downloader/archive_downloader.rs
+++ b/crates/shirabe/src/downloader/archive_downloader.rs
@@ -2,11 +2,12 @@
 
 use anyhow::Result;
 use indexmap::IndexMap;
-use shirabe_external_packages::symfony::finder::{Finder, SplFileInfo};
+use shirabe_external_packages::symfony::finder::Finder;
 use shirabe_php_shim::{
-    DIRECTORY_SEPARATOR, PhpMixed, RuntimeException, basename, bin2hex, file_exists, is_dir,
-    random_bytes, realpath,
+    DIRECTORY_SEPARATOR, PhpMixed, RuntimeException, bin2hex, file_exists, is_dir, random_bytes,
+    realpath,
 };
+use std::path::{Path, PathBuf};
 
 use crate::dependency_resolver::operation::InstallOperation;
 use crate::downloader::DownloaderInterface;
@@ -167,15 +168,15 @@ pub trait ArchiveDownloader {
                 let single_dir_at_top_level = content_dir.len() == 1
                     && content_dir
                         .first()
-                        .map(|file| is_dir(&file.get_pathname()))
+                        .map(|file| is_dir(file))
                         .unwrap_or(false);
 
                 if rename_as_one {
                     // if the target $path is clear, we can rename the whole package in one go instead of looping over the contents
-                    let extracted_dir = if single_dir_at_top_level {
-                        content_dir.first().unwrap().get_pathname()
+                    let extracted_dir: PathBuf = if single_dir_at_top_level {
+                        content_dir.first().unwrap().clone()
                     } else {
-                        temporary_dir.clone()
+                        PathBuf::from(&temporary_dir)
                     };
                     self.inner()
                         .filesystem
@@ -183,12 +184,17 @@ pub trait ArchiveDownloader {
                         .rename(&extracted_dir, path)?;
                 } else {
                     // only one dir in the archive, extract its contents out of it
-                    let mut from = temporary_dir.clone();
+                    let mut from = PathBuf::from(&temporary_dir);
                     if single_dir_at_top_level {
-                        from = content_dir.first().unwrap().get_pathname();
+                        from = content_dir.first().unwrap().clone();
                     }
 
-                    rename_recursively(&self.inner().filesystem, package.clone(), &from, path)?;
+                    rename_recursively(
+                        &self.inner().filesystem,
+                        package.clone(),
+                        &from,
+                        Path::new(path),
+                    )?;
                 }
 
                 self.inner()
@@ -242,14 +248,14 @@ fn install_cleanup(
 }
 
 /// Returns the folder content, excluding .DS_Store
-fn get_folder_content(dir: &str) -> Vec<SplFileInfo> {
+fn get_folder_content(dir: impl AsRef<Path>) -> Vec<PathBuf> {
     let mut finder = Finder::create();
     finder
         .ignore_vcs(false)
         .ignore_dot_files(false)
         .not_name(".DS_Store")
         .depth(0)
-        .r#in(dir);
+        .r#in(dir.as_ref());
 
     finder.iter().collect()
 }
@@ -262,37 +268,32 @@ fn get_folder_content(dir: &str) -> Vec<SplFileInfo> {
 fn rename_recursively(
     filesystem: &std::rc::Rc<std::cell::RefCell<Filesystem>>,
     package: PackageInterfaceHandle,
-    from: &str,
-    to: &str,
+    from: &Path,
+    to: &Path,
 ) -> Result<()> {
     let content_dir = get_folder_content(from);
 
     // move files back out of the temp dir
     for file in &content_dir {
-        let file = file.get_pathname();
-        if is_dir(&format!("{}/{}", to, basename(&file))) {
-            if !is_dir(&file) {
+        let target = to.join(
+            file.file_name()
+                .expect("Finder always yields entries with a file name"),
+        );
+        if is_dir(&target) {
+            if !is_dir(file) {
                 return Err(RuntimeException {
                     message: format!(
-                        "Installing {} would lead to overwriting the {}/{} directory with a file from the package, invalid operation.",
+                        "Installing {} would lead to overwriting the {} directory with a file from the package, invalid operation.",
                         package,
-                        to,
-                        basename(&file)
+                        target.display()
                     ),
                     code: 0,
                 }
                 .into());
             }
-            rename_recursively(
-                filesystem,
-                package.clone(),
-                &file,
-                &format!("{}/{}", to, basename(&file)),
-            )?;
+            rename_recursively(filesystem, package.clone(), file, &target)?;
         } else {
-            filesystem
-                .borrow_mut()
-                .rename(&file, &format!("{}/{}", to, basename(&file)))?;
+            filesystem.borrow_mut().rename(file, &target)?;
         }
     }
 
diff --git a/crates/shirabe/src/downloader/file_downloader.rs b/crates/shirabe/src/downloader/file_downloader.rs
index 4c30f9e..702a58f 100644
--- a/crates/shirabe/src/downloader/file_downloader.rs
+++ b/crates/shirabe/src/downloader/file_downloader.rs
@@ -8,9 +8,9 @@ use std::sync::{LazyLock, Mutex};
 use crate::util::Silencer;
 use shirabe_php_shim::{
     DIRECTORY_SEPARATOR, InvalidArgumentException, PATHINFO_BASENAME, PATHINFO_EXTENSION,
-    PHP_URL_PATH, PhpMixed, RuntimeException, UnexpectedValueException, array_search, array_shift,
-    file_exists, filesize, get_class, hash, hash_file, in_array, is_dir, is_executable, parse_url,
-    pathinfo, realpath, rtrim, spl_object_hash, strlen, strpos, strtr, trim, umask, usleep,
+    PHP_URL_PATH, PhpMixed, RuntimeException, UnexpectedValueException, array_search, file_exists,
+    filesize, get_class, hash, hash_file, in_array, is_dir, is_executable, parse_url, pathinfo,
+    realpath, rtrim, spl_object_hash, strlen, strpos, strtr, trim, umask, usleep,
 };
 
 use crate::cache::Cache;
@@ -801,17 +801,3 @@ struct UrlEntry {
     processed: String,
     cache_key: String,
 }
-
-// Suppress unused-import warnings for items kept for parity with the PHP source.
-#[allow(dead_code)]
-fn _use_parity() {
-    let _ = filesize;
-    let _ = hash_file;
-    let _ = in_array;
-    let _ = usleep;
-    let _ = array_shift::<u8>;
-    let _ = UnexpectedValueException {
-        message: String::new(),
-        code: 0,
-    };
-}
diff --git a/crates/shirabe/src/package/archiver/archivable_files_finder.rs b/crates/shirabe/src/package/archiver/archivable_files_finder.rs
index cb1c3e5..08f8df1 100644
--- a/crates/shirabe/src/package/archiver/archivable_files_finder.rs
+++ b/crates/shirabe/src/package/archiver/archivable_files_finder.rs
@@ -77,11 +77,7 @@ impl ArchivableFilesFinder {
             .ignore_dot_files(false)
             .sort_by_name();
 
-        let inner_iter: Box<dyn Iterator<Item = PathBuf>> = Box::new(
-            finder
-                .get_iterator()
-                .map(|f| PathBuf::from(f.get_pathname())),
-        );
+        let inner_iter: Box<dyn Iterator<Item = PathBuf>> = Box::new(finder.get_iterator());
 
         Ok(Self { finder, inner_iter })
     }
diff --git a/crates/shirabe/src/package/locker.rs b/crates/shirabe/src/package/locker.rs
index 8910645..8237d76 100644
--- a/crates/shirabe/src/package/locker.rs
+++ b/crates/shirabe/src/package/locker.rs
@@ -7,9 +7,8 @@ use shirabe_external_packages::composer::pcre::{CaptureKey, Preg};
 use shirabe_external_packages::seld::json_lint::ParsingException;
 use shirabe_php_shim::{
     DATE_RFC3339, LogicException, PhpMixed, RuntimeException, array_intersect, array_keys,
-    array_map, array_merge, call_user_func, file_get_contents, filemtime, function_exists, hash,
-    in_array, is_array, is_int, ksort, realpath, reset_first, sprintf, strcmp, strtolower, touch2,
-    trim, usort,
+    array_map, array_merge, file_get_contents, filemtime, function_exists, hash, in_array, is_int,
+    ksort, realpath, reset_first, sprintf, strcmp, strtolower, touch2, trim, usort,
 };
 
 use crate::installer::InstallationManager;
@@ -1006,10 +1005,3 @@ struct SetEntry {
     method: String,
     description: String,
 }
-
-// Suppress unused-import warnings for items kept for parity with the PHP source.
-#[allow(dead_code)]
-fn _use_parity() {
-    let _ = is_array;
-    let _: PhpMixed = call_user_func("", &[]);
-}
diff --git a/crates/shirabe/src/util/filesystem.rs b/crates/shirabe/src/util/filesystem.rs
index 6ca6be0..2d73422 100644
--- a/crates/shirabe/src/util/filesystem.rs
+++ b/crates/shirabe/src/util/filesystem.rs
@@ -14,6 +14,8 @@ use shirabe_php_shim::{
     substr_count, symlink, touch, unlink, usleep, var_export,
 };
 
+use std::path::Path;
+
 use crate::util::Platform;
 use crate::util::ProcessExecutor;
 use crate::util::Silencer;
@@ -30,7 +32,8 @@ impl Filesystem {
         }
     }
 
-    pub fn remove(&mut self, file: &str) -> anyhow::Result<bool> {
+    pub fn remove(&mut self, file: impl AsRef<Path>) -> anyhow::Result<bool> {
+        let file = file.as_ref();
         if is_dir(file) {
             return self.remove_directory(file);
         }
@@ -76,7 +79,7 @@ impl Filesystem {
                 .r#in(dir);
 
             for path in finder.iter() {
-                self.remove(&path.get_pathname())?;
+                self.remove(path)?;
             }
         }
         Ok(())
@@ -86,7 +89,16 @@ impl Filesystem {
     ///
     /// Uses the process component if proc_open is enabled on the PHP
     /// installation.
-    pub fn remove_directory(&mut self, directory: &str) -> anyhow::Result<bool> {
+    pub fn remove_directory(&mut self, directory: impl AsRef<Path>) -> anyhow::Result<bool> {
+        // TODO(phase-c):
+        // This path is matched against a regex (remove_edge_cases) and passed to an
+        // `rm -rf`/`rmdir` subprocess via the String-based ProcessExecutor, so it has to be
+        // representable as UTF-8.
+        let directory = directory.as_ref();
+        let directory = directory.to_str().ok_or_else(|| RuntimeException {
+            message: format!("Path contains invalid UTF-8: {}", directory.display()),
+            code: 0,
+        })?;
         let edge_case_result = self.remove_edge_cases(directory, true)?;
         if let Some(r) = edge_case_result {
             return Ok(r);
@@ -256,7 +268,7 @@ impl Filesystem {
                 .into());
             }
 
-            if is_link(directory) && !self.unlink_implementation(directory) {
+            if is_link(directory) && !self.unlink_implementation(Path::new(directory)) {
                 return Err(RuntimeException {
                     message: format!(
                         "Could not delete symbolic link {}: {}",
@@ -305,7 +317,8 @@ impl Filesystem {
     }
 
     /// Attempts to unlink a file and in case of failure retries after 350ms on windows
-    pub fn unlink(&self, path: &str) -> anyhow::Result<bool> {
+    pub fn unlink(&self, path: impl AsRef<Path>) -> anyhow::Result<bool> {
+        let path = path.as_ref();
         let mut unlinked = self.unlink_implementation(path);
         if !unlinked {
             // retry after a bit on windows since it tends to be touchy with mass removals
@@ -318,7 +331,7 @@ impl Filesystem {
                 let error = error_get_last();
                 let mut message = format!(
                     "Could not delete {}: {}",
-                    path,
+                    path.display(),
                     error
                         .as_ref()
                         .and_then(|m| m.get("message"))
@@ -337,7 +350,8 @@ impl Filesystem {
     }
 
     /// Attempts to rmdir a file and in case of failure retries after 350ms on windows
-    pub fn rmdir(&self, path: &str) -> anyhow::Result<bool> {
+    pub fn rmdir(&self, path: impl AsRef<Path>) -> anyhow::Result<bool> {
+        let path = path.as_ref();
         let mut deleted = rmdir(path);
         if !deleted {
             // retry after a bit on windows since it tends to be touchy with mass removals
@@ -350,7 +364,7 @@ impl Filesystem {
                 let error = error_get_last();
                 let mut message = format!(
                     "Could not delete {}: {}",
-                    path,
+                    path.display(),
                     error
                         .as_ref()
                         .and_then(|m| m.get("message"))
@@ -443,11 +457,29 @@ impl Filesystem {
         Ok(result)
     }
 
-    pub fn rename(&mut self, source: &str, target: &str) -> anyhow::Result<()> {
+    pub fn rename(
+        &mut self,
+        source: impl AsRef<Path>,
+        target: impl AsRef<Path>,
+    ) -> anyhow::Result<()> {
+        let source = source.as_ref();
+        let target = target.as_ref();
         if rename(source, target) {
             return Ok(());
         }
 
+        // TODO(phase-c):
+        // The fallbacks below (copy_then_remove and the mv/xcopy subprocesses) operate on
+        // path strings, so beyond this point the paths have to be representable as UTF-8.
+        let source = source.to_str().ok_or_else(|| RuntimeException {
+            message: format!("Path contains invalid UTF-8: {}", source.display()),
+            code: 0,
+        })?;
+        let target = target.to_str().ok_or_else(|| RuntimeException {
+            message: format!("Path contains invalid UTF-8: {}", target.display()),
+            code: 0,
+        })?;
+
         if !function_exists("proc_open") {
             return self.copy_then_remove(source, target);
         }
@@ -651,10 +683,11 @@ impl Filesystem {
 
     /// Returns size of a file or directory specified by path. If a directory is
     /// given, its size will be computed recursively.
-    pub fn size(&self, path: &str) -> anyhow::Result<i64> {
+    pub fn size(&self, path: impl AsRef<Path>) -> anyhow::Result<i64> {
+        let path = path.as_ref();
         if !file_exists(path) {
             return Err(RuntimeException {
-                message: format!("{} does not exist.", path),
+                message: format!("{} does not exist.", path.display()),
                 code: 0,
             }
             .into());
@@ -795,7 +828,7 @@ impl Filesystem {
         false
     }
 
-    pub(crate) fn directory_size(&self, directory: &str) -> anyhow::Result<i64> {
+    pub(crate) fn directory_size(&self, directory: &Path) -> anyhow::Result<i64> {
         let it =
             shirabe_php_shim::recursive_directory_iterator(directory, shirabe_php_shim::SKIP_DOTS)?;
         let ri = shirabe_php_shim::recursive_iterator_iterator(it, shirabe_php_shim::CHILD_FIRST);
@@ -823,7 +856,7 @@ impl Filesystem {
     /// delete symbolic link implementation (commonly known as "unlink()")
     ///
     /// symbolic links on windows which link to directories need rmdir instead of unlink
-    fn unlink_implementation(&self, path: &str) -> bool {
+    fn unlink_implementation(&self, path: &Path) -> bool {
         if Platform::is_windows() && is_dir(path) && is_link(path) {
             return rmdir(path);
         }