From 139cae166aa09cfba1d510accde202ae6f0acd81 Mon Sep 17 00:00:00 2001
From: mejrs <brunokolenbrander@hotmail.com>
Date: Sun, 1 May 2022 11:23:09 +0200
Subject: [PATCH] Improve `IntoPyCallbackOutput` compile error

---
 pyo3-macros-backend/src/method.rs         | 12 +++++++++--
 pyo3-macros-backend/src/pymethod.rs       | 19 ++++++++++++++++-
 src/callback.rs                           | 24 +++++++++++++++++++++
 tests/ui/invalid_result_conversion.stderr | 26 +++++++++++------------
 4 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs
index ddf2244f199..d0209c67f5e 100644
--- a/pyo3-macros-backend/src/method.rs
+++ b/pyo3-macros-backend/src/method.rs
@@ -478,8 +478,16 @@ impl<'a> FnSpec<'a> {
         } else {
             quote!(#func_name)
         };
-        let rust_call =
-            quote! { _pyo3::callback::convert(#py, #rust_name(#self_arg #(#arg_names),*)) };
+
+        // The method call is necessary to generate a decent error message.
+        let rust_call = quote! {
+            let ret = #rust_name(#self_arg #(#arg_names),*);
+
+            use _pyo3::callback::IntoResult;
+            let ret: _pyo3::PyResult<_> = ret.into_result();
+
+            _pyo3::callback::convert(#py, ret)
+        };
         let krate = &self.krate;
         Ok(match self.convention {
             CallingConvention::Noargs => {
diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs
index 9ed9b4300b8..9d5a1bbf20f 100644
--- a/pyo3-macros-backend/src/pymethod.rs
+++ b/pyo3-macros-backend/src/pymethod.rs
@@ -495,6 +495,22 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType<'_>) -> R
             self_type.receiver(cls, ExtractErrorMode::Raise)
         }
     };
+
+    let conversion = match property_type {
+        PropertyType::Descriptor { .. } => {
+            quote! {
+                let item: _pyo3::Py<_pyo3::PyAny> = _pyo3::IntoPy::into_py(item, _py);
+                ::std::result::Result::Ok(_pyo3::conversion::IntoPyPointer::into_ptr(item))
+            }
+        }
+        // Forward to `IntoPyCallbackOutput`, to handle `#[getter]`s returning results.
+        PropertyType::Function { .. } => {
+            quote! {
+                _pyo3::callback::convert(_py, item)
+            }
+        }
+    };
+
     Ok(quote! {
         _pyo3::class::PyMethodDefType::Getter({
             #deprecations
@@ -509,7 +525,8 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType<'_>) -> R
                         let _py = gil.python();
                         _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #slf
-                            _pyo3::callback::convert(_py, #getter_impl)
+                            let item = #getter_impl;
+                            #conversion
                         }))
                     }
                     __wrap
diff --git a/src/callback.rs b/src/callback.rs
index 3794b9e5906..81fb4d1d5b1 100644
--- a/src/callback.rs
+++ b/src/callback.rs
@@ -35,6 +35,30 @@ impl PyCallbackOutput for () {
     const ERR_VALUE: Self = ();
 }
 
+#[doc(hidden)]
+pub trait IntoResult<R> {
+    fn into_result(self) -> R;
+}
+
+impl<T> IntoResult<PyResult<T>> for T
+where
+    T: IntoPy<PyObject>,
+{
+    fn into_result(self) -> PyResult<Self> {
+        Ok(self)
+    }
+}
+
+impl<T, E> IntoResult<PyResult<T>> for Result<T, E>
+where
+    T: IntoPy<PyObject>,
+    E: Into<PyErr>,
+{
+    fn into_result(self) -> PyResult<T> {
+        self.map_err(Into::into)
+    }
+}
+
 /// Convert the result of callback function into the appropriate return value.
 pub trait IntoPyCallbackOutput<Target> {
     fn convert(self, py: Python<'_>) -> PyResult<Target>;
diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr
index a606f605fcf..2e3e2be3c74 100644
--- a/tests/ui/invalid_result_conversion.stderr
+++ b/tests/ui/invalid_result_conversion.stderr
@@ -1,14 +1,12 @@
-error[E0277]: the trait bound `Result<(), MyError>: IntoPyCallbackOutput<_>` is not satisfied
-   --> tests/ui/invalid_result_conversion.rs:21:1
-    |
-21  | #[pyfunction]
-    | ^^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Result<(), MyError>`
-    |
-    = help: the following implementations were found:
-              <Result<T, E> as IntoPyCallbackOutput<U>>
-note: required by a bound in `pyo3::callback::convert`
-   --> src/callback.rs:182:8
-    |
-182 |     T: IntoPyCallbackOutput<U>,
-    |        ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `pyo3::callback::convert`
-    = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
+error[E0599]: the method `into_result` exists for enum `Result<(), MyError>`, but its trait bounds were not satisfied
+  --> tests/ui/invalid_result_conversion.rs:21:1
+   |
+21 | #[pyfunction]
+   | ^^^^^^^^^^^^^ method cannot be called on `Result<(), MyError>` due to unsatisfied trait bounds
+   |
+   = note: the following trait bounds were not satisfied:
+           `&Result<(), MyError>: IntoPy<Py<PyAny>>`
+           which is required by `&Result<(), MyError>: IntoResult<Result<&Result<(), MyError>, PyErr>>`
+           `&mut Result<(), MyError>: IntoPy<Py<PyAny>>`
+           which is required by `&mut Result<(), MyError>: IntoResult<Result<&mut Result<(), MyError>, PyErr>>`
+   = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)