Merge pull request #3505 from davidhewitt/deprecate_dunder_new

deprecate undocumented `#[__new__]` form of `#[new]`
PyO3 · Oct 10, 2023 · 76bf521 · 76bf521
2 parents c0b5004 + 6c90325
commit 76bf521
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 13 deletions.
diff --git a/newsfragments/3505.changed.md b/newsfragments/3505.changed.md
@@ -0,0 +1 @@
+Deprecate undocumented `#[__new__]` form of `#[new]` attribute.
diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs
@@ -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)
     }

diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs
@@ -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};
@@ -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 {
@@ -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;
@@ -315,6 +318,7 @@ impl<'a> FnSpec<'a> {
             output: ty,
             text_signature,
             unsafety: sig.unsafety,
+            deprecations,
         })
     }
 
@@ -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| {
@@ -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(()),
@@ -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())))
@@ -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),
         }

diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs
@@ -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},
@@ -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);

diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs
@@ -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,
@@ -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]

diff --git a/src/impl_/deprecations.rs b/src/impl_/deprecations.rs
@@ -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: () = ();
diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs
@@ -6,4 +6,12 @@ use pyo3::prelude::*;
 #[pyo3(text_signature = "()")]
 struct MyClass;
 
+#[pymethods]
+impl MyClass {
+    #[__new__]
+    fn new() -> Self {
+        Self
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/deprecations.stderr b/tests/ui/deprecations.stderr
@@ -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)]
-  |         ^^^^^^^^^^