Integrate OSError creations into OSErrorBuilder

youknowone · Dec 16, 2025 · Dec 15, 2025 · Dec 15, 2025 · Dec 16, 2025 · Dec 16, 2025
commit d7357f8bdcc2e6679938fb6196325a1e1c48be10
diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs
@@ -1193,6 +1193,8 @@ pub(crate) fn errno_to_exc_type(_errno: i32, _vm: &VirtualMachine) -> Option<&'s
     None
 }
 
+pub(crate) use types::{OSErrorBuilder, ToOSErrorBuilder};
+
 pub(super) mod types {
     use crate::common::lock::PyRwLock;
     use crate::object::{MaybeTraverse, Traverse, TraverseFn};
@@ -1204,6 +1206,7 @@ pub(super) mod types {
             PyInt, PyStrRef, PyTupleRef, PyType, PyTypeRef, traceback::PyTracebackRef,
             tuple::IntoPyTuple,
         },
+        convert::ToPyObject,
         convert::ToPyResult,
         function::{ArgBytesLike, FuncArgs},
         types::{Constructor, Initializer},
@@ -1212,6 +1215,113 @@ pub(super) mod types {
     use itertools::Itertools;
     use rustpython_common::str::UnicodeEscapeCodepoint;
 
+    pub(crate) trait ToOSErrorBuilder {
+        fn to_os_error_builder(&self, vm: &VirtualMachine) -> OSErrorBuilder;
+    }
+
+    pub struct OSErrorBuilder {
+        exc_type: PyTypeRef,
+        errno: Option<i32>,
+        strerror: Option<PyObjectRef>,
+        filename: Option<PyObjectRef>,
+        #[cfg(windows)]
+        winerror: Option<PyObjectRef>,
+        filename2: Option<PyObjectRef>,
+    }
+
+    impl OSErrorBuilder {
+        #[must_use]
+        pub fn with_subtype(
+            exc_type: PyTypeRef,
+            errno: Option<i32>,
+            strerror: impl ToPyObject,
+            vm: &VirtualMachine,
+        ) -> Self {
+            let strerror = strerror.to_pyobject(vm);
+            Self {
+                exc_type,
+                errno,
+                strerror: Some(strerror.to_pyobject(vm)),
+                filename: None,
+                #[cfg(windows)]
+                winerror: None,
+                filename2: None,
+            }
+        }
+        #[must_use]
+        pub fn with_errno(errno: i32, strerror: impl ToPyObject, vm: &VirtualMachine) -> Self {
+            let exc_type = crate::exceptions::errno_to_exc_type(errno, vm)
+                .unwrap_or(vm.ctx.exceptions.os_error)
+                .to_owned();
+            Self::with_subtype(exc_type, Some(errno), strerror, vm)
+        }
+
+        // #[must_use]
+        // pub(crate) fn errno(mut self, errno: i32) -> Self {
+        //     self.errno.replace(errno);
+        //     self
+        // }
+
+        #[must_use]
+        pub(crate) fn filename(mut self, filename: PyObjectRef) -> Self {
+            self.filename.replace(filename);
+            self
+        }
+
+        #[must_use]
+        pub(crate) fn filename2(mut self, filename: PyObjectRef) -> Self {
+            self.filename2.replace(filename);
+            self
+        }
+
+        #[must_use]
+        #[cfg(windows)]
+        pub(crate) fn winerror(mut self, winerror: PyObjectRef) -> Self {
+            self.winerror.replace(winerror);
+            self
+        }
+
+        pub fn build(self, vm: &VirtualMachine) -> PyRef<PyOSError> {
+            let OSErrorBuilder {
+                exc_type,
+                errno,
+                strerror,
+                filename,
+                #[cfg(windows)]
+                winerror,
+                filename2,
+            } = self;
+
+            #[cfg(windows)]
+            let winerror = winerror.to_pyobject(vm);
+            #[cfg(not(windows))]
+            let winerror = vm.ctx.none();
+
+            let args = vec![
+                errno.to_pyobject(vm),
+                strerror.to_pyobject(vm),
+                filename.to_pyobject(vm),
+                winerror,
+                filename2.to_pyobject(vm),
+            ];
+
+            let payload = PyOSError::py_new(&exc_type, args.clone().into(), vm)
+                .expect("new_os_error usage error");
+            let os_error = payload
+                .into_ref_with_type(vm, exc_type)
+                .expect("new_os_error usage error");
+            PyOSError::slot_init(os_error.as_object().to_owned(), args.into(), vm)
+                .expect("new_os_error usage error");
+            os_error
+        }
+    }
+
+    impl crate::convert::IntoPyException for OSErrorBuilder {
+        fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef {
+            self.build(vm).upcast()
+        }
+    }
+
     // Re-export exception group types from dedicated module
     pub use crate::exception_group::types::PyBaseExceptionGroup;
 

diff --git a/crates/vm/src/ospath.rs b/crates/vm/src/ospath.rs
@@ -2,10 +2,8 @@ use rustpython_common::crt_fd;
 
 use crate::{
     PyObjectRef, PyResult, VirtualMachine,
-    builtins::PyBaseExceptionRef,
     convert::{IntoPyException, ToPyException, ToPyObject, TryFromObject},
     function::FsPath,
-    object::AsObject,
 };
 use std::path::{Path, PathBuf};
 
@@ -144,62 +142,17 @@ impl OsPathOrFd<'_> {
     }
 }
 
