impl PathConverter

RustPython · youknowone · Dec 25, 2025 · Dec 24, 2025 · Dec 24, 2025 · Dec 24, 2025
commit fd1878435f39402fcede4d50ec209bbdafad38b2
diff --git a/crates/vm/src/function/fspath.rs b/crates/vm/src/function/fspath.rs
@@ -7,6 +7,7 @@ use crate::{
 };
 use std::{borrow::Cow, ffi::OsStr, path::PathBuf};
 
+/// Helper to implement os.fspath()
 #[derive(Clone)]
 pub enum FsPath {
     Str(PyStrRef),
@@ -27,7 +28,7 @@ impl FsPath {
         )
     }
 
-    // PyOS_FSPath in CPython
+    // PyOS_FSPath
     pub fn try_from(
         obj: PyObjectRef,
         check_for_nul: bool,

diff --git a/crates/vm/src/ospath.rs b/crates/vm/src/ospath.rs
@@ -2,22 +2,181 @@ use rustpython_common::crt_fd;
 
 use crate::{
     PyObjectRef, PyResult, VirtualMachine,
+    builtins::{PyBytes, PyStr},
     convert::{IntoPyException, ToPyException, ToPyObject, TryFromObject},
     function::FsPath,
 };
 use std::path::{Path, PathBuf};
 
-// path_ without allow_fd in CPython
+/// path_converter
+#[derive(Clone, Copy, Default)]
+pub struct PathConverter {
+    /// Function name for error messages (e.g., "rename")
+    pub function_name: Option<&'static str>,
+    /// Argument name for error messages (e.g., "src", "dst")
+    pub argument_name: Option<&'static str>,
+    /// If true, embedded null characters are allowed
+    pub non_strict: bool,
+}
+
+impl PathConverter {
+    pub const fn new() -> Self {
+        Self {
+            function_name: None,
+            argument_name: None,
+            non_strict: false,
+        }
+    }
+
+    pub const fn function(mut self, name: &'static str) -> Self {
+        self.function_name = Some(name);
+        self
+    }
+
+    pub const fn argument(mut self, name: &'static str) -> Self {
+        self.argument_name = Some(name);
+        self
+    }
+
+    pub const fn non_strict(mut self) -> Self {
+        self.non_strict = true;
+        self
+    }
+
+    /// Generate error message prefix like "rename: "
+    fn error_prefix(&self) -> String {
+        match self.function_name {
+            Some(func) => format!("{}: ", func),
+            None => String::new(),
+        }
+    }
+
+    /// Get argument name for error messages, defaults to "path"
+    fn arg_name(&self) -> &'static str {
+        self.argument_name.unwrap_or("path")
+    }
+
+    /// Format a type error message
+    fn type_error_msg(&self, type_name: &str, allow_fd: bool) -> String {
+        let expected = if allow_fd {
+            "string, bytes, os.PathLike or integer"
+        } else {
+            "string, bytes or os.PathLike"
+        };
+        format!(
+            "{}{} should be {}, not {}",
+            self.error_prefix(),
+            self.arg_name(),
+            expected,
+            type_name
+        )
+    }
+
+    /// Convert to OsPathOrFd (path or file descriptor)
+    pub(crate) fn try_path_or_fd<'fd>(
+        &self,
+        obj: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<OsPathOrFd<'fd>> {
+        // Handle fd (before __fspath__ check, like CPython)
+        if let Some(int) = obj.try_index_opt(vm) {
+            let fd = int?.try_to_primitive(vm)?;
+            return unsafe { crt_fd::Borrowed::try_borrow_raw(fd) }
+                .map(OsPathOrFd::Fd)
+                .map_err(|e| e.into_pyexception(vm));
+        }
+
+        self.try_path_inner(obj, true, vm).map(OsPathOrFd::Path)
+    }
+
+    /// Convert to OsPath only (no fd support)
+    fn try_path_inner(
+        &self,
+        obj: PyObjectRef,
+        allow_fd: bool,
+        vm: &VirtualMachine,
+    ) -> PyResult<OsPath> {
+        // Try direct str/bytes match
+        let obj = match self.try_match_str_bytes(obj.clone(), vm)? {
+            Ok(path) => return Ok(path),
+            Err(obj) => obj,
+        };
+
+        // Call __fspath__
+        let type_error_msg = || self.type_error_msg(&obj.class().name(), allow_fd);
+        let method =
+            vm.get_method_or_type_error(obj.clone(), identifier!(vm, __fspath__), type_error_msg)?;
+        if vm.is_none(&method) {
+            return Err(vm.new_type_error(type_error_msg()));
+        }
+        let result = method.call((), vm)?;
+
+        // Match __fspath__ result
+        self.try_match_str_bytes(result.clone(), vm)?.map_err(|_| {
+            vm.new_type_error(format!(
+                "{}expected {}.__fspath__() to return str or bytes, not {}",
+                self.error_prefix(),
+                obj.class().name(),
+                result.class().name(),
+            ))
+        })
+    }
+
+    /// Try to match str or bytes, returns Err(obj) if neither
+    fn try_match_str_bytes(
+        &self,
+        obj: PyObjectRef,
+        vm: &VirtualMachine,
+    ) -> PyResult<Result<OsPath, PyObjectRef>> {
+        let check_nul = |b: &[u8]| {
+            if self.non_strict || memchr::memchr(b'\0', b).is_none() {
+                Ok(())
+            } else {
+                Err(vm.new_value_error(format!(
+                    "{}embedded null character in {}",
+                    self.error_prefix(),
+                    self.arg_name()
+                )))
+            }
+        };
+
+        match_class!(match obj {
+            s @ PyStr => {
+                check_nul(s.as_bytes())?;
+                let path = vm.fsencode(&s)?.into_owned();
+                Ok(Ok(OsPath {
+                    path,
+                    origin: Some(s.into()),
+                }))
+            }
+            b @ PyBytes => {
+                check_nul(&b)?;
+                let path = FsPath::bytes_as_os_str(&b, vm)?.to_owned();
+                Ok(Ok(OsPath {
+                    path,
+                    origin: Some(b.into()),
+                }))
+            }
+            obj => Ok(Err(obj)),
+        })
+    }
+
+    /// Convert to OsPath directly
+    pub fn try_path(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<OsPath> {
+        self.try_path_inner(obj, false, vm)
+    }
+}
+
+/// path_t output - the converted path
 #[derive(Clone)]
 pub struct OsPath {
     pub path: std::ffi::OsString,
-    pub(super) mode: OutputMode,
     /// Original Python object for identity preservation in OSError
     pub(super) origin: Option<PyObjectRef>,
 }
 
 #[derive(Debug, Copy, Clone)]
