From 23778f5386f5254f4c5ad038a39a295ef779ead2 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sun, 7 Nov 2021 08:16:15 +0000 Subject: [PATCH] pymethods: test and document opt-out of protos --- CHANGELOG.md | 10 +++ guide/src/class/protocols.md | 37 ++++++++ pyo3-macros-backend/src/konst.rs | 21 +++-- pyo3-macros-backend/src/pyimpl.rs | 9 +- pyo3-macros-backend/src/pymethod.rs | 130 ++++++++++++++++++++-------- tests/test_proto_methods.rs | 74 +++++++++++++++- 6 files changed, 234 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fdb5114..2d5bc19e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ PyO3 versions, please see the [migration guide](https://pyo3.rs/latest/migration The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- `#[classattr]` constants with a known magic method name (which is lowercase) no longer trigger lint warnings expecting constants to be uppercase. [#1969](https://github.com/PyO3/pyo3/pull/1969) + +### Fixed + +- Fix creating `#[classattr]` by functions with the name of a known magic method. [#1969](https://github.com/PyO3/pyo3/pull/1969) + ## [0.15.0] - 2021-11-03 ### Packaging diff --git a/guide/src/class/protocols.md b/guide/src/class/protocols.md index 251447be..cc3e11c3 100644 --- a/guide/src/class/protocols.md +++ b/guide/src/class/protocols.md @@ -47,6 +47,25 @@ given signatures should be interpreted as follows: - `__str__() -> object (str)` - `__repr__() -> object (str)` - `__hash__() -> isize` +
+ Disabling Python's default hash + + By default, all `#[pyclass]` types have a default hash implementation from Python. Types which should not be hashable can override this by setting `__hash__` to `None`. This is the same mechanism as for a pure-Python class. This is done like so: + + ```rust + # use pyo3::prelude::*; + # + #[pyclass] + struct NotHashable { } + + #[pymethods] + impl NotHashable { + #[classattr] + const __hash__: Option = None; + } + ``` +
+ - `__richcmp__(, object, pyo3::basic::CompareOp) -> object` - `__getattr__(, object) -> object` - `__setattr__(, object, object) -> ()` @@ -140,6 +159,24 @@ TODO; see [#1884](https://github.com/PyO3/pyo3/issues/1884) - `__len__() -> usize` - `__contains__(, object) -> bool` +
+ Disabling Python's default contains + + By default, all `#[pyclass]` types with an `__iter__` method support a default implementation of the `in` operator. Types which do not want this can override this by setting `__contains__` to `None`. This is the same mechanism as for a pure-Python class. This is done like so: + + ```rust + # use pyo3::prelude::*; + # + #[pyclass] + struct NoContains { } + + #[pymethods] + impl NoContains { + #[classattr] + const __contains__: Option = None; + } + ``` +
- `__getitem__(, object) -> object` - `__setitem__(, object, object) -> ()` - `__delitem__(, object) -> ()` diff --git a/pyo3-macros-backend/src/konst.rs b/pyo3-macros-backend/src/konst.rs index ddece4f0..d0bc458d 100644 --- a/pyo3-macros-backend/src/konst.rs +++ b/pyo3-macros-backend/src/konst.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use crate::{ attributes::{ self, get_deprecated_name_attribute, get_pyo3_options, is_attribute_ident, take_attributes, @@ -5,7 +7,7 @@ use crate::{ }, deprecations::Deprecations, }; -use proc_macro2::TokenStream; +use proc_macro2::{Ident, TokenStream}; use quote::quote; use syn::{ ext::IdentExt, @@ -20,15 +22,18 @@ pub struct ConstSpec { } impl ConstSpec { + pub fn python_name(&self) -> Cow { + if let Some(name) = &self.attributes.name { + Cow::Borrowed(&name.0) + } else { + Cow::Owned(self.rust_ident.unraw()) + } + } + /// Null-terminated Python name pub fn null_terminated_python_name(&self) -> TokenStream { - if let Some(name) = &self.attributes.name { - let name = format!("{}\0", name.0); - quote!({#name}) - } else { - let name = format!("{}\0", self.rust_ident.unraw().to_string()); - quote!(#name) - } + let name = format!("{}\0", self.python_name()); + quote!({#name}) } } diff --git a/pyo3-macros-backend/src/pyimpl.rs b/pyo3-macros-backend/src/pyimpl.rs index 885c2540..54e248c9 100644 --- a/pyo3-macros-backend/src/pyimpl.rs +++ b/pyo3-macros-backend/src/pyimpl.rs @@ -5,7 +5,7 @@ use std::collections::HashSet; use crate::{ konst::{ConstAttributes, ConstSpec}, pyfunction::PyFunctionOptions, - pymethod, + pymethod::{self, is_proto_method}, }; use proc_macro2::TokenStream; use pymethod::GeneratedPyMethod; @@ -79,6 +79,13 @@ pub fn impl_methods( let attrs = get_cfg_attributes(&konst.attrs); let meth = gen_py_const(ty, &spec); methods.push(quote!(#(#attrs)* #meth)); + if is_proto_method(&spec.python_name().to_string()) { + // If this is a known protocol method e.g. __contains__, then allow this + // symbol even though it's not an uppercase constant. + konst + .attrs + .push(syn::parse_quote!(#[allow(non_upper_case_globals)])); + } } } _ => (), diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 39758a90..eb8611dc 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -24,6 +24,63 @@ pub enum GeneratedPyMethod { SlotTraitImpl(String, TokenStream), } +pub struct PyMethod<'a> { + kind: PyMethodKind, + method_name: String, + spec: FnSpec<'a>, +} + +enum PyMethodKind { + Fn, + Proto(PyMethodProtoKind), +} + +impl PyMethodKind { + fn from_name(name: &str) -> Self { + if let Some(slot_def) = pyproto(name) { + PyMethodKind::Proto(PyMethodProtoKind::Slot(slot_def)) + } else if name == "__call__" { + PyMethodKind::Proto(PyMethodProtoKind::Call) + } else if let Some(slot_fragment_def) = pyproto_fragment(name) { + PyMethodKind::Proto(PyMethodProtoKind::SlotFragment(slot_fragment_def)) + } else { + PyMethodKind::Fn + } + } +} + +enum PyMethodProtoKind { + Slot(&'static SlotDef), + Call, + SlotFragment(&'static SlotFragmentDef), +} + +impl<'a> PyMethod<'a> { + fn parse( + sig: &'a mut syn::Signature, + meth_attrs: &mut Vec, + options: PyFunctionOptions, + ) -> Result { + let spec = FnSpec::parse(sig, meth_attrs, options)?; + + let method_name = spec.python_name.to_string(); + let kind = PyMethodKind::from_name(&method_name); + + Ok(Self { + kind, + method_name, + spec, + }) + } +} + +pub fn is_proto_method(name: &str) -> bool { + match PyMethodKind::from_name(name) { + PyMethodKind::Fn => false, + PyMethodKind::Proto(_) => true, + } +} + pub fn gen_py_method( cls: &syn::Type, sig: &mut syn::Signature, @@ -33,56 +90,55 @@ pub fn gen_py_method( check_generic(sig)?; ensure_not_async_fn(sig)?; ensure_function_options_valid(&options)?; - let spec = FnSpec::parse(sig, &mut *meth_attrs, options)?; + let method = PyMethod::parse(sig, &mut *meth_attrs, options)?; + let spec = &method.spec; - let method_name = spec.python_name.to_string(); - - if let Some(slot_def) = pyproto(&method_name) { - ensure_no_forbidden_protocol_attributes(&spec, &method_name)?; - let slot = slot_def.generate_type_slot(cls, &spec)?; - return Ok(GeneratedPyMethod::Proto(slot)); - } else if method_name == "__call__" { - ensure_no_forbidden_protocol_attributes(&spec, &method_name)?; - return Ok(GeneratedPyMethod::Proto(impl_call_slot(cls, spec)?)); - } - - if let Some(slot_fragment_def) = pyproto_fragment(&method_name) { - ensure_no_forbidden_protocol_attributes(&spec, &method_name)?; - let proto = slot_fragment_def.generate_pyproto_fragment(cls, &spec)?; - return Ok(GeneratedPyMethod::SlotTraitImpl(method_name, proto)); - } - - Ok(match &spec.tp { + Ok(match (method.kind, &spec.tp) { + // Class attributes go before protos so that class attributes can be used to set proto + // method to None. + (_, FnType::ClassAttribute) => { + GeneratedPyMethod::Method(impl_py_class_attribute(cls, spec)) + } + (PyMethodKind::Proto(proto_kind), _) => { + ensure_no_forbidden_protocol_attributes(spec, &method.method_name)?; + match proto_kind { + PyMethodProtoKind::Slot(slot_def) => { + let slot = slot_def.generate_type_slot(cls, spec)?; + GeneratedPyMethod::Proto(slot) + } + PyMethodProtoKind::Call => { + GeneratedPyMethod::Proto(impl_call_slot(cls, method.spec)?) + } + PyMethodProtoKind::SlotFragment(slot_fragment_def) => { + let proto = slot_fragment_def.generate_pyproto_fragment(cls, spec)?; + GeneratedPyMethod::SlotTraitImpl(method.method_name, proto) + } + } + } // ordinary functions (with some specialties) - FnType::Fn(_) => GeneratedPyMethod::Method(impl_py_method_def(cls, &spec, None)?), - FnType::FnClass => GeneratedPyMethod::Method(impl_py_method_def( + (_, FnType::Fn(_)) => GeneratedPyMethod::Method(impl_py_method_def(cls, spec, None)?), + (_, FnType::FnClass) => GeneratedPyMethod::Method(impl_py_method_def( cls, - &spec, + spec, Some(quote!(::pyo3::ffi::METH_CLASS)), )?), - FnType::FnStatic => GeneratedPyMethod::Method(impl_py_method_def( + (_, FnType::FnStatic) => GeneratedPyMethod::Method(impl_py_method_def( cls, - &spec, + spec, Some(quote!(::pyo3::ffi::METH_STATIC)), )?), // special prototypes - FnType::FnNew => GeneratedPyMethod::TraitImpl(impl_py_method_def_new(cls, &spec)?), - FnType::ClassAttribute => GeneratedPyMethod::Method(impl_py_class_attribute(cls, &spec)), - FnType::Getter(self_type) => GeneratedPyMethod::Method(impl_py_getter_def( + (_, FnType::FnNew) => GeneratedPyMethod::TraitImpl(impl_py_method_def_new(cls, spec)?), + + (_, FnType::Getter(self_type)) => GeneratedPyMethod::Method(impl_py_getter_def( cls, - PropertyType::Function { - self_type, - spec: &spec, - }, + PropertyType::Function { self_type, spec }, )?), - FnType::Setter(self_type) => GeneratedPyMethod::Method(impl_py_setter_def( + (_, FnType::Setter(self_type)) => GeneratedPyMethod::Method(impl_py_setter_def( cls, - PropertyType::Function { - self_type, - spec: &spec, - }, + PropertyType::Function { self_type, spec }, )?), - FnType::FnModule => { + (_, FnType::FnModule) => { unreachable!("methods cannot be FnModule") } }) diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index d5324bf6..0b6c30d4 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -1,11 +1,14 @@ use pyo3::exceptions::PyValueError; -use pyo3::types::{PySlice, PyType}; +use pyo3::types::{PyList, PySlice, PyType}; use pyo3::{exceptions::PyAttributeError, prelude::*}; use pyo3::{ffi, py_run, AsPyPointer, PyCell}; use std::{isize, iter}; mod common; +#[pyclass] +struct EmptyClass; + #[pyclass] struct ExampleClass { #[pyo3(get, set)] @@ -560,3 +563,72 @@ assert c.counter.count == 1 .map_err(|e| e.print(py)) .unwrap(); } + +#[pyclass] +struct NotHashable; + +#[pymethods] +impl NotHashable { + #[classattr] + const __hash__: Option = None; +} + +#[test] +fn test_hash_opt_out() { + // By default Python provides a hash implementation, which can be disabled by setting __hash__ + // to None. + Python::with_gil(|py| { + let empty = Py::new(py, EmptyClass).unwrap(); + py_assert!(py, empty, "hash(empty) is not None"); + + let not_hashable = Py::new(py, NotHashable).unwrap(); + py_expect_exception!(py, not_hashable, "hash(not_hashable)", PyTypeError); + }) +} + +/// Class with __iter__ gets default contains from CPython. +#[pyclass] +struct DefaultedContains; + +#[pymethods] +impl DefaultedContains { + fn __iter__(&self, py: Python) -> PyObject { + PyList::new(py, &["a", "b", "c"]) + .as_ref() + .iter() + .unwrap() + .into() + } +} + +#[pyclass] +struct NoContains; + +#[pymethods] +impl NoContains { + fn __iter__(&self, py: Python) -> PyObject { + PyList::new(py, &["a", "b", "c"]) + .as_ref() + .iter() + .unwrap() + .into() + } + + // Equivalent to the opt-out const form in NotHashable above, just more verbose, to confirm this + // also works. + #[classattr] + fn __contains__() -> Option { + None + } +} + +#[test] +fn test_contains_opt_out() { + Python::with_gil(|py| { + let defaulted_contains = Py::new(py, DefaultedContains).unwrap(); + py_assert!(py, defaulted_contains, "'a' in defaulted_contains"); + + let no_contains = Py::new(py, NoContains).unwrap(); + py_expect_exception!(py, no_contains, "'a' in no_contains", PyTypeError); + }) +}