-// TODO: preserve the input `PyObjectRef` of filename and filename2 (Failing check `self.assertIs(err.filename, name, str(func)`)
-pub struct OSErrorBuilder<'a> {
-    error: &'a std::io::Error,
-    filename: Option<OsPathOrFd<'a>>,
-    filename2: Option<OsPathOrFd<'a>>,
-}
-
-impl<'a> OSErrorBuilder<'a> {
-    pub const fn new(error: &'a std::io::Error) -> Self {
-        Self {
-            error,
-            filename: None,
-            filename2: None,
-        }
-    }
-
-    pub(crate) fn filename(mut self, filename: impl Into<OsPathOrFd<'a>>) -> Self {
-        let filename = filename.into();
-        self.filename.replace(filename);
-        self
-    }
-
-    pub(crate) fn filename2(mut self, filename: impl Into<OsPathOrFd<'a>>) -> Self {
-        let filename = filename.into();
-        self.filename2.replace(filename);
-        self
-    }
-
-    pub(crate) fn with_filename(
-        error: &'a std::io::Error,
+impl crate::exceptions::OSErrorBuilder {
+    #[must_use]
+    pub(crate) fn with_filename<'a>(
+        error: &std::io::Error,
         filename: impl Into<OsPathOrFd<'a>>,
         vm: &VirtualMachine,
-    ) -> PyBaseExceptionRef {
-        let zelf = OSErrorBuilder {
-            error,
-            filename: Some(filename.into()),
-            filename2: None,
-        };
-        zelf.to_pyexception(vm)
-    }
-}
-
-impl ToPyException for OSErrorBuilder<'_> {
-    fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef {
-        let exc = self.error.to_pyexception(vm);
-
-        if let Some(filename) = &self.filename {
-            exc.as_object()
-                .set_attr("filename", filename.filename(vm), vm)
-                .unwrap();
-        }
-        if let Some(filename2) = &self.filename2 {
-            exc.as_object()
-                .set_attr("filename2", filename2.filename(vm), vm)
-                .unwrap();
-        }
-        exc
+    ) -> crate::builtins::PyBaseExceptionRef {
+        // TODO: return type to PyRef<PyOSError>
+        use crate::exceptions::ToOSErrorBuilder;
+        let builder = error.to_os_error_builder(vm);
+        let builder = builder.filename(filename.into().filename(vm));
+        builder.build(vm).upcast()
     }
 }
