Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate undocumented #[__new__] form of #[new] #3505

Merged
merged 1 commit into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/3505.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate undocumented `#[__new__]` form of `#[new]` attribute.
2 changes: 2 additions & 0 deletions pyo3-macros-backend/src/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ use quote::{quote_spanned, ToTokens};

pub enum Deprecation {
PyClassTextSignature,
PyMethodsNewDeprecatedForm,
}

impl Deprecation {
fn ident(&self, span: Span) -> syn::Ident {
let string = match self {
Deprecation::PyClassTextSignature => "PYCLASS_TEXT_SIGNATURE",
Deprecation::PyMethodsNewDeprecatedForm => "PYMETHODS_NEW_DEPRECATED_FORM",
};
syn::Ident::new(string, span)
}
Expand Down
26 changes: 19 additions & 7 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt::Display;

use crate::attributes::{TextSignatureAttribute, TextSignatureAttributeValue};
use crate::deprecations::{Deprecation, Deprecations};
use crate::params::impl_arg_params;
use crate::pyfunction::{FunctionSignature, PyFunctionArgPyO3Attributes};
use crate::pyfunction::{PyFunctionOptions, SignatureAttribute};
Expand Down Expand Up @@ -228,6 +229,7 @@ pub struct FnSpec<'a> {
pub convention: CallingConvention,
pub text_signature: Option<TextSignatureAttribute>,
pub unsafety: Option<syn::Token![unsafe]>,
pub deprecations: Deprecations,
}

pub fn get_return_info(output: &syn::ReturnType) -> syn::Type {
Expand Down Expand Up @@ -275,8 +277,9 @@ impl<'a> FnSpec<'a> {
} = options;

let mut python_name = name.map(|name| name.value.0);
let mut deprecations = Deprecations::new();

let fn_type = Self::parse_fn_type(sig, meth_attrs, &mut python_name)?;
let fn_type = Self::parse_fn_type(sig, meth_attrs, &mut python_name, &mut deprecations)?;
ensure_signatures_on_valid_method(&fn_type, signature.as_ref(), text_signature.as_ref())?;

let name = &sig.ident;
Expand Down Expand Up @@ -315,6 +318,7 @@ impl<'a> FnSpec<'a> {
output: ty,
text_signature,
unsafety: sig.unsafety,
deprecations,
})
}

