Improve error messages for #[pyfunction] defined inside #[pymethods]

Make error message more specific when `#[pyfunction]` is used in `#[pymethods]`. Effectively, this replaces the error message: ``` error: static method needs #[staticmethod] attribute ``` To: ``` functions inside #[pymethods] do not need to be annotated with #[pyfunction] ``` Fixes #4340 Co-authored-by: László Vaskó <[email protected]>
PyO3 · Jul 14, 2024 · 9c54a0b · 9c54a0b
1 parent 0881fa0
commit 9c54a0b
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 0 deletions.
diff --git a/newsfragments/4349.fixed.md b/newsfragments/4349.fixed.md
@@ -0,0 +1 @@
+Improve error messages for `#[pyfunction]` defined inside `#[pymethods]`
diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs
@@ -10,6 +10,7 @@ use crate::{
 use proc_macro2::TokenStream;
 use pymethod::GeneratedPyMethod;
 use quote::{format_ident, quote};
+use syn::ImplItemFn;
 use syn::{
     parse::{Parse, ParseStream},
     spanned::Spanned,
@@ -84,6 +85,17 @@ pub fn build_py_methods(
     }
 }
 
+fn check_pyfunction(meth: &ImplItemFn) -> syn::Result<()> {
+    if meth
+        .attrs
+        .iter()
+        .any(|attr| attr.path().is_ident("pyfunction"))
+    {
+        bail_spanned!(meth.span() => "functions inside #[pymethods] do not need to be annotated with #[pyfunction]");
+    }
+    Ok(())
+}
+
 pub fn impl_methods(
     ty: &syn::Type,
     impls: &mut [syn::ImplItem],
@@ -103,6 +115,9 @@ pub fn impl_methods(
                 let ctx = &Ctx::new(&options.krate, Some(&meth.sig));
                 let mut fun_options = PyFunctionOptions::from_attrs(&mut meth.attrs)?;
                 fun_options.krate = fun_options.krate.or_else(|| options.krate.clone());
+
+                check_pyfunction(meth)?;
+
                 match pymethod::gen_py_method(ty, &mut meth.sig, &mut meth.attrs, fun_options, ctx)?
                 {
                     GeneratedPyMethod::Method(MethodAndMethodDef {

diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs
@@ -11,6 +11,7 @@ fn test_compile_errors() {
     t.compile_fail("tests/ui/invalid_pyclass_enum.rs");
     t.compile_fail("tests/ui/invalid_pyclass_item.rs");
     t.compile_fail("tests/ui/invalid_pyfunction_signatures.rs");
+    t.compile_fail("tests/ui/invalid_pyfunction_definition.rs");
     #[cfg(any(not(Py_LIMITED_API), Py_3_11))]
     t.compile_fail("tests/ui/invalid_pymethods_buffer.rs");
     // The output is not stable across abi3 / not abi3 and features

diff --git a/tests/ui/invalid_pyfunction_definition.rs b/tests/ui/invalid_pyfunction_definition.rs
@@ -0,0 +1,15 @@
+
+
+#[pyo3::pymodule]
+mod pyo3_scratch {
+    use pyo3::prelude::*;
+
+    #[pyclass]
+    struct Foo {}
+
+    #[pymethods]
+    impl Foo {
+        #[pyfunction]
+        fn bug() {}
+    }
+}
diff --git a/tests/ui/invalid_pyfunction_definition.stderr b/tests/ui/invalid_pyfunction_definition.stderr
@@ -0,0 +1,38 @@
+error: module is not supported in `trait`s or `impl`s
+  --> tests/ui/invalid_pyfunction_definition.rs:12:9
+   |
+12 |         #[pyfunction]
+   |         ^^^^^^^^^^^^^
+   |
+   = help: consider moving the module out to a nearby module scope
+   = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: implementation is not supported in `trait`s or `impl`s
+  --> tests/ui/invalid_pyfunction_definition.rs:12:9
+   |
+12 |         #[pyfunction]
+   |         ^^^^^^^^^^^^^
+   |
+   = help: consider moving the implementation out to a nearby module scope
+   = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: functions inside #[pymethods] do not need to be annotated with #[pyfunction]
+  --> tests/ui/invalid_pyfunction_definition.rs:12:9
+   |
+12 |         #[pyfunction]
+   |         ^
+
+error[E0425]: cannot find value `bug` in this scope
+  --> tests/ui/invalid_pyfunction_definition.rs:13:12
+   |
+13 |         fn bug() {}
+   |            ^^^
+   |            |
+   |            an associated function by that name is available on `Self` here
+   |            not found in this scope
+
+error[E0601]: `main` function not found in crate `$CRATE`
+  --> tests/ui/invalid_pyfunction_definition.rs:15:2
+   |
+15 | }
+   |  ^ consider adding a `main` function to `$DIR/tests/ui/invalid_pyfunction_definition.rs`