diff --git a/newsfragments/4100.changed.md b/newsfragments/4100.changed.md new file mode 100644 index 00000000000..13fd2e02aa8 --- /dev/null +++ b/newsfragments/4100.changed.md @@ -0,0 +1 @@ +Change `PyAnyMethods::dir` to be fallible and return `PyResult>` (and similar for `PyAny::dir`). diff --git a/src/types/any.rs b/src/types/any.rs index a5ea4c80d4c..6ba34c86bc6 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -845,8 +845,8 @@ impl PyAny { /// Returns the list of attributes of this object. /// /// This is equivalent to the Python expression `dir(self)`. - pub fn dir(&self) -> &PyList { - self.as_borrowed().dir().into_gil_ref() + pub fn dir(&self) -> PyResult<&PyList> { + self.as_borrowed().dir().map(Bound::into_gil_ref) } /// Checks whether this object is an instance of type `ty`. @@ -1674,7 +1674,7 @@ pub trait PyAnyMethods<'py>: crate::sealed::Sealed { /// Returns the list of attributes of this object. /// /// This is equivalent to the Python expression `dir(self)`. - fn dir(&self) -> Bound<'py, PyList>; + fn dir(&self) -> PyResult>; /// Checks whether this object is an instance of type `ty`. /// @@ -2220,10 +2220,10 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { Ok(v as usize) } - fn dir(&self) -> Bound<'py, PyList> { + fn dir(&self) -> PyResult> { unsafe { ffi::PyObject_Dir(self.as_ptr()) - .assume_owned(self.py()) + .assume_owned_or_err(self.py()) .downcast_into_unchecked() } } @@ -2471,6 +2471,7 @@ class SimpleClass: .unwrap(); let a = obj .dir() + .unwrap() .into_iter() .map(|x| x.extract::().unwrap()); let b = dir.into_iter().map(|x| x.extract::().unwrap()); @@ -2745,4 +2746,27 @@ class SimpleClass: assert!(not_container.is_empty().is_err()); }); } + + #[cfg(feature = "macros")] + #[test] + #[allow(unknown_lints, non_local_definitions)] + fn test_fallible_dir() { + use crate::exceptions::PyValueError; + use crate::prelude::*; + + #[pyclass(crate = "crate")] + struct DirFail; + + #[pymethods(crate = "crate")] + impl DirFail { + fn __dir__(&self) -> PyResult { + Err(PyValueError::new_err("uh-oh!")) + } + } + + Python::with_gil(|py| { + let obj = Bound::new(py, DirFail).unwrap(); + assert!(obj.dir().unwrap_err().is_instance_of::(py)); + }) + } }