Expand All @@ -326,8 +330,9 @@ impl<'a> FnSpec<'a> {
sig: &syn::Signature,
meth_attrs: &mut Vec<syn::Attribute>,
python_name: &mut Option<syn::Ident>,
deprecations: &mut Deprecations,
) -> Result<FnType> {
let mut method_attributes = parse_method_attributes(meth_attrs)?;
let mut method_attributes = parse_method_attributes(meth_attrs, deprecations)?;

let name = &sig.ident;
let parse_receiver = |msg: &'static str| {
Expand Down Expand Up @@ -648,7 +653,10 @@ impl MethodTypeAttribute {
/// If the attribute does not match one of the attribute names, returns `Ok(None)`.
///
/// Otherwise will either return a parse error or the attribute.
fn parse_if_matching_attribute(attr: &syn::Attribute) -> Result<Option<Self>> {
fn parse_if_matching_attribute(
attr: &syn::Attribute,
deprecations: &mut Deprecations,
) -> Result<Option<Self>> {
fn ensure_no_arguments(meta: &syn::Meta, ident: &str) -> syn::Result<()> {
match meta {
syn::Meta::Path(_) => Ok(()),
Expand Down Expand Up @@ -693,9 +701,10 @@ impl MethodTypeAttribute {
ensure_no_arguments(meta, "new")?;
Ok(Some(MethodTypeAttribute::New(path.span())))
} else if path.is_ident("__new__") {
// TODO deprecate this form?
let span = path.span();
deprecations.push(Deprecation::PyMethodsNewDeprecatedForm, span);
ensure_no_arguments(meta, "__new__")?;
Ok(Some(MethodTypeAttribute::New(path.span())))
Ok(Some(MethodTypeAttribute::New(span)))
} else if path.is_ident("classmethod") {
ensure_no_arguments(meta, "classmethod")?;
Ok(Some(MethodTypeAttribute::ClassMethod(path.span())))
Expand Down Expand Up @@ -730,12 +739,15 @@ impl Display for MethodTypeAttribute {
}
}

fn parse_method_attributes(attrs: &mut Vec<syn::Attribute>) -> Result<Vec<MethodTypeAttribute>> {
fn parse_method_attributes(
attrs: &mut Vec<syn::Attribute>,
deprecations: &mut Deprecations,
) -> Result<Vec<MethodTypeAttribute>> {
let mut new_attrs = Vec::new();
let mut found_attrs = Vec::new();

for attr in attrs.drain(..) {
match MethodTypeAttribute::parse_if_matching_attribute(&attr)? {
match MethodTypeAttribute::parse_if_matching_attribute(&attr, deprecations)? {
Some(attr) => found_attrs.push(attr),
None => new_attrs.push(attr),
}
Expand Down
2 changes: 2 additions & 0 deletions pyo3-macros-backend/src/pyfunction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{
self, get_pyo3_options, take_attributes, take_pyo3_options, CrateAttribute,
FromPyWithAttribute, NameAttribute, TextSignatureAttribute,
},
deprecations::Deprecations,
method::{self, CallingConvention, FnArg},
pymethod::check_generic,
utils::{ensure_not_async_fn, get_pyo3_crate},
Expand Down Expand Up @@ -230,6 +231,7 @@ pub fn impl_wrap_pyfunction(
output: ty,
text_signature,
unsafety: func.sig.unsafety,
deprecations: Deprecations::new(),
};

let krate = get_pyo3_crate(&krate);
Expand Down
3 changes: 3 additions & 0 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,7 @@ fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<MethodAn
|| quote!(::std::option::Option::None),
|text_signature| quote!(::std::option::Option::Some(#text_signature)),
);
let deprecations = &spec.deprecations;
let slot_def = quote! {
_pyo3::ffi::PyType_Slot {
slot: _pyo3::ffi::Py_tp_new,
Expand All @@ -345,6 +346,8 @@ fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result<MethodAn
kwargs: *mut _pyo3::ffi::PyObject,
) -> *mut _pyo3::ffi::PyObject
{
#deprecations

use _pyo3::impl_::pyclass::*;
impl PyClassNewTextSignature<#cls> for PyClassImplCollector<#cls> {
#[inline]
Expand Down
3 changes: 3 additions & 0 deletions src/impl_/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@
note = "put `text_signature` on `#[new]` instead of `#[pyclass]`"
)]
pub const PYCLASS_TEXT_SIGNATURE: () = ();

#[deprecated(since = "0.20.0", note = "use `#[new]` instead of `#[__new__]`")]
pub const PYMETHODS_NEW_DEPRECATED_FORM: () = ();
8 changes: 8 additions & 0 deletions tests/ui/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,12 @@ use pyo3::prelude::*;
#[pyo3(text_signature = "()")]
struct MyClass;

#[pymethods]
impl MyClass {
#[__new__]
fn new() -> Self {
Self
}
}

fn main() {}
18 changes: 12 additions & 6 deletions tests/ui/deprecations.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
error: use of deprecated constant `pyo3::impl_::deprecations::PYMETHODS_NEW_DEPRECATED_FORM`: use `#[new]` instead of `#[__new__]`
--> tests/ui/deprecations.rs:11:7
|
11 | #[__new__]
| ^^^^^^^
|
note: the lint level is defined here
--> tests/ui/deprecations.rs:1:9
|
1 | #![deny(deprecated)]
| ^^^^^^^^^^

error: use of deprecated constant `pyo3::impl_::deprecations::PYCLASS_TEXT_SIGNATURE`: put `text_signature` on `#[new]` instead of `#[pyclass]`
--> tests/ui/deprecations.rs:6:8
|
6 | #[pyo3(text_signature = "()")]
| ^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> tests/ui/deprecations.rs:1:9
|
1 | #![deny(deprecated)]
| ^^^^^^^^^^
Loading