Revamp PyType name functions to match PEP 737

guide/src/migration.md

@@ -81,6 +81,61 @@ enum SimpleEnum {
 ```
 </details>
 
+### `PyType::name` now returns `PyResult<String>` instead of `PyResult<Cow<'_, str>>`
+<details open>
+<summary><small>Click to expand</small></summary>
+
+This function previously would try to reuse a raw C API field (`tp_name`) in which case it would return a `Cow::Borrowed`,
+but this caused inconsistency with Python.
+It now always returned a fully owned String.
+
+To migrate, remove any `.into_owned()` and `.into_mut()` calls on the output of `PyType::name`.
+
+Before:
+
+```rust,ignore
+# #![allow(deprecated, dead_code)]
+# use pyo3::prelude::*;
+# use pyo3::types::{PyBool};
+# fn main() -> PyResult<()> {
+Python::with_gil(|py| {
+    let bool_type = py.get_type_bound::<PyBool>();
+    let name = bool_type.name()?.into_owned();
+    println!("Hello, {}", name);
+
+    let mut name_upper = bool_type.name()?;
+    name_upper.to_mut().make_ascii_uppercase();
+    println!("Hello, {}", name_upper);
+
+    Ok(())
+})
+# }
+```
+
+After:
+
+```rust
+# #![allow(dead_code)]
+# use pyo3::prelude::*;
+# use pyo3::types::{PyBool};
+# fn main() -> PyResult<()> {
+Python::with_gil(|py| {
+    let bool_type = py.get_type_bound::<PyBool>();
+    let name = bool_type.name()?;
+    println!("Hello, {}", name);
+
+    let mut name_upper = bool_type.name()?;
+    name_upper.make_ascii_uppercase();
+    println!("Hello, {}", name_upper);
+
+    Ok(())
+})
+# }
+```
+</details>
+
+
+
 ## from 0.20.* to 0.21
 <details>
 <summary><small>Click to expand</small></summary>

newsfragments/4196.added.md

@@ -0,0 +1,4 @@
+Add `PyType::module`, which always matches Python `__module__`.
+Add `PyType::fully_qualified_name` which matches the "fully qualified name"
+defined in https://peps.python.org/pep-0737 (not exposed in Python),
+which is useful for error messages and `repr()` implementations.

newsfragments/4196.changed.md

@@ -0,0 +1 @@
+Change `PyType::name` to always match Python `__name__`.

pyo3-ffi/src/object.rs

@@ -261,6 +261,14 @@ extern "C" {
     #[cfg_attr(PyPy, link_name = "PyPyType_GetQualName")]
     pub fn PyType_GetQualName(arg1: *mut PyTypeObject) -> *mut PyObject;
 
+    #[cfg(Py_3_13)]
+    #[cfg_attr(PyPy, link_name = "PyPyType_GetFullyQualifiedName")]
+    pub fn PyType_GetFullyQualifiedName(arg1: *mut PyTypeObject) -> *mut PyObject;
+
+    #[cfg(Py_3_13)]
+    #[cfg_attr(PyPy, link_name = "PyPyType_GetModuleName")]
+    pub fn PyType_GetModuleName(arg1: *mut PyTypeObject) -> *mut PyObject;
+
     #[cfg(Py_3_12)]
     #[cfg_attr(PyPy, link_name = "PyPyType_FromMetaclass")]
     pub fn PyType_FromMetaclass(

pytests/src/misc.rs

@@ -1,5 +1,4 @@
 use pyo3::{prelude::*, types::PyDict};
-use std::borrow::Cow;
 
 #[pyfunction]
 fn issue_219() {
@@ -8,8 +7,8 @@ fn issue_219() {
 }
 
 #[pyfunction]
-fn get_type_full_name(obj: &Bound<'_, PyAny>) -> PyResult<String> {
-    obj.get_type().name().map(Cow::into_owned)
+fn get_type_full_qualified_name(obj: &Bound<'_, PyAny>) -> PyResult<String> {
+    obj.get_type().fully_qualified_name()
 }
 
 #[pyfunction]
@@ -33,7 +32,7 @@ fn get_item_and_run_callback(dict: Bound<'_, PyDict>, callback: Bound<'_, PyAny>
 #[pymodule]
 pub fn misc(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_function(wrap_pyfunction!(issue_219, m)?)?;
-    m.add_function(wrap_pyfunction!(get_type_full_name, m)?)?;
+    m.add_function(wrap_pyfunction!(get_type_full_qualified_name, m)?)?;
     m.add_function(wrap_pyfunction!(accepts_bool, m)?)?;
     m.add_function(wrap_pyfunction!(get_item_and_run_callback, m)?)?;
     Ok(())

src/types/typeobject.rs

@@ -5,9 +5,6 @@ use crate::types::PyTuple;
 #[cfg(feature = "gil-refs")]
 use crate::PyNativeType;
 use crate::{ffi, Bound, PyAny, PyTypeInfo, Python};
-use std::borrow::Cow;
-#[cfg(not(any(Py_LIMITED_API, PyPy)))]
-use std::ffi::CStr;
 /// Represents a reference to a Python `type object`.
 #[repr(transparent)]
 pub struct PyType(PyAny);
@@ -71,15 +68,18 @@ impl PyType {
         Self::from_borrowed_type_ptr(py, p).into_gil_ref()
     }
 
+    /// Gets the name of the `PyType`. Equivalent to `self.__name__` in Python.
+    pub fn name(&self) -> PyResult<String> {
+        self.as_borrowed().name()
+    }
+
     /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
+    /// Equivalent to `self.__qualname__` in Python.
     pub fn qualname(&self) -> PyResult<String> {
         self.as_borrowed().qualname()
     }
 
-    /// Gets the full name, which includes the module, of the `PyType`.
-    pub fn name(&self) -> PyResult<Cow<'_, str>> {
-        self.as_borrowed().name()
-    }
+    // `module` and `fully_qualified_name` intentionally omitted
 
     /// Checks whether `self` is a subclass of `other`.
     ///
@@ -110,12 +110,19 @@ pub trait PyTypeMethods<'py>: crate::sealed::Sealed {
     /// Retrieves the underlying FFI pointer associated with this Python object.
     fn as_type_ptr(&self) -> *mut ffi::PyTypeObject;
 
-    /// Gets the full name, which includes the module, of the `PyType`.
-    fn name(&self) -> PyResult<Cow<'_, str>>;
+    /// Gets the name of the `PyType`. Equivalent to `self.__name__` in Python.
+    fn name(&self) -> PyResult<String>;
 
     /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
+    /// Equivalent to `self.__qualname__` in Python.
     fn qualname(&self) -> PyResult<String>;
 
+    /// Gets the name of the module defining the `PyType`.
+    fn module(&self) -> PyResult<String>;
+
+    /// Gets the [fully qualified name](https://peps.python.org/pep-0737/#add-pytype-getfullyqualifiedname-function) of the `PyType`.
+    fn fully_qualified_name(&self) -> PyResult<String>;
+
     /// Checks whether `self` is a subclass of `other`.
     ///
     /// Equivalent to the Python expression `issubclass(self, other)`.
@@ -148,10 +155,23 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
     }
 
     /// Gets the name of the `PyType`.
-    fn name(&self) -> PyResult<Cow<'_, str>> {
-        Borrowed::from(self).name()
+    fn name(&self) -> PyResult<String> {
+        #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))]
+        let name = self.getattr(intern!(self.py(), "__name__"))?.extract();
+
+        #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))]
+        let name = {
+            use crate::ffi_ptr_ext::FfiPtrExt;
+            let obj =
+                unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? };
+
+            obj.extract()
+        };
+
+        name
     }
 
+    /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
     fn qualname(&self) -> PyResult<String> {
         #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))]
         let name = self.getattr(intern!(self.py(), "__qualname__"))?.extract();
@@ -169,6 +189,53 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
         name
     }
 
+    /// Gets the name of the module defining the `PyType`.
+    fn module(&self) -> PyResult<String> {
+        #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_13)))]
+        let name = self.getattr(intern!(self.py(), "__module__"))?.extract();
+
+        #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_13))))]
+        let name = {
+            use crate::ffi_ptr_ext::FfiPtrExt;
+            let obj = unsafe {
+                ffi::PyType_GetModuleName(self.as_type_ptr()).assume_owned_or_err(self.py())?
+            };
+
+            obj.extract()
+        };
+
+        name
+    }
+
+    /// Gets the [fully qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
+    fn fully_qualified_name(&self) -> PyResult<String> {
+        #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_13)))]
+        let name = {
+            let module = self.getattr(intern!(self.py(), "__module__"))?;
+            let qualname = self.getattr(intern!(self.py(), "__qualname__"))?;
+
+            let module_str = module.extract::<&str>()?;
+            if module_str == "builtins" || module_str == "__main__" {
+                qualname.extract()
+            } else {
+                Ok(format!("{}.{}", module, qualname))
+            }
+        };
+
+        #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_13))))]
+        let name = {
+            use crate::ffi_ptr_ext::FfiPtrExt;
+            let obj = unsafe {
+                ffi::PyType_GetFullyQualifiedName(self.as_type_ptr())
+                    .assume_owned_or_err(self.py())?
+            };
+
+            obj.extract()
+        };
+
+        name
+    }
+
     /// Checks whether `self` is a subclass of `other`.
     ///
     /// Equivalent to the Python expression `issubclass(self, other)`.
@@ -232,43 +299,11 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
     }
 }
 
-impl<'a> Borrowed<'a, '_, PyType> {
-    fn name(self) -> PyResult<Cow<'a, str>> {
-        #[cfg(not(any(Py_LIMITED_API, PyPy)))]
-        {
-            let ptr = self.as_type_ptr();
-
-            let name = unsafe { CStr::from_ptr((*ptr).tp_name) }.to_str()?;
-
-            #[cfg(Py_3_10)]
-            if unsafe { ffi::PyType_HasFeature(ptr, ffi::Py_TPFLAGS_IMMUTABLETYPE) } != 0 {
-                return Ok(Cow::Borrowed(name));
-            }
-
-            Ok(Cow::Owned(name.to_owned()))
-        }
-
-        #[cfg(any(Py_LIMITED_API, PyPy))]
-        {
-            let module = self.getattr(intern!(self.py(), "__module__"))?;
-
-            #[cfg(not(Py_3_11))]
-            let name = self.getattr(intern!(self.py(), "__name__"))?;
-
-            #[cfg(Py_3_11)]
-            let name = {
-                use crate::ffi_ptr_ext::FfiPtrExt;
-                unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? }
-            };
-
-            Ok(Cow::Owned(format!("{}.{}", module, name)))
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use crate::types::{PyAnyMethods, PyBool, PyInt, PyLong, PyTuple, PyTypeMethods};
+    use crate::types::{
+        PyAnyMethods, PyBool, PyInt, PyLong, PyModule, PyTuple, PyType, PyTypeMethods,
+    };
     use crate::PyAny;
     use crate::Python;
 
@@ -330,4 +365,72 @@ mod tests {
                 .unwrap());
         });
     }