diff --git a/crates/vm/src/stdlib/io.rs b/crates/vm/src/stdlib/io.rs
@@ -14,11 +14,12 @@ use crate::{
     builtins::{PyBaseExceptionRef, PyModule},
     common::os::ErrorExt,
     convert::{IntoPyException, ToPyException},
+    exceptions::{OSErrorBuilder, ToOSErrorBuilder},
 };
 pub use _io::{OpenArgs, io_open as open};
 
-impl ToPyException for std::io::Error {
-    fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef {
+impl ToOSErrorBuilder for std::io::Error {
+    fn to_os_error_builder(&self, vm: &VirtualMachine) -> OSErrorBuilder {
         let errno = self.posix_errno();
         #[cfg(windows)]
         let msg = 'msg: {
@@ -53,23 +54,23 @@ impl ToPyException for std::io::Error {
         #[cfg(not(any(windows, unix)))]
         let msg = self.to_string();
 
-        #[allow(clippy::let_and_return)]
-        let exc = vm.new_errno_error(errno, msg);
-        #[cfg(windows)]
-        {
-            use crate::object::AsObject;
-            let winerror = if let Some(winerror) = self.raw_os_error() {
-                vm.new_pyobj(winerror)
-            } else {
-                vm.ctx.none()
-            };
+        #[allow(unused_mut)]
+        let mut builder = OSErrorBuilder::with_errno(errno, msg, vm);
 
-            // FIXME: manual setup winerror due to lack of OSError.__init__ support
-            exc.as_object()
-                .set_attr("winerror", vm.new_pyobj(winerror), vm)
-                .unwrap();
+        #[cfg(windows)]
+        if let Some(winerror) = self.raw_os_error() {
+            use crate::convert::ToPyObject;
+            builder = builder.winerror(winerror.to_pyobject(vm));
         }
-        exc.upcast()
+
+        builder
+    }
+}
+
+impl ToPyException for std::io::Error {
+    fn to_pyexception(&self, vm: &VirtualMachine) -> PyBaseExceptionRef {
+        let builder = self.to_os_error_builder(vm);
+        builder.into_pyexception(vm)
     }
 }
 
@@ -4328,8 +4329,9 @@ mod fileio {
         builtins::{PyBaseExceptionRef, PyUtf8Str, PyUtf8StrRef},
         common::crt_fd,
         convert::{IntoPyException, ToPyException},
+        exceptions::OSErrorBuilder,
         function::{ArgBytesLike, ArgMemoryBuffer, OptionalArg, OptionalOption},
-        ospath::{OSErrorBuilder, OsPath, OsPathOrFd},
+        ospath::{OsPath, OsPathOrFd},
         stdlib::os,
         types::{Constructor, DefaultConstructor, Destructor, Initializer, Representable},
     };

diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs
@@ -153,6 +153,7 @@ pub(super) mod _os {
         AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject,
         builtins::{
             PyBytesRef, PyGenericAlias, PyIntRef, PyStrRef, PyTuple, PyTupleRef, PyTypeRef,
+            ToOSErrorBuilder,
         },
         common::{
             crt_fd,
@@ -161,8 +162,9 @@ pub(super) mod _os {
             suppress_iph,
         },
         convert::{IntoPyException, ToPyObject},
+        exceptions::OSErrorBuilder,
         function::{ArgBytesLike, FsPath, FuncArgs, OptionalArg},
-        ospath::{OSErrorBuilder, OsPath, OsPathOrFd, OutputMode},
+        ospath::{OsPath, OsPathOrFd, OutputMode},
         protocol::PyIterReturn,
         recursion::ReprGuard,
         types::{IterNext, Iterable, PyStructSequence, Representable, SelfIter, Unconstructible},
@@ -1127,10 +1129,10 @@ pub(super) mod _os {
     #[pyfunction(name = "replace")]
     fn rename(src: OsPath, dst: OsPath, vm: &VirtualMachine) -> PyResult<()> {
         fs::rename(&src.path, &dst.path).map_err(|err| {
-            OSErrorBuilder::new(&err)
-                .filename(src)
-                .filename2(dst)
-                .into_pyexception(vm)
+            let builder = err.to_os_error_builder(vm);
+            let builder = builder.filename(src.filename(vm));
+            let builder = builder.filename2(dst.filename(vm));
+            builder.build(vm).upcast()
         })
     }
 
@@ -1219,10 +1221,10 @@ pub(super) mod _os {
     #[pyfunction]
     fn link(src: OsPath, dst: OsPath, vm: &VirtualMachine) -> PyResult<()> {
         fs::hard_link(&src.path, &dst.path).map_err(|err| {
-            OSErrorBuilder::new(&err)
-                .filename(src)
-                .filename2(dst)
-                .into_pyexception(vm)
+            let builder = err.to_os_error_builder(vm);
+            let builder = builder.filename(src.filename(vm));
+            let builder = builder.filename2(dst.filename(vm));
+            builder.build(vm).upcast()
         })
     }
 

diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs
@@ -26,8 +26,9 @@ pub mod module {
         AsObject, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine,
         builtins::{PyDictRef, PyInt, PyListRef, PyStrRef, PyTupleRef, PyType, PyUtf8StrRef},
         convert::{IntoPyException, ToPyObject, TryFromObject},
+        exceptions::OSErrorBuilder,
         function::{Either, KwArgs, OptionalArg},
-        ospath::{OSErrorBuilder, OsPath, OsPathOrFd},
+        ospath::{OsPath, OsPathOrFd},
         stdlib::os::{_os, DirFd, FollowSymlinks, SupportFunc, TargetIsDirectory, fs_metadata},
         types::{Constructor, Representable},
         utils::ToCString,
@@ -482,7 +483,7 @@ pub mod module {
     #[cfg(not(target_os = "redox"))]
     #[pyfunction]
     fn chroot(path: OsPath, vm: &VirtualMachine) -> PyResult<()> {
-        use crate::ospath::OSErrorBuilder;
+        use crate::exceptions::OSErrorBuilder;
 
         nix::unistd::chroot(&*path.path).map_err(|err| {
             // Use `From<nix::Error> for io::Error` when it is available

diff --git a/crates/vm/src/vm/vm_new.rs b/crates/vm/src/vm/vm_new.rs
@@ -1,5 +1,5 @@
 use crate::{
-    AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef,
+    AsObject, Py, PyObject, PyObjectRef, PyRef,
     builtins::{
         PyBaseException, PyBaseExceptionRef, PyBytesRef, PyDictRef, PyModule, PyOSError, PyStrRef,
         PyType, PyTypeRef,
@@ -8,9 +8,9 @@ use crate::{
         tuple::{IntoPyTuple, PyTupleRef},
     },
     convert::{ToPyException, ToPyObject},
+    exceptions::OSErrorBuilder,
     function::{IntoPyNativeFn, PyMethodFlags},
     scope::Scope,
-    types::Constructor,
     vm::VirtualMachine,
 };
 use rustpython_compiler_core::SourceLocation;
@@ -119,26 +119,8 @@ impl VirtualMachine {
         msg: impl ToPyObject,
     ) -> PyRef<PyOSError> {
         debug_assert_eq!(exc_type.slots.basicsize, std::mem::size_of::<PyOSError>());
-        let msg = msg.to_pyobject(self);
-
-        fn new_os_subtype_error_impl(
-            vm: &VirtualMachine,
-            exc_type: PyTypeRef,
-            errno: Option<i32>,
-            msg: PyObjectRef,
-        ) -> PyRef<PyOSError> {
-            let args = match errno {
-                Some(e) => vec![vm.new_pyobj(e), msg],
-                None => vec![msg],
-            };
-            let payload =
-                PyOSError::py_new(&exc_type, args.into(), vm).expect("new_os_error usage error");
-            payload
-                .into_ref_with_type(vm, exc_type)
-                .expect("new_os_error usage error")
-        }
 
-        new_os_subtype_error_impl(self, exc_type, errno, msg)
+        OSErrorBuilder::with_subtype(exc_type, errno, msg, self).build(self)
     }
 
     /// Instantiate an exception with no arguments.