-pub(super) enum OutputMode {
+pub enum OutputMode {
     String,
     Bytes,
 }
@@ -40,22 +199,17 @@ impl OutputMode {
 impl OsPath {
     pub fn new_str(path: impl Into<std::ffi::OsString>) -> Self {
         let path = path.into();
-        Self {
-            path,
-            mode: OutputMode::String,
-            origin: None,
-        }
+        Self { path, origin: None }
     }
 
     pub(crate) fn from_fspath(fspath: FsPath, vm: &VirtualMachine) -> PyResult<Self> {
         let path = fspath.as_os_str(vm)?.into_owned();
-        let (mode, origin) = match fspath {
-            FsPath::Str(s) => (OutputMode::String, s.into()),
-            FsPath::Bytes(b) => (OutputMode::Bytes, b.into()),
+        let origin = match fspath {
+            FsPath::Str(s) => s.into(),
+            FsPath::Bytes(b) => b.into(),
         };
         Ok(Self {
             path,
-            mode,
             origin: Some(origin),
         })
     }
@@ -93,7 +247,16 @@ impl OsPath {
         if let Some(ref origin) = self.origin {
             origin.clone()
         } else {
-            self.mode.process_path(self.path.clone(), vm)
+            // Default to string when no origin (e.g., from new_str)
+            OutputMode::String.process_path(self.path.clone(), vm)
+        }
+    }
+
+    /// Get the output mode based on origin type (bytes -> Bytes, otherwise -> String)
+    pub fn mode(&self) -> OutputMode {
+        match &self.origin {
+            Some(obj) if obj.downcast_ref::<PyBytes>().is_some() => OutputMode::Bytes,
+            _ => OutputMode::String,
         }
     }
 }
@@ -105,10 +268,8 @@ impl AsRef<Path> for OsPath {
 }
 
 impl TryFromObject for OsPath {
-    // path_converter with allow_fd=0
     fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
-        let fspath = FsPath::try_from(obj, true, "should be string, bytes or os.PathLike", vm)?;
-        Self::from_fspath(fspath, vm)
+        PathConverter::new().try_path(obj, vm)
     }
 }
 
@@ -121,15 +282,7 @@ pub(crate) enum OsPathOrFd<'fd> {
 
 impl TryFromObject for OsPathOrFd<'_> {
     fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
-        match obj.try_index_opt(vm) {
-            Some(int) => {
-                let fd = int?.try_to_primitive(vm)?;
-                unsafe { crt_fd::Borrowed::try_borrow_raw(fd) }
-                    .map(Self::Fd)
-                    .map_err(|e| e.into_pyexception(vm))
-            }
-            None => obj.try_into_value(vm).map(Self::Path),
-        }
+        PathConverter::new().try_path_or_fd(obj, vm)
     }
 }
 

