error when `#[pyo3(signature = ())]` used on invalid methods

This commit is contained in:
David Hewitt 2022-11-15 20:10:19 +00:00
parent f5735614fb
commit 00fc0358b9
13 changed files with 227 additions and 118 deletions

View File

@ -11,8 +11,8 @@ The magic methods handled by PyO3 are very similar to the standard Python ones o
- Magic methods for the buffer protocol
When PyO3 handles a magic method, a couple of changes apply compared to other `#[pymethods]`:
- The `#[pyo3(text_signature = "...")]` attribute is not allowed
- The signature is restricted to match the magic method
- The Rust function signature is restricted to match the magic method.
- The `#[pyo3(signature = (...)]` and `#[pyo3(text_signature = "...")]` attributes are not allowed.
The following sections list of all magic methods PyO3 currently handles. The
given signatures should be interpreted as follows:

View File

@ -5,8 +5,8 @@ use std::borrow::Cow;
use crate::attributes::TextSignatureAttribute;
use crate::deprecations::{Deprecation, Deprecations};
use crate::params::impl_arg_params;
use crate::pyfunction::PyFunctionOptions;
use crate::pyfunction::{DeprecatedArgs, FunctionSignature, PyFunctionArgPyO3Attributes};
use crate::pyfunction::{PyFunctionOptions, SignatureAttribute};
use crate::utils::{self, PythonDoc};
use proc_macro2::{Span, TokenStream};
use quote::ToTokens;
@ -282,7 +282,7 @@ impl<'a> FnSpec<'a> {
let (fn_type, skip_first_arg, fixed_convention) =
Self::parse_fn_type(sig, fn_type_attr, &mut python_name)?;
Self::ensure_text_signature_on_valid_method(&fn_type, text_signature.as_ref())?;
ensure_signatures_on_valid_method(&fn_type, signature.as_ref(), text_signature.as_ref())?;
let name = &sig.ident;
let ty = get_return_info(&sig.output);
@ -341,26 +341,6 @@ impl<'a> FnSpec<'a> {
syn::LitStr::new(&format!("{}\0", self.python_name), self.python_name.span())
}
fn ensure_text_signature_on_valid_method(
fn_type: &FnType,
text_signature: Option<&TextSignatureAttribute>,
) -> syn::Result<()> {
if let Some(text_signature) = text_signature {
match &fn_type {
FnType::FnNew => bail_spanned!(
text_signature.kw.span() =>
"text_signature not allowed on __new__; if you want to add a signature on \
__new__, put it on the struct definition instead"
),
FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => bail_spanned!(
text_signature.kw.span() => "text_signature not allowed with this method type"
),
_ => {}
}
}
Ok(())
}
fn parse_fn_type(
sig: &syn::Signature,
fn_type_attr: Option<MethodTypeAttribute>,
@ -742,3 +722,44 @@ const IMPL_TRAIT_ERR: &str = "Python functions cannot have `impl Trait` argument
const RECEIVER_BY_VALUE_ERR: &str =
"Python objects are shared, so 'self' cannot be moved out of the Python interpreter.
Try `&self`, `&mut self, `slf: PyRef<'_, Self>` or `slf: PyRefMut<'_, Self>`.";
fn ensure_signatures_on_valid_method(
fn_type: &FnType,
signature: Option<&SignatureAttribute>,
text_signature: Option<&TextSignatureAttribute>,
) -> syn::Result<()> {
if let Some(signature) = signature {
match fn_type {
FnType::Getter(_) => {
bail_spanned!(signature.kw.span() => "`signature` not allowed with `getter`")
}
FnType::Setter(_) => {
bail_spanned!(signature.kw.span() => "`signature` not allowed with `setter`")
}
FnType::ClassAttribute => {
bail_spanned!(signature.kw.span() => "`signature` not allowed with `classattr`")
}
_ => {}
}
}
if let Some(text_signature) = text_signature {
match fn_type {
FnType::FnNew => bail_spanned!(
text_signature.kw.span() =>
"`text_signature` not allowed on `__new__`; if you want to add a signature on \
`__new__`, put it on the struct definition instead"
),
FnType::Getter(_) => {
bail_spanned!(text_signature.kw.span() => "`text_signature` not allowed with `getter`")
}
FnType::Setter(_) => {
bail_spanned!(text_signature.kw.span() => "`text_signature` not allowed with `setter`")
}
FnType::ClassAttribute => {
bail_spanned!(text_signature.kw.span() => "`text_signature` not allowed with `classattr`")
}
_ => {}
}
}
Ok(())
}

View File

@ -214,6 +214,7 @@ pub struct PythonSignature {
pub struct FunctionSignature<'a> {
pub arguments: Vec<FnArg<'a>>,
pub python_signature: PythonSignature,
pub attribute: Option<SignatureAttribute>,
}
pub enum ParseState {
@ -362,7 +363,7 @@ impl<'a> FunctionSignature<'a> {
),
};
for item in attribute.value.items {
for item in &attribute.value.items {
match item {
SignatureItem::Argument(arg) => {
let fn_arg = next_argument_checked(&arg.ident)?;
@ -372,8 +373,8 @@ impl<'a> FunctionSignature<'a> {
arg.eq_and_default.is_none(),
arg.span(),
)?;
if let Some((_, default)) = arg.eq_and_default {
fn_arg.default = Some(default);
if let Some((_, default)) = &arg.eq_and_default {
fn_arg.default = Some(default.clone());
}
}
SignatureItem::VarargsSep(sep) => parse_state.finish_pos_args(sep.span())?,
@ -402,6 +403,7 @@ impl<'a> FunctionSignature<'a> {
Ok(FunctionSignature {
arguments,
python_signature,
attribute: Some(attribute),
})
}
@ -506,6 +508,7 @@ impl<'a> FunctionSignature<'a> {
keyword_only_parameters,
accepts_kwargs,
},
attribute: None,
})
}
@ -541,6 +544,7 @@ impl<'a> FunctionSignature<'a> {
Self {
arguments,
python_signature,
attribute: None,
}
}
}

View File

@ -196,7 +196,7 @@ pub fn gen_py_method(
GeneratedPyMethod::Method(impl_py_class_attribute(cls, spec)?)
}
(PyMethodKind::Proto(proto_kind), _) => {
ensure_no_forbidden_protocol_attributes(spec, &method.method_name)?;
ensure_no_forbidden_protocol_attributes(&proto_kind, spec, &method.method_name)?;
match proto_kind {
PyMethodProtoKind::Slot(slot_def) => {
let slot = slot_def.generate_type_slot(cls, spec, &method.method_name)?;
@ -206,7 +206,7 @@ pub fn gen_py_method(
GeneratedPyMethod::Proto(impl_call_slot(cls, method.spec)?)
}
PyMethodProtoKind::Traverse => {
GeneratedPyMethod::Proto(impl_traverse_slot(cls, method.spec))
GeneratedPyMethod::Proto(impl_traverse_slot(cls, &spec.name))
}
PyMethodProtoKind::SlotFragment(slot_fragment_def) => {
let proto = slot_fragment_def.generate_pyproto_fragment(cls, spec)?;
@ -263,11 +263,18 @@ fn ensure_function_options_valid(options: &PyFunctionOptions) -> syn::Result<()>
}
fn ensure_no_forbidden_protocol_attributes(
proto_kind: &PyMethodProtoKind,
spec: &FnSpec<'_>,
method_name: &str,
) -> syn::Result<()> {
if let Some(signature) = &spec.signature.attribute {
// __call__ is allowed to have a signature, but nothing else is.
if !matches!(proto_kind, PyMethodProtoKind::Call) {
bail_spanned!(signature.kw.span() => format!("`signature` cannot be used with magic method `{}`", method_name));
}
}
if let Some(text_signature) = &spec.text_signature {
bail_spanned!(text_signature.kw.span() => format!("`text_signature` cannot be used with `{}`", method_name));
bail_spanned!(text_signature.kw.span() => format!("`text_signature` cannot be used with magic method `{}`", method_name));
}
Ok(())
}
@ -360,8 +367,7 @@ fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec<'_>) -> Result<MethodAndSlot
})
}
fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec<'_>) -> MethodAndSlotDef {
let ident = spec.name;
fn impl_traverse_slot(cls: &syn::Type, rust_fn_ident: &syn::Ident) -> MethodAndSlotDef {
let associated_method = quote! {
pub unsafe extern "C" fn __pymethod_traverse__(
slf: *mut _pyo3::ffi::PyObject,
@ -377,7 +383,7 @@ fn impl_traverse_slot(cls: &syn::Type, spec: FnSpec<'_>) -> MethodAndSlotDef {
let visit = _pyo3::class::gc::PyVisit::from_raw(visit, arg, py);
let borrow = slf.try_borrow();
let retval = if let ::std::result::Result::Ok(borrow) = borrow {
_pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#ident(visit))
_pyo3::impl_::pymethods::unwrap_traverse_result(borrow.#rust_fn_ident(visit))
} else {
0
};

View File

@ -35,6 +35,7 @@ fn _test_compile_errors() {
t.compile_fail("tests/ui/invalid_macro_args.rs");
t.compile_fail("tests/ui/invalid_need_module_arg_position.rs");
t.compile_fail("tests/ui/invalid_property_args.rs");
t.compile_fail("tests/ui/invalid_proto_pymethods.rs");
t.compile_fail("tests/ui/invalid_pyclass_args.rs");
t.compile_fail("tests/ui/invalid_pyclass_enum.rs");
t.compile_fail("tests/ui/invalid_pyclass_item.rs");
@ -44,8 +45,6 @@ fn _test_compile_errors() {
t.compile_fail("tests/ui/invalid_pymethod_names.rs");
t.compile_fail("tests/ui/invalid_pymodule_args.rs");
t.compile_fail("tests/ui/reject_generics.rs");
t.compile_fail("tests/ui/invalid_pymethod_proto_args.rs");
t.compile_fail("tests/ui/invalid_pymethod_proto_args_py.rs");
tests_rust_1_49(&t);
tests_rust_1_56(&t);

View File

@ -0,0 +1,51 @@
//! Check that some magic methods edge cases error as expected.
//!
//! For convenience use #[pyo3(name = "__some_dunder__")] to create the methods,
//! so that the function names can describe the edge case to be rejected.
use pyo3::prelude::*;
#[pyclass]
struct MyClass {}
//
// Argument counts
//
#[pymethods]
impl MyClass {
#[pyo3(name = "__truediv__")]
fn truediv_expects_one_argument(&self) -> PyResult<()> {
Ok(())
}
}
#[pymethods]
impl MyClass {
#[pyo3(name = "__truediv__")]
fn truediv_expects_one_argument_py(&self, _py: Python<'_>) -> PyResult<()> {
Ok(())
}
}
//
// Forbidden attributes
//
#[pymethods]
impl MyClass {
#[pyo3(name = "__bool__", signature = ())]
fn signature_is_forbidden(&self) -> bool {
true
}
}
#[pymethods]
impl MyClass {
#[pyo3(name = "__bool__", text_signature = "")]
fn text_signature_is_forbidden(&self) -> bool {
true
}
}
fn main() {}

View File

@ -0,0 +1,23 @@
error: Expected 1 arguments, got 0
--> tests/ui/invalid_proto_pymethods.rs:18:8
|
18 | fn truediv_expects_one_argument(&self) -> PyResult<()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: Expected 1 arguments, got 0
--> tests/ui/invalid_proto_pymethods.rs:26:8
|
26 | fn truediv_expects_one_argument_py(&self, _py: Python<'_>) -> PyResult<()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: `signature` cannot be used with magic method `__bool__`
--> tests/ui/invalid_proto_pymethods.rs:37:31
|
37 | #[pyo3(name = "__bool__", signature = ())]
| ^^^^^^^^^
error: `text_signature` cannot be used with magic method `__bool__`
--> tests/ui/invalid_proto_pymethods.rs:45:31
|
45 | #[pyo3(name = "__bool__", text_signature = "")]
| ^^^^^^^^^^^^^^

View File

@ -1,13 +0,0 @@
use pyo3::prelude::*;
#[pyclass]
struct MyClass {}
#[pymethods]
impl MyClass {
fn __truediv__(&self) -> PyResult<()> {
Ok(())
}
}
fn main() {}

View File

@ -1,5 +0,0 @@
error: Expected 1 arguments, got 0
--> tests/ui/invalid_pymethod_proto_args.rs:8:8
|
8 | fn __truediv__(&self) -> PyResult<()> {
| ^^^^^^^^^^^

View File

@ -1,13 +0,0 @@
use pyo3::prelude::*;
#[pyclass]
struct MyClass {}
#[pymethods]
impl MyClass {
fn __truediv__(&self, _py: Python<'_>) -> PyResult<()> {
Ok(())
}
}
fn main() {}

View File

@ -1,5 +0,0 @@
error: Expected 1 arguments, got 0
--> tests/ui/invalid_pymethod_proto_args_py.rs:8:8
|
8 | fn __truediv__(&self, _py: Python<'_>) -> PyResult<()> {
| ^^^^^^^^^^^

View File

@ -54,8 +54,8 @@ impl MyClass {
#[pymethods]
impl MyClass {
#[pyo3(text_signature = "()")]
fn __call__(&self) {}
#[pyo3(name = "__call__", text_signature = "()")]
fn text_signature_on_call() {}
}
#[pymethods]
@ -79,6 +79,27 @@ impl MyClass {
fn text_signature_on_classattr() {}
}
#[pymethods]
impl MyClass {
#[getter(x)]
#[pyo3(signature = ())]
fn signature_on_getter(&self) {}
}
#[pymethods]
impl MyClass {
#[setter(x)]
#[pyo3(signature = ())]
fn signature_on_setter(&self) {}
}
#[pymethods]
impl MyClass {
#[classattr]
#[pyo3(signature = ())]
fn signature_on_classattr() {}
}
#[pymethods]
impl MyClass {
#[classattr]
@ -91,7 +112,6 @@ impl MyClass {
fn generic_method<T>(value: T) {}
}
#[pymethods]
impl MyClass {
fn impl_trait_method_first_arg(impl_trait: impl AsRef<PyAny>) {}
@ -123,10 +143,14 @@ struct TwoNew { }
#[pymethods]
impl TwoNew {
#[new]
fn new_1() -> Self { Self { } }
fn new_1() -> Self {
Self {}
}
#[new]
fn new_2() -> Self { Self { } }
fn new_2() -> Self {
Self {}
}
}
struct DuplicateMethod {}
@ -140,5 +164,4 @@ impl DuplicateMethod {
fn func_b(&self) {}
}
fn main() {}

View File

@ -34,85 +34,103 @@ error: expected receiver for #[setter]
45 | fn setter_without_receiver() {}
| ^^
error: text_signature not allowed on __new__; if you want to add a signature on __new__, put it on the struct definition instead
error: `text_signature` not allowed on `__new__`; if you want to add a signature on `__new__`, put it on the struct definition instead
--> tests/ui/invalid_pymethods.rs:51:12
|
51 | #[pyo3(text_signature = "()")]
| ^^^^^^^^^^^^^^
error: `text_signature` cannot be used with `__call__`
--> tests/ui/invalid_pymethods.rs:57:12
error: static method needs #[staticmethod] attribute
--> tests/ui/invalid_pymethods.rs:58:5
|
57 | #[pyo3(text_signature = "()")]
| ^^^^^^^^^^^^^^
58 | fn text_signature_on_call() {}
| ^^
error: text_signature not allowed with this method type
error: `text_signature` not allowed with `getter`
--> tests/ui/invalid_pymethods.rs:64:12
|
64 | #[pyo3(text_signature = "()")]
| ^^^^^^^^^^^^^^
error: text_signature not allowed with this method type
error: `text_signature` not allowed with `setter`
--> tests/ui/invalid_pymethods.rs:71:12
|
71 | #[pyo3(text_signature = "()")]
| ^^^^^^^^^^^^^^
error: text_signature not allowed with this method type
error: `text_signature` not allowed with `classattr`
--> tests/ui/invalid_pymethods.rs:78:12
|
78 | #[pyo3(text_signature = "()")]
| ^^^^^^^^^^^^^^
error: cannot specify a second method type
--> tests/ui/invalid_pymethods.rs:85:7
error: `signature` not allowed with `getter`
--> tests/ui/invalid_pymethods.rs:85:12
|
85 | #[staticmethod]
85 | #[pyo3(signature = ())]
| ^^^^^^^^^
error: `signature` not allowed with `setter`
--> tests/ui/invalid_pymethods.rs:92:12
|
92 | #[pyo3(signature = ())]
| ^^^^^^^^^
error: `signature` not allowed with `classattr`
--> tests/ui/invalid_pymethods.rs:99:12
|
99 | #[pyo3(signature = ())]
| ^^^^^^^^^
error: cannot specify a second method type
--> tests/ui/invalid_pymethods.rs:106:7
|
106 | #[staticmethod]
| ^^^^^^^^^^^^
error: Python functions cannot have generic type parameters
--> tests/ui/invalid_pymethods.rs:91:23
--> tests/ui/invalid_pymethods.rs:112:23
|
91 | fn generic_method<T>(value: T) {}
112 | fn generic_method<T>(value: T) {}
| ^
error: Python functions cannot have `impl Trait` arguments
--> tests/ui/invalid_pymethods.rs:97:48
--> tests/ui/invalid_pymethods.rs:117:48
|
97 | fn impl_trait_method_first_arg(impl_trait: impl AsRef<PyAny>) {}
117 | fn impl_trait_method_first_arg(impl_trait: impl AsRef<PyAny>) {}
| ^^^^
error: Python functions cannot have `impl Trait` arguments
--> tests/ui/invalid_pymethods.rs:102:56
--> tests/ui/invalid_pymethods.rs:122:56
|
102 | fn impl_trait_method_second_arg(&self, impl_trait: impl AsRef<PyAny>) {}
122 | fn impl_trait_method_second_arg(&self, impl_trait: impl AsRef<PyAny>) {}
| ^^^^
error: `async fn` is not yet supported for Python functions.
Additional crates such as `pyo3-asyncio` can be used to integrate async Rust and Python. For more information, see https://github.com/PyO3/pyo3/issues/1632
--> tests/ui/invalid_pymethods.rs:107:5
--> tests/ui/invalid_pymethods.rs:127:5
|
107 | async fn async_method(&self) {}
127 | async fn async_method(&self) {}
| ^^^^^
error: `pass_module` cannot be used on Python methods
--> tests/ui/invalid_pymethods.rs:112:12
--> tests/ui/invalid_pymethods.rs:132:12
|
112 | #[pyo3(pass_module)]
132 | #[pyo3(pass_module)]
| ^^^^^^^^^^^
error: Python objects are shared, so 'self' cannot be moved out of the Python interpreter.
Try `&self`, `&mut self, `slf: PyRef<'_, Self>` or `slf: PyRefMut<'_, Self>`.
--> tests/ui/invalid_pymethods.rs:118:29
--> tests/ui/invalid_pymethods.rs:138:29
|
118 | fn method_self_by_value(self){}
138 | fn method_self_by_value(self) {}
| ^^^^
error[E0201]: duplicate definitions with name `__pymethod___new____`:
--> tests/ui/invalid_pymethods.rs:123:1
--> tests/ui/invalid_pymethods.rs:143:1
|
123 | #[pymethods]
143 | #[pymethods]
| ^^^^^^^^^^^^
| |
| previous definition of `__pymethod___new____` here
@ -121,9 +139,9 @@ error[E0201]: duplicate definitions with name `__pymethod___new____`:
= note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0201]: duplicate definitions with name `__pymethod_func__`:
--> tests/ui/invalid_pymethods.rs:134:1
--> tests/ui/invalid_pymethods.rs:158:1
|
134 | #[pymethods]
158 | #[pymethods]
| ^^^^^^^^^^^^
| |
| previous definition of `__pymethod_func__` here