support text_signature on #[new]

PyO3 · Apr 19, 2023 · 8acb224 · 8acb224
1 parent f08098f
commit 8acb224
Show file tree

Hide file tree

Showing 20 changed files with 409 additions and 275 deletions.
diff --git a/guide/src/class.md b/guide/src/class.md
@@ -722,22 +722,20 @@ py_args=(), py_kwargs=None, name=World, num=-1, num_before=44
 
 ## Making class method signatures available to Python
 
-The [`text_signature = "..."`](./function.md#text_signature) option for `#[pyfunction]` also works for classes and methods:
+The [`text_signature = "..."`](./function.md#text_signature) option for `#[pyfunction]` also works for `#[pymethods]`:
 
 ```rust
 # #![allow(dead_code)]
 use pyo3::prelude::*;
 use pyo3::types::PyType;
 
-// it works even if the item is not documented:
-#[pyclass(text_signature = "(c, d, /)")]
+#[pyclass]
 struct MyClass {}
 
 #[pymethods]
 impl MyClass {
-    // the signature for the constructor is attached
-    // to the struct definition instead.
     #[new]
+    #[pyo3(text_signature = "(c, d)")]
     fn new(c: i32, d: &str) -> Self {
         Self {}
     }
@@ -746,8 +744,9 @@ impl MyClass {
     fn my_method(&self, e: i32, f: i32) -> i32 {
         e + f
     }
+    // similarly for classmethod arguments, use $cls
     #[classmethod]
-    #[pyo3(text_signature = "(cls, e, f)")]
+    #[pyo3(text_signature = "($cls, e, f)")]
     fn my_class_method(cls: &PyType, e: i32, f: i32) -> i32 {
         e + f
     }
@@ -773,7 +772,7 @@ impl MyClass {
 #                 .call1((class,))?
 #                 .call_method0("__str__")?
 #                 .extract()?;
-#             assert_eq!(sig, "(c, d, /)");
+#             assert_eq!(sig, "(c, d)");
 #         } else {
 #             let doc: String = class.getattr("__doc__")?.extract()?;
 #             assert_eq!(doc, "");
@@ -802,7 +801,7 @@ impl MyClass {
 #                 .call1((method,))?
 #                 .call_method0("__str__")?
 #                 .extract()?;
-#             assert_eq!(sig, "(cls, e, f)");
+#             assert_eq!(sig, "(e, f)");  // inspect.signature skips the $cls arg
 #         }
 #
 #         {
@@ -822,7 +821,7 @@ impl MyClass {
 # }
 ```
 
-Note that `text_signature` on classes is not compatible with compilation in
+Note that `text_signature` on `#[new]` is not compatible with compilation in
 `abi3` mode until Python 3.10 or greater.
 
 ## #[pyclass] enums
@@ -1018,7 +1017,6 @@ impl pyo3::IntoPy<PyObject> for MyClass {
 }
 
 impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
-    const DOC: &'static str = "Class for demonstration\u{0}";
     const IS_BASETYPE: bool = false;
     const IS_SUBCLASS: bool = false;
     type Layout = PyCell<MyClass>;
@@ -1041,6 +1039,15 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
         static TYPE_OBJECT: LazyTypeObject<MyClass> = LazyTypeObject::new();
         &TYPE_OBJECT
     }
+
+    fn doc(py: Python<'_>) -> pyo3::PyResult<&'static ::std::ffi::CStr> {
+        use pyo3::impl_::pyclass::*;
+        static DOC: pyo3::once_cell::GILOnceCell<::std::borrow::Cow<'static, ::std::ffi::CStr>> = pyo3::once_cell::GILOnceCell::new();
+        DOC.get_or_try_init(py, || {
+            let collector = PyClassImplCollector::<Self>::new();
+            build_pyclass_doc(<MyClass as pyo3::PyTypeInfo>::NAME, "", None.or_else(|| collector.new_text_signature()))
+        }).map(::std::ops::Deref::deref)
+    }
 }
 
 # Python::with_gil(|py| {

diff --git a/guide/src/class/numeric.md b/guide/src/class/numeric.md
@@ -42,7 +42,7 @@ fn wrap(obj: &PyAny) -> Result<i32, PyErr> {
     Ok(val as i32)
 }
 ```
-We also add documentation, via `///` comments and the `#[pyo3(text_signature = "...")]` attribute, both of which are visible to Python users.
+We also add documentation, via `///` comments, which are visible to Python users.
 
 ```rust
 # #![allow(dead_code)]
@@ -57,7 +57,6 @@ fn wrap(obj: &PyAny) -> Result<i32, PyErr> {
 /// Did you ever hear the tragedy of Darth Signed The Overfloweth? I thought not.
 /// It's not a story C would tell you. It's a Rust legend.
 #[pyclass(module = "my_module")]
-#[pyo3(text_signature = "(int)")]
 struct Number(i32);
 
 #[pymethods]
@@ -223,7 +222,6 @@ fn wrap(obj: &PyAny) -> Result<i32, PyErr> {
 /// Did you ever hear the tragedy of Darth Signed The Overfloweth? I thought not.
 /// It's not a story C would tell you. It's a Rust legend.
 #[pyclass(module = "my_module")]
-#[pyo3(text_signature = "(int)")]
 struct Number(i32);
 
 #[pymethods]
@@ -377,7 +375,7 @@ fn my_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
 # assert Number(12345234523452) == Number(1498514748)
 # try:
 #     import inspect
-#     assert inspect.signature(Number).__str__() == '(int)'
+#     assert inspect.signature(Number).__str__() == '(value)'
 # except ValueError:
 #     # Not supported with `abi3` before Python 3.10
 #     pass

diff --git a/guide/src/function/signature.md b/guide/src/function/signature.md
@@ -272,9 +272,7 @@ impl MyClass {
 
 The function signature is exposed to Python via the `__text_signature__` attribute. PyO3 automatically generates this for every `#[pyfunction]` and all `#[pymethods]` directly from the Rust function, taking into account any override done with the `#[pyo3(signature = (...))]` option.
 
-This automatic generation has some limitations, which may be improved in the future:
-- It will not include the value of default arguments, replacing them all with `...`. (`.pyi` type stub files commonly also use `...` for all default arguments in the same way.)
-- Nothing is generated for the `#[new]` method of a `#[pyclass]`.
+This automatic generation can only display the value of default arguments for strings, integers, boolean types, and `None`. Any other default arguments will be displayed as `...`. (`.pyi` type stub files commonly also use `...` for default arguments in the same way.)
 
 In cases where the automatically-generated signature needs adjusting, it can [be overridden](#overriding-the-generated-signature) using the `#[pyo3(text_signature)]` option.)
 

diff --git a/newsfragments/2980.added.md b/newsfragments/2980.added.md
@@ -0,0 +1 @@
+Support `text_signature` option (and automatically generate signature) for `#[new]` in `#[pymethods]`.
diff --git a/newsfragments/2980.changed.md b/newsfragments/2980.changed.md
@@ -0,0 +1 @@
+Deprecate `text_signature` option on `#[pyclass]`.
diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs
@@ -2,6 +2,7 @@ use proc_macro2::{Span, TokenStream};
 use quote::{quote_spanned, ToTokens};
 
 pub enum Deprecation {
+    PyClassTextSignature,
     PyFunctionArguments,
     PyMethodArgsAttribute,
     RequiredArgumentAfterOption,
@@ -10,6 +11,7 @@ pub enum Deprecation {
 impl Deprecation {
     fn ident(&self, span: Span) -> syn::Ident {
         let string = match self {
+            Deprecation::PyClassTextSignature => "PYCLASS_TEXT_SIGNATURE",
             Deprecation::PyFunctionArguments => "PYFUNCTION_ARGUMENTS",
             Deprecation::PyMethodArgsAttribute => "PYMETHODS_ARGS_ATTRIBUTE",
             Deprecation::RequiredArgumentAfterOption => "REQUIRED_ARGUMENT_AFTER_OPTION",

diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs
@@ -1,6 +1,6 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 
-use crate::attributes::TextSignatureAttribute;
+use crate::attributes::{TextSignatureAttribute, TextSignatureAttributeValue};
 use crate::deprecations::{Deprecation, Deprecations};
 use crate::params::impl_arg_params;
 use crate::pyfunction::{DeprecatedArgs, FunctionSignature, PyFunctionArgPyO3Attributes};
@@ -593,6 +593,33 @@ impl<'a> FnSpec<'a> {
             CallingConvention::TpNew => unreachable!("tp_new cannot get a methoddef"),
         }
     }
+
+    /// Forwards to [utils::get_doc] with the text signature of this spec.
+    pub fn get_doc(&self, attrs: &[syn::Attribute]) -> PythonDoc {
+        let text_signature = self
+            .text_signature_call_signature()
+            .map(|sig| format!("{}{}", self.python_name, sig));
+        utils::get_doc(attrs, text_signature)
+    }
+
+    /// Creates the parenthesised arguments list for `__text_signature__` snippet based on this spec's signature
+    /// and/or attributes. Prepend the callable name to make a complete `__text_signature__`.
+    pub fn text_signature_call_signature(&self) -> Option<String> {
+        let self_argument = match &self.tp {
+            // Getters / Setters / ClassAttribute are not callables on the Python side
+            FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => return None,
+            FnType::Fn(_) => Some("self"),
+            FnType::FnModule => Some("module"),
+            FnType::FnClass => Some("cls"),
+            FnType::FnStatic | FnType::FnNew => None,
+        };
+
+        match self.text_signature.as_ref().map(|attr| &attr.value) {
+            Some(TextSignatureAttributeValue::Str(s)) => Some(s.value()),
+            None => Some(self.signature.text_signature(self_argument)),
+            Some(TextSignatureAttributeValue::Disabled(_)) => None,
+        }
+    }
 }
 
 #[derive(Debug)]
@@ -755,11 +782,6 @@ fn ensure_signatures_on_valid_method(
     }
     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`")
             }

diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -5,11 +5,11 @@ use std::borrow::Cow;
 use crate::attributes::{
     self, kw, take_pyo3_options, CrateAttribute, ExtendsAttribute, FreelistAttribute,
     ModuleAttribute, NameAttribute, NameLitStr, TextSignatureAttribute,
+    TextSignatureAttributeValue,
 };
-use crate::deprecations::Deprecations;
+use crate::deprecations::{Deprecation, Deprecations};
 use crate::konst::{ConstAttributes, ConstSpec};
 use crate::method::FnSpec;
-use crate::pyfunction::text_signature_or_none;
 use crate::pyimpl::{gen_py_const, PyClassMethodsType};
 use crate::pymethod::{
     impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType,
@@ -177,7 +177,11 @@ impl PyClassPyO3Options {
             PyClassPyO3Option::Sequence(sequence) => set_option!(sequence),
             PyClassPyO3Option::SetAll(set_all) => set_option!(set_all),
             PyClassPyO3Option::Subclass(subclass) => set_option!(subclass),
-            PyClassPyO3Option::TextSignature(text_signature) => set_option!(text_signature),
+            PyClassPyO3Option::TextSignature(text_signature) => {
+                self.deprecations
+                    .push(Deprecation::PyClassTextSignature, text_signature.span());
+                set_option!(text_signature)
+            }
             PyClassPyO3Option::Unsendable(unsendable) => set_option!(unsendable),
             PyClassPyO3Option::Weakref(weakref) => set_option!(weakref),
         }
@@ -191,11 +195,7 @@ pub fn build_py_class(
     methods_type: PyClassMethodsType,
 ) -> syn::Result<TokenStream> {
     args.options.take_pyo3_options(&mut class.attrs)?;
-    let text_signature_string = text_signature_or_none(args.options.text_signature.as_ref());
-    let doc = utils::get_doc(
-        &class.attrs,
-        text_signature_string.map(|s| (get_class_python_name(&class.ident, &args), s)),
-    );
+    let doc = utils::get_doc(&class.attrs, None);
     let krate = get_pyo3_crate(&args.options.krate);
 
     if let Some(lt) = class.generics.lifetimes().next() {
@@ -452,12 +452,7 @@ pub fn build_py_enum(
         bail_spanned!(enum_.brace_token.span => "#[pyclass] can't be used on enums without any variants");
     }
 
-    let text_signature_string = text_signature_or_none(args.options.text_signature.as_ref());
-
-    let doc = utils::get_doc(
-        &enum_.attrs,
-        text_signature_string.map(|s| (get_class_python_name(&enum_.ident, &args), s)),
-    );
+    let doc = utils::get_doc(&enum_.attrs, None);
     let enum_ = PyClassEnum::new(enum_)?;
     impl_enum(enum_, &args, doc, method_type)
 }
@@ -509,16 +504,6 @@ fn impl_enum(
     methods_type: PyClassMethodsType,
 ) -> Result<TokenStream> {
     let krate = get_pyo3_crate(&args.options.krate);
-    impl_enum_class(enum_, args, doc, methods_type, krate)
-}
-
-fn impl_enum_class(
-    enum_: PyClassEnum<'_>,
-    args: &PyClassArgs,
-    doc: PythonDoc,
-    methods_type: PyClassMethodsType,
-    krate: syn::Path,
-) -> Result<TokenStream> {
     let cls = enum_.ident;
     let ty: syn::Type = syn::parse_quote!(#cls);
     let variants = enum_.variants;
@@ -889,6 +874,18 @@ impl<'a> PyClassImplsBuilder<'a> {
     fn impl_pyclassimpl(&self) -> Result<TokenStream> {
         let cls = self.cls;
         let doc = self.doc.as_ref().map_or(quote! {"\0"}, |doc| quote! {#doc});
+        let deprecated_text_signature = match self
+            .attr
+            .options
+            .text_signature
+            .as_ref()
+            .map(|attr| &attr.value)
+        {
+            Some(TextSignatureAttributeValue::Str(s)) => quote!(::std::option::Option::Some(#s)),
+            Some(TextSignatureAttributeValue::Disabled(_)) | None => {
+                quote!(::std::option::Option::None)
+            }
+        };
         let is_basetype = self.attr.options.subclass.is_some();
         let base = self
             .attr
@@ -1009,7 +1006,6 @@ impl<'a> PyClassImplsBuilder<'a> {
 
         Ok(quote! {
             impl _pyo3::impl_::pyclass::PyClassImpl for #cls {
-                const DOC: &'static str = #doc;
                 const IS_BASETYPE: bool = #is_basetype;
                 const IS_SUBCLASS: bool = #is_subclass;
                 const IS_MAPPING: bool = #is_mapping;
@@ -1035,6 +1031,15 @@ impl<'a> PyClassImplsBuilder<'a> {
                     PyClassItemsIter::new(&INTRINSIC_ITEMS, #pymethods_items)
                 }
 
+                fn doc(py: _pyo3::Python<'_>) -> _pyo3::PyResult<&'static ::std::ffi::CStr>  {
+                    use _pyo3::impl_::pyclass::*;
+                    static DOC: _pyo3::once_cell::GILOnceCell<::std::borrow::Cow<'static, ::std::ffi::CStr>> = _pyo3::once_cell::GILOnceCell::new();
+                    DOC.get_or_try_init(py, || {
+                        let collector = PyClassImplCollector::<Self>::new();
+                        build_pyclass_doc(<#cls as _pyo3::PyTypeInfo>::NAME, #doc, #deprecated_text_signature.or_else(|| collector.new_text_signature()))
+                    }).map(::std::ops::Deref::deref)
+                }
+
                 #dict_offset
 
                 #weaklist_offset

diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs
@@ -1,16 +1,14 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 
-use std::borrow::Cow;
-
 use crate::{
     attributes::{
         self, get_pyo3_options, take_attributes, take_pyo3_options, CrateAttribute,
-        FromPyWithAttribute, NameAttribute, TextSignatureAttribute, TextSignatureAttributeValue,
+        FromPyWithAttribute, NameAttribute, TextSignatureAttribute,
     },
     deprecations::{Deprecation, Deprecations},
-    method::{self, CallingConvention, FnArg, FnType},
+    method::{self, CallingConvention, FnArg},
     pymethod::check_generic,
-    utils::{self, ensure_not_async_fn, get_pyo3_crate},
+    utils::{ensure_not_async_fn, get_pyo3_crate},
 };
 use proc_macro2::TokenStream;
 use quote::{format_ident, quote};
@@ -409,15 +407,6 @@ pub fn impl_wrap_pyfunction(
 
     let ty = method::get_return_info(&func.sig.output);
 
-    let text_signature_string = text_signature_or_auto(text_signature.as_ref(), &signature, &tp);
-
-    let doc = utils::get_doc(
-        &func.attrs,
-        text_signature_string.map(|s| (Cow::Borrowed(&python_name), s)),
-    );
-
-    let krate = get_pyo3_crate(&krate);
-
     let spec = method::FnSpec {
         tp,
         name: &func.sig.ident,
@@ -430,12 +419,14 @@ pub fn impl_wrap_pyfunction(
         unsafety: func.sig.unsafety,
     };
 
+    let krate = get_pyo3_crate(&krate);
+
     let vis = &func.vis;
     let name = &func.sig.ident;
 
     let wrapper_ident = format_ident!("__pyfunction_{}", spec.name);
     let wrapper = spec.get_wrapper_function(&wrapper_ident, None)?;
-    let methoddef = spec.get_methoddef(wrapper_ident, &doc);
+    let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&func.attrs));
 
     let wrapped_pyfunction = quote! {
 
@@ -480,26 +471,3 @@ fn type_is_pymodule(ty: &syn::Type) -> bool {
     }
     false
 }
-
-/// Helper to get a text signature string, or None if unset or disabled
-pub(crate) fn text_signature_or_none(
-    text_signature: Option<&TextSignatureAttribute>,
-) -> Option<String> {
-    match text_signature.map(|attr| &attr.value) {
-        Some(TextSignatureAttributeValue::Str(s)) => Some(s.value()),
-        Some(TextSignatureAttributeValue::Disabled(_)) | None => None,
-    }
-}
-
-/// Helper to get a text signature string, using automatic generation if unset, or None if disabled
-pub(crate) fn text_signature_or_auto(
-    text_signature: Option<&TextSignatureAttribute>,
-    signature: &FunctionSignature<'_>,
-    fn_type: &FnType,
-) -> Option<String> {
-    match text_signature.map(|attr| &attr.value) {
-        Some(TextSignatureAttributeValue::Str(s)) => Some(s.value()),
-        None => Some(signature.text_signature(fn_type)),
-        Some(TextSignatureAttributeValue::Disabled(_)) => None,
-    }
-}