diff --git a/crates/vm/src/stdlib/nt.rs b/crates/vm/src/stdlib/nt.rs
@@ -925,7 +925,7 @@ pub(crate) mod module {
             .as_ref()
             .canonicalize()
             .map_err(|e| e.to_pyexception(vm))?;
-        Ok(path.mode.process_path(real, vm))
+        Ok(path.mode().process_path(real, vm))
     }
 
     #[pyfunction]
@@ -958,7 +958,7 @@ pub(crate) mod module {
             }
         }
         let buffer = widestring::WideCString::from_vec_truncate(buffer);
-        Ok(path.mode.process_path(buffer.to_os_string(), vm))
+        Ok(path.mode().process_path(buffer.to_os_string(), vm))
     }
 
     #[pyfunction]
@@ -973,7 +973,7 @@ pub(crate) mod module {
             return Err(vm.new_last_os_error());
         }
         let buffer = widestring::WideCString::from_vec_truncate(buffer);
-        Ok(path.mode.process_path(buffer.to_os_string(), vm))
+        Ok(path.mode().process_path(buffer.to_os_string(), vm))
     }
 
     /// Implements _Py_skiproot logic for Windows paths
@@ -1053,7 +1053,7 @@ pub(crate) mod module {
         use crate::builtins::{PyBytes, PyStr};
         use rustpython_common::wtf8::Wtf8Buf;
 
-        // Handle path-like objects via os.fspath, but without null check (nonstrict=True)
+        // Handle path-like objects via os.fspath, but without null check (non_strict=True)
         let path = if let Some(fspath) = vm.get_method(path.clone(), identifier!(vm, __fspath__)) {
             fspath?.call((), vm)?
         } else {
@@ -1585,7 +1585,7 @@ pub(crate) mod module {
         use windows_sys::Win32::System::IO::DeviceIoControl;
         use windows_sys::Win32::System::Ioctl::FSCTL_GET_REPARSE_POINT;
 
-        let mode = path.mode;
+        let mode = path.mode();
         let wide_path = path.as_ref().to_wide_with_nul();
 
         // Open the file/directory with reparse point flag