+
+    #[test]
+    fn test_type_names_standard() {
+        Python::with_gil(|py| {
+            let module = PyModule::from_code_bound(
+                py,
+                r#"
+class MyClass:
+    pass
+"#,
+                file!(),
+                "test_module",
+            )
+            .expect("module create failed");
+
+            let my_class = module.getattr("MyClass").unwrap();
+            let my_class_type = my_class.downcast_into::<PyType>().unwrap();
+            assert_eq!(my_class_type.name().unwrap(), "MyClass");
+            assert_eq!(my_class_type.qualname().unwrap(), "MyClass");
+            assert_eq!(my_class_type.module().unwrap(), "test_module");
+            assert_eq!(
+                my_class_type.fully_qualified_name().unwrap(),
+                "test_module.MyClass"
+            );
+        });
+    }
+
+    #[test]
+    fn test_type_names_builtin() {
+        Python::with_gil(|py| {
+            let bool_type = py.get_type_bound::<PyBool>();
+            assert_eq!(bool_type.name().unwrap(), "bool");
+            assert_eq!(bool_type.qualname().unwrap(), "bool");
+            assert_eq!(bool_type.module().unwrap(), "builtins");
+            assert_eq!(bool_type.fully_qualified_name().unwrap(), "bool");
+        });
+    }
+
+    #[test]
+    fn test_type_names_nested() {
+        Python::with_gil(|py| {
+            let module = PyModule::from_code_bound(
+                py,
+                r#"
+class OuterClass:
+    class InnerClass:
+        pass
+"#,
+                file!(),
+                "test_module",
+            )
+            .expect("module create failed");
+
+            let outer_class = module.getattr("OuterClass").unwrap();
+            let inner_class = outer_class.getattr("InnerClass").unwrap();
+            let inner_class_type = inner_class.downcast_into::<PyType>().unwrap();
+            assert_eq!(inner_class_type.name().unwrap(), "InnerClass");
+            assert_eq!(
+                inner_class_type.qualname().unwrap(),
+                "OuterClass.InnerClass"
+            );
+            assert_eq!(inner_class_type.module().unwrap(), "test_module");
+            assert_eq!(
+                inner_class_type.fully_qualified_name().unwrap(),
+                "test_module.OuterClass.InnerClass"
+            );
+        });
+    }
 }