improve error message

pyo3-macros-backend/src/pymethod.rs

@@ -432,26 +432,38 @@ fn impl_traverse_slot(
     ctx: &Ctx,
 ) -> syn::Result<MethodAndSlotDef> {
     let Ctx { pyo3_path } = ctx;
-    if let (Some(py_arg), _) = split_off_python_arg(&spec.signature.arguments) {
+    let (py_arg, args) = split_off_python_arg(&spec.signature.arguments);
+    if let Some(py_arg) = py_arg {
         return Err(syn::Error::new_spanned(py_arg.ty, "__traverse__ may not take `Python`. \
-            Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. \
-            Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, \
-            i.e. `Python::with_gil` will panic."));
+            Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` \
+            should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited \
+            inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic."));
     }
 
+    // check that the receiver does not try to smuggle an (implicit) `Python` token into here
     if let FnType::Fn(SelfType::TryFromBoundRef(span))
     | FnType::Fn(SelfType::Receiver {
         mutable: true,
         span,
     }) = spec.tp
     {
         bail_spanned! { span =>
-            "__traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` \
+            "__traverse__ may not take a receiver other than `&self`. Usually, an implementation of \
+            `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` \
             should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited \
             inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic."
         }
     }
 
+    // check that we have a single argument (other than `&self`) of type `PyVisit`
+    if !matches!(args, [FnArg { ty: syn::Type::Path(p), .. }] if p.path.segments.last().map(|seg| seg.ident == "PyVisit").unwrap_or(false))
+    {
+        bail_spanned! { spec.name.span() =>
+            "`__traverse__` has to have the following signature: \
+            `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>`"
+        }
+    }
+
     let rust_fn_ident = spec.name;
 
     let associated_method = quote! {

tests/ui/traverse.rs

@@ -52,6 +52,36 @@ impl TraverseTriesToTakeSelf {
     }
 }
 
+#[pyclass]
+struct TraverseMissingPyVisit {}
+
+#[pymethods]
+impl TraverseMissingPyVisit {
+    fn __traverse__(&self) -> Result<(), PyTraverseError> {
+        Ok(())
+    }
+}
+
+#[pyclass]
+struct TraverseWrongArg {}
+
+#[pymethods]
+impl TraverseWrongArg {
+    fn __traverse__(&self, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> {
+        Ok(())
+    }
+}
+
+#[pyclass]
+struct TraverseAdditionalArg {}
+
+#[pymethods]
+impl TraverseAdditionalArg {
+    fn __traverse__(&self, visit: PyVisit, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> {
+        Ok(())
+    }
+}
+
 #[pyclass]
 struct Class;
 

tests/ui/traverse.stderr

@@ -1,29 +1,47 @@
-error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
+error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
   --> tests/ui/traverse.rs:10:26
    |
 10 |     fn __traverse__(slf: PyRef<Self>, visit: PyVisit) -> Result<(), PyTraverseError> {
    |                          ^^^^^
 
-error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
+error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
   --> tests/ui/traverse.rs:20:26
    |
 20 |     fn __traverse__(slf: PyRefMut<Self>, visit: PyVisit) -> Result<(), PyTraverseError> {
    |                          ^^^^^^^^
 
-error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
+error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
   --> tests/ui/traverse.rs:30:26
    |
 30 |     fn __traverse__(slf: Bound<'_, Self>, visit: PyVisit) -> Result<(), PyTraverseError> {
    |                          ^^^^^
 
-error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
+error: __traverse__ may not take a receiver other than `&self`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
   --> tests/ui/traverse.rs:40:21
    |
 40 |     fn __traverse__(&mut self, visit: PyVisit) -> Result<(), PyTraverseError> {
    |                     ^
 
-error: __traverse__ may not take `Python`. Usually, an implementation of `__traverse__` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
-  --> tests/ui/traverse.rs:60:32
+error: `__traverse__` has to have the following signature: `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>`
+  --> tests/ui/traverse.rs:60:8
    |
-60 |     fn __traverse__(&self, py: Python<'_>, visit: PyVisit<'_>) -> Result<(), PyTraverseError> {
+60 |     fn __traverse__(&self) -> Result<(), PyTraverseError> {
+   |        ^^^^^^^^^^^^
+
+error: `__traverse__` has to have the following signature: `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>`
+  --> tests/ui/traverse.rs:70:8
+   |
+70 |     fn __traverse__(&self, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> {
+   |        ^^^^^^^^^^^^
+
+error: `__traverse__` has to have the following signature: `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>`
+  --> tests/ui/traverse.rs:80:8
+   |
+80 |     fn __traverse__(&self, visit: PyVisit, arg: &Bound<'_, PyAny>) -> Result<(), PyTraverseError> {
+   |        ^^^^^^^^^^^^
+
+error: __traverse__ may not take `Python`. Usually, an implementation of `__traverse__(&self, visit: PyVisit<'_>) -> Result<(), PyTraverseError>` should do nothing but calls to `visit.call`. Most importantly, safe access to the GIL is prohibited inside implementations of `__traverse__`, i.e. `Python::with_gil` will panic.
+  --> tests/ui/traverse.rs:90:32
+   |
+90 |     fn __traverse__(&self, py: Python<'_>, visit: PyVisit<'_>) -> Result<(), PyTraverseError> {
    |                                ^^^^^^^^^^