diff --git a/newsfragments/6135.fixed.md b/newsfragments/6135.fixed.md new file mode 100644 index 00000000000..c308ceda5dd --- /dev/null +++ b/newsfragments/6135.fixed.md @@ -0,0 +1 @@ +Fix `#[pymethods]` so a regular method whose Python name starts with `get_`, `set_`, or `delete_` no longer collides with a property of the trimmed name (e.g. `fn get_x` alongside `#[getter] fn x`). diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index c529c40f4ff..f4c2574177a 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -372,7 +372,7 @@ pub fn impl_py_method_def( ctx: &Ctx, ) -> Result { let Ctx { pyo3_path, .. } = ctx; - let wrapper_ident = format_ident!("__pymethod_{}__", spec.python_name); + let wrapper_ident = format_ident!("__pymethod_method_{}__", spec.python_name); let calling_convention = CallingConvention::from_signature(&spec.signature); let associated_method = spec.get_wrapper_function( &wrapper_ident, diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index 582cfd89c5f..e61b450c126 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -317,3 +317,82 @@ fn test_optional_setter() { ); }) } + +// Regression test for #5974: previously, the wrapper generated for a regular +// method called `get_x` collided with the wrapper generated for a `#[getter]` +// of `x`, because both produced an associated function named +// `__pymethod_get_x__`. The same problem applied to setters/deleters and to +// `#[pyo3(name = ...)]` renames that happened to start with `get_`/`set_`/ +// `delete_`. The fix is to use a distinct infix for regular methods. +#[test] +fn property_and_regular_method_can_share_name_prefix() { + #[pyclass] + struct Object { + x: u32, + y: u32, + z: u32, + } + + #[pymethods] + impl Object { + #[getter] + fn x(&self) -> u32 { + self.x + } + + // Was previously a compile error: wrapper collided with the `x` getter. + fn get_x(&self) -> u32 { + self.x + 1 + } + + #[getter] + fn y(&self) -> u32 { + self.y + } + + // Was previously a compile error too: `#[pyo3(name = ...)]` is also + // routed through `python_name`. + #[pyo3(name = "get_y")] + fn y_get(&self) -> u32 { + self.y + 2 + } + + #[setter] + fn z(&mut self, value: u32) { + self.z = value; + } + + // Same collision pattern as above, but for setters. + fn set_z(&mut self, value: u32) { + self.z = value + 1; + } + } + + Python::attach(|py| { + let instance = Py::new( + py, + Object { + x: 10, + y: 20, + z: 30, + }, + ) + .unwrap(); + py_run!( + py, + instance, + "assert instance.x == 10 and instance.get_x() == 11" + ); + py_run!( + py, + instance, + "assert instance.y == 20 and instance.get_y() == 22" + ); + // `z` has only a setter; we exercise both the setter and the regular + // method by routing reads through the borrowed Rust value. + py_run!(py, instance, "instance.z = 5"); + assert_eq!(instance.borrow(py).z, 5); + py_run!(py, instance, "instance.set_z(5)"); + assert_eq!(instance.borrow(py).z, 6); + }) +} diff --git a/tests/ui/invalid_pyclass_generic.rs b/tests/ui/invalid_pyclass_generic.rs index 23524bb1237..f79fc7c147b 100644 --- a/tests/ui/invalid_pyclass_generic.rs +++ b/tests/ui/invalid_pyclass_generic.rs @@ -2,7 +2,7 @@ use pyo3::prelude::*; use pyo3::types::PyType; #[pyclass(generic)] -//~^ ERROR: duplicate definitions with name `__pymethod___class_getitem____` +//~^ ERROR: duplicate definitions with name `__pymethod_method___class_getitem____` //~| ERROR: duplicate definitions with name `__class_getitem__` //~| ERROR: multiple applicable items in scope //~| ERROR: multiple applicable items in scope diff --git a/tests/ui/invalid_pyclass_generic.stderr b/tests/ui/invalid_pyclass_generic.stderr index 1a99c4444d4..1d6b38ede87 100644 --- a/tests/ui/invalid_pyclass_generic.stderr +++ b/tests/ui/invalid_pyclass_generic.stderr @@ -1,11 +1,11 @@ -error[E0592]: duplicate definitions with name `__pymethod___class_getitem____` +error[E0592]: duplicate definitions with name `__pymethod_method___class_getitem____` --> tests/ui/invalid_pyclass_generic.rs:4:1 | 4 | #[pyclass(generic)] - | ^^^^^^^^^^^^^^^^^^^ duplicate definitions for `__pymethod___class_getitem____` + | ^^^^^^^^^^^^^^^^^^^ duplicate definitions for `__pymethod_method___class_getitem____` ... 12 | #[pymethods] - | ------------ other definition for `__pymethod___class_getitem____` + | ------------ other definition for `__pymethod_method___class_getitem____` | = note: this error originates in the macro `::pyo3::impl_::pymethods::maybe_define_fastcall_function_with_keywords` which comes from the expansion of the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -29,7 +29,7 @@ error[E0034]: multiple applicable items in scope --> tests/ui/invalid_pyclass_generic.rs:4:1 | 4 | #[pyclass(generic)] - | ^^^^^^^^^^^^^^^^^^^ multiple `__pymethod___class_getitem____` found + | ^^^^^^^^^^^^^^^^^^^ multiple `__pymethod_method___class_getitem____` found | note: candidate #1 is defined in an impl for the type `ClassRedefinesClassGetItem` --> tests/ui/invalid_pyclass_generic.rs:4:1 @@ -92,7 +92,7 @@ error[E0034]: multiple applicable items in scope --> tests/ui/invalid_pyclass_generic.rs:20:12 | 20 | pub fn __class_getitem__( - | ^^^^^^^^^^^^^^^^^ multiple `__pymethod___class_getitem____` found + | ^^^^^^^^^^^^^^^^^ multiple `__pymethod_method___class_getitem____` found | note: candidate #1 is defined in an impl for the type `ClassRedefinesClassGetItem` --> tests/ui/invalid_pyclass_generic.rs:4:1 diff --git a/tests/ui/invalid_pymethods_duplicates.rs b/tests/ui/invalid_pymethods_duplicates.rs index 018e2b80c12..5c842dfea53 100644 --- a/tests/ui/invalid_pymethods_duplicates.rs +++ b/tests/ui/invalid_pymethods_duplicates.rs @@ -25,7 +25,7 @@ impl TwoNew { struct DuplicateMethod {} #[pymethods] -//~^ ERROR: duplicate definitions with name `__pymethod_func__` +//~^ ERROR: duplicate definitions with name `__pymethod_method_func__` impl DuplicateMethod { #[pyo3(name = "func")] fn func_a(&self) {} diff --git a/tests/ui/invalid_pymethods_duplicates.stderr b/tests/ui/invalid_pymethods_duplicates.stderr index 6856ee97863..a8d0817ccf3 100644 --- a/tests/ui/invalid_pymethods_duplicates.stderr +++ b/tests/ui/invalid_pymethods_duplicates.stderr @@ -20,14 +20,14 @@ error[E0592]: 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[E0592]: duplicate definitions with name `__pymethod_func__` +error[E0592]: duplicate definitions with name `__pymethod_method_func__` --> tests/ui/invalid_pymethods_duplicates.rs:27:1 | 27 | #[pymethods] | ^^^^^^^^^^^^ | | - | duplicate definitions for `__pymethod_func__` - | other definition for `__pymethod_func__` + | duplicate definitions for `__pymethod_method_func__` + | other definition for `__pymethod_method_func__` | = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)