Skip to content

Commit 28db16c

Browse files
authored
Fetch type name dynamically on cast errors instead of using PyTypeInfo::NAME (#5387)
* Fetch type name dynamically on cast errors * Lazily fetch type name * Derive Debug * Introduce PyTypeCheck::classinfo_object and make use of it * Add back PyTypeCheck::NAME * Fixes Python 3.9 tests
1 parent 6a6ed99 commit 28db16c

File tree

17 files changed

+286
-73
lines changed

17 files changed

+286
-73
lines changed

newsfragments/5387.added.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add `PyTypeCheck::classinfo_object` that returns an object that can be used as parameter in `isinstance` or `issubclass`

newsfragments/5387.changed.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- Fetch type name dynamically on cast errors instead of using `PyTypeCheck::NAME`
2+
- Deprecate `PyTypeCheck::NAME`, please `TypeTypeCheck::classinfo_object` to get the expected type and format it at runtime.

src/conversions/chrono.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -692,37 +692,37 @@ mod tests {
692692
let none = py.None().into_bound(py);
693693
assert_eq!(
694694
none.extract::<Duration>().unwrap_err().to_string(),
695-
"TypeError: 'NoneType' object cannot be converted to 'PyDelta'"
695+
"TypeError: 'NoneType' object cannot be converted to 'timedelta'"
696696
);
697697
assert_eq!(
698698
none.extract::<FixedOffset>().unwrap_err().to_string(),
699-
"TypeError: 'NoneType' object cannot be converted to 'PyTzInfo'"
699+
"TypeError: 'NoneType' object cannot be converted to 'tzinfo'"
700700
);
701701
assert_eq!(
702702
none.extract::<Utc>().unwrap_err().to_string(),
703703
"ValueError: expected datetime.timezone.utc"
704704
);
705705
assert_eq!(
706706
none.extract::<NaiveTime>().unwrap_err().to_string(),
707-
"TypeError: 'NoneType' object cannot be converted to 'PyTime'"
707+
"TypeError: 'NoneType' object cannot be converted to 'time'"
708708
);
709709
assert_eq!(
710710
none.extract::<NaiveDate>().unwrap_err().to_string(),
711-
"TypeError: 'NoneType' object cannot be converted to 'PyDate'"
711+
"TypeError: 'NoneType' object cannot be converted to 'date'"
712712
);
713713
assert_eq!(
714714
none.extract::<NaiveDateTime>().unwrap_err().to_string(),
715-
"TypeError: 'NoneType' object cannot be converted to 'PyDateTime'"
715+
"TypeError: 'NoneType' object cannot be converted to 'datetime'"
716716
);
717717
assert_eq!(
718718
none.extract::<DateTime<Utc>>().unwrap_err().to_string(),
719-
"TypeError: 'NoneType' object cannot be converted to 'PyDateTime'"
719+
"TypeError: 'NoneType' object cannot be converted to 'datetime'"
720720
);
721721
assert_eq!(
722722
none.extract::<DateTime<FixedOffset>>()
723723
.unwrap_err()
724724
.to_string(),
725-
"TypeError: 'NoneType' object cannot be converted to 'PyDateTime'"
725+
"TypeError: 'NoneType' object cannot be converted to 'datetime'"
726726
);
727727
});
728728
}

src/conversions/jiff.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -553,31 +553,31 @@ mod tests {
553553
let none = py.None().into_bound(py);
554554
assert_eq!(
555555
none.extract::<Span>().unwrap_err().to_string(),
556-
"TypeError: 'NoneType' object cannot be converted to 'PyDelta'"
556+
"TypeError: 'NoneType' object cannot be converted to 'timedelta'"
557557
);
558558
assert_eq!(
559559
none.extract::<Offset>().unwrap_err().to_string(),
560-
"TypeError: 'NoneType' object cannot be converted to 'PyTzInfo'"
560+
"TypeError: 'NoneType' object cannot be converted to 'tzinfo'"
561561
);
562562
assert_eq!(
563563
none.extract::<TimeZone>().unwrap_err().to_string(),
564-
"TypeError: 'NoneType' object cannot be converted to 'PyTzInfo'"
564+
"TypeError: 'NoneType' object cannot be converted to 'tzinfo'"
565565
);
566566
assert_eq!(
567567
none.extract::<Time>().unwrap_err().to_string(),
568-
"TypeError: 'NoneType' object cannot be converted to 'PyTime'"
568+
"TypeError: 'NoneType' object cannot be converted to 'time'"
569569
);
570570
assert_eq!(
571571
none.extract::<Date>().unwrap_err().to_string(),
572-
"TypeError: 'NoneType' object cannot be converted to 'PyDate'"
572+
"TypeError: 'NoneType' object cannot be converted to 'date'"
573573
);
574574
assert_eq!(
575575
none.extract::<DateTime>().unwrap_err().to_string(),
576-
"TypeError: 'NoneType' object cannot be converted to 'PyDateTime'"
576+
"TypeError: 'NoneType' object cannot be converted to 'datetime'"
577577
);
578578
assert_eq!(
579579
none.extract::<Zoned>().unwrap_err().to_string(),
580-
"TypeError: 'NoneType' object cannot be converted to 'PyDateTime'"
580+
"TypeError: 'NoneType' object cannot be converted to 'datetime'"
581581
);
582582
});
583583
}

src/conversions/smallvec.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ use crate::inspect::types::TypeInfo;
2222
use crate::types::any::PyAnyMethods;
2323
use crate::types::{PySequence, PyString};
2424
use crate::{
25-
err::DowncastError, ffi, Borrowed, Bound, FromPyObject, PyAny, PyErr, PyResult, Python,
25+
err::DowncastError, ffi, Borrowed, Bound, FromPyObject, PyAny, PyErr, PyResult, PyTypeInfo,
26+
Python,
2627
};
2728
use smallvec::{Array, SmallVec};
2829

@@ -102,7 +103,11 @@ where
102103
if ffi::PySequence_Check(obj.as_ptr()) != 0 {
103104
obj.cast_unchecked::<PySequence>()
104105
} else {
105-
return Err(DowncastError::new_from_borrowed(obj, "Sequence").into());
106+
return Err(DowncastError::new_from_type(
107+
obj,
108+
PySequence::type_object(obj.py()).into_any(),
109+
)
110+
.into());
106111
}
107112
};
108113

src/conversions/std/array.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::conversion::{FromPyObjectOwned, IntoPyObject};
22
use crate::types::any::PyAnyMethods;
33
use crate::types::PySequence;
4-
use crate::{err::DowncastError, ffi, FromPyObject, PyAny, PyResult, Python};
4+
use crate::{err::DowncastError, ffi, FromPyObject, PyAny, PyResult, PyTypeInfo, Python};
55
use crate::{exceptions, Borrowed, Bound, PyErr};
66

77
impl<'py, T, const N: usize> IntoPyObject<'py> for [T; N]
@@ -75,7 +75,11 @@ where
7575
if ffi::PySequence_Check(obj.as_ptr()) != 0 {
7676
obj.cast_unchecked::<PySequence>()
7777
} else {
78-
return Err(DowncastError::new_from_borrowed(obj, "Sequence").into());
78+
return Err(DowncastError::new_from_type(
79+
obj,
80+
PySequence::type_object(obj.py()).into_any(),
81+
)
82+
.into());
7983
}
8084
};
8185
let seq_len = seq.len()?;

src/conversions/std/vec.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
exceptions::PyTypeError,
66
ffi,
77
types::{PyAnyMethods, PySequence, PyString},
8-
Borrowed, DowncastError, PyResult,
8+
Borrowed, DowncastError, PyResult, PyTypeInfo,
99
};
1010
use crate::{Bound, PyAny, PyErr, Python};
1111

@@ -91,7 +91,11 @@ where
9191
if ffi::PySequence_Check(obj.as_ptr()) != 0 {
9292
obj.downcast_unchecked::<PySequence>()
9393
} else {
94-
return Err(DowncastError::new_from_borrowed(obj, "Sequence").into());
94+
return Err(DowncastError::new_from_type(
95+
obj,
96+
PySequence::type_object(obj.py()).into_any(),
97+
)
98+
.into());
9599
}
96100
};
97101

src/err/mod.rs

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::type_object::PyTypeInfo;
44
use crate::types::any::PyAnyMethods;
55
use crate::types::{
66
string::PyStringMethods, traceback::PyTracebackMethods, typeobject::PyTypeMethods, PyTraceback,
7-
PyType,
7+
PyTuple, PyTupleMethods, PyType,
88
};
99
use crate::{
1010
exceptions::{self, PyBaseException},
@@ -46,25 +46,23 @@ pub type PyResult<T> = Result<T, PyErr>;
4646
#[derive(Debug)]
4747
pub struct DowncastError<'a, 'py> {
4848
from: Borrowed<'a, 'py, PyAny>,
49-
to: Cow<'static, str>,
49+
to: TypeNameOrValue<'py>,
5050
}
5151

5252
impl<'a, 'py> DowncastError<'a, 'py> {
5353
/// Create a new `PyDowncastError` representing a failure to convert the object
5454
/// `from` into the type named in `to`.
5555
pub fn new(from: &'a Bound<'py, PyAny>, to: impl Into<Cow<'static, str>>) -> Self {
56-
DowncastError {
56+
Self {
5757
from: from.as_borrowed(),
58-
to: to.into(),
58+
to: TypeNameOrValue::Name(to.into()),
5959
}
6060
}
61-
pub(crate) fn new_from_borrowed(
62-
from: Borrowed<'a, 'py, PyAny>,
63-
to: impl Into<Cow<'static, str>>,
64-
) -> Self {
65-
DowncastError {
61+
62+
pub(crate) fn new_from_type(from: Borrowed<'a, 'py, PyAny>, to: Bound<'py, PyAny>) -> Self {
63+
Self {
6664
from,
67-
to: to.into(),
65+
to: TypeNameOrValue::Value(to),
6866
}
6967
}
7068
}
@@ -73,16 +71,23 @@ impl<'a, 'py> DowncastError<'a, 'py> {
7371
#[derive(Debug)]
7472
pub struct DowncastIntoError<'py> {
7573
from: Bound<'py, PyAny>,
76-
to: Cow<'static, str>,
74+
to: TypeNameOrValue<'py>,
7775
}
7876

7977
impl<'py> DowncastIntoError<'py> {
8078
/// Create a new `DowncastIntoError` representing a failure to convert the object
8179
/// `from` into the type named in `to`.
8280
pub fn new(from: Bound<'py, PyAny>, to: impl Into<Cow<'static, str>>) -> Self {
83-
DowncastIntoError {
81+
Self {
82+
from,
83+
to: TypeNameOrValue::Name(to.into()),
84+
}
85+
}
86+
87+
pub(crate) fn new_from_type(from: Bound<'py, PyAny>, to: Bound<'py, PyAny>) -> Self {
88+
Self {
8489
from,
85-
to: to.into(),
90+
to: TypeNameOrValue::Value(to),
8691
}
8792
}
8893

@@ -95,6 +100,12 @@ impl<'py> DowncastIntoError<'py> {
95100
}
96101
}
97102

103+
// Helper to store either a concrete type or a type name
104+
#[derive(Debug)]
105+
enum TypeNameOrValue<'py> {
106+
Name(Cow<'static, str>),
107+
Value(Bound<'py, PyAny>),
108+
}
98109
/// Helper conversion trait that allows to use custom arguments for lazy exception construction.
99110
pub trait PyErrArguments: Send + Sync {
100111
/// Arguments for exception
@@ -721,25 +732,33 @@ impl<'py> IntoPyObject<'py> for &PyErr {
721732

722733
struct PyDowncastErrorArguments {
723734
from: Py<PyType>,
724-
to: Cow<'static, str>,
735+
to: OwnedTypeNameOrValue,
725736
}
726737

727738
impl PyErrArguments for PyDowncastErrorArguments {
728739
fn arguments(self, py: Python<'_>) -> Py<PyAny> {
729-
const FAILED_TO_EXTRACT: Cow<'_, str> = Cow::Borrowed("<failed to extract type name>");
730740
let from = self.from.bind(py).qualname();
731-
let from = match &from {
732-
Ok(qn) => qn.to_cow().unwrap_or(FAILED_TO_EXTRACT),
733-
Err(_) => FAILED_TO_EXTRACT,
741+
let from = from
742+
.as_ref()
743+
.map(|name| name.to_string_lossy())
744+
.unwrap_or(Cow::Borrowed("<failed to extract type name>"));
745+
let to = match self.to {
746+
OwnedTypeNameOrValue::Name(name) => TypeNameOrValue::Name(name),
747+
OwnedTypeNameOrValue::Value(t) => TypeNameOrValue::Value(t.into_bound(py)),
734748
};
735-
format!("'{}' object cannot be converted to '{}'", from, self.to)
749+
format!("'{}' object cannot be converted to '{}'", from, to)
736750
.into_pyobject(py)
737751
.unwrap()
738752
.into_any()
739753
.unbind()
740754
}
741755
}
742756

757+
enum OwnedTypeNameOrValue {
758+
Name(Cow<'static, str>),
759+
Value(Py<PyAny>),
760+
}
761+
743762
/// Python exceptions that can be converted to [`PyErr`].
744763
///
745764
/// This is used to implement [`From<Bound<'_, T>> for PyErr`].
@@ -763,7 +782,10 @@ impl std::convert::From<DowncastError<'_, '_>> for PyErr {
763782
fn from(err: DowncastError<'_, '_>) -> PyErr {
764783
let args = PyDowncastErrorArguments {
765784
from: err.from.get_type().into(),
766-
to: err.to,
785+
to: match err.to {
786+
TypeNameOrValue::Name(name) => OwnedTypeNameOrValue::Name(name),
787+
TypeNameOrValue::Value(t) => OwnedTypeNameOrValue::Value(t.into()),
788+
},
767789
};
768790

769791
exceptions::PyTypeError::new_err(args)
@@ -783,7 +805,10 @@ impl std::convert::From<DowncastIntoError<'_>> for PyErr {
783805
fn from(err: DowncastIntoError<'_>) -> PyErr {
784806
let args = PyDowncastErrorArguments {
785807
from: err.from.get_type().into(),
786-
to: err.to,
808+
to: match err.to {
809+
TypeNameOrValue::Name(name) => OwnedTypeNameOrValue::Name(name),
810+
TypeNameOrValue::Value(t) => OwnedTypeNameOrValue::Value(t.into()),
811+
},
787812
};
788813

789814
exceptions::PyTypeError::new_err(args)
@@ -801,7 +826,7 @@ impl std::fmt::Display for DowncastIntoError<'_> {
801826
fn display_downcast_error(
802827
f: &mut std::fmt::Formatter<'_>,
803828
from: &Bound<'_, PyAny>,
804-
to: &str,
829+
to: &TypeNameOrValue<'_>,
805830
) -> std::fmt::Result {
806831
write!(
807832
f,
@@ -811,6 +836,32 @@ fn display_downcast_error(
811836
)
812837
}
813838

839+
impl std::fmt::Display for TypeNameOrValue<'_> {
840+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
841+
match self {
842+
Self::Name(name) => name.fmt(f),
843+
Self::Value(t) => {
844+
if let Ok(t) = t.downcast::<PyType>() {
845+
t.qualname()
846+
.map_err(|_| std::fmt::Error)?
847+
.to_string_lossy()
848+
.fmt(f)
849+
} else if let Ok(t) = t.downcast::<PyTuple>() {
850+
for (i, t) in t.iter().enumerate() {
851+
if i > 0 {
852+
f.write_str(" | ")?;
853+
}
854+
TypeNameOrValue::Value(t).fmt(f)?;
855+
}
856+
Ok(())
857+
} else {
858+
t.fmt(f)
859+
}
860+
}
861+
}
862+
}
863+
}
864+
814865
#[track_caller]
815866
pub fn panic_after_error(_py: Python<'_>) -> ! {
816867
unsafe {

0 commit comments

Comments
 (0)