From 3c1a975ec0ef3224696cae811191564e00dc6d57 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Sun, 24 Nov 2019 06:00:21 -0800 Subject: [PATCH 1/8] initial untested implementation --- pyo3-derive-backend/src/method.rs | 4 +- pyo3-derive-backend/src/module.rs | 11 ++- pyo3-derive-backend/src/pyclass.rs | 18 +++-- pyo3-derive-backend/src/pymethod.rs | 34 ++++++++- pyo3-derive-backend/src/utils.rs | 111 +++++++++++++++++++++++++++- pyo3cls/src/lib.rs | 11 ++- 6 files changed, 172 insertions(+), 17 deletions(-) diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs index b195978044b..75e78d460ad 100644 --- a/pyo3-derive-backend/src/method.rs +++ b/pyo3-derive-backend/src/method.rs @@ -47,10 +47,10 @@ pub fn get_return_info(output: &syn::ReturnType) -> syn::Type { impl<'a> FnSpec<'a> { /// Parser function signature and function attributes - pub fn parse( + pub fn parse<'b>( name: &'a syn::Ident, sig: &'a syn::Signature, - meth_attrs: &'a mut Vec, + meth_attrs: &'b mut Vec, ) -> syn::Result> { let (mut fn_type, fn_attrs) = parse_attributes(meth_attrs)?; diff --git a/pyo3-derive-backend/src/module.rs b/pyo3-derive-backend/src/module.rs index 181cde1c479..3ee1433b5eb 100644 --- a/pyo3-derive-backend/src/module.rs +++ b/pyo3-derive-backend/src/module.rs @@ -133,7 +133,7 @@ fn function_wrapper_ident(name: &Ident) -> Ident { /// Generates python wrapper over a function that allows adding it to a python module as a python /// function pub fn add_fn_to_module( - func: &syn::ItemFn, + func: &mut syn::ItemFn, python_name: &Ident, pyfn_attrs: Vec, ) -> TokenStream { @@ -157,7 +157,14 @@ pub fn add_fn_to_module( let function_wrapper_ident = function_wrapper_ident(&func.sig.ident); let wrapper = function_c_wrapper(&func.sig.ident, &spec); - let doc = utils::get_doc(&func.attrs, true); + let text_signature = match utils::parse_text_signature_attrs(&mut func.attrs, python_name) { + Ok(text_signature) => text_signature, + Err(err) => return err.to_compile_error(), + }; + let doc = match utils::get_doc(&func.attrs, text_signature, true) { + Ok(doc) => doc, + Err(err) => return err.to_compile_error(), + }; let tokens = quote! { fn #function_wrapper_ident(py: pyo3::Python) -> pyo3::PyObject { diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 1f902a95915..311f5c7c593 100644 --- a/pyo3-derive-backend/src/pyclass.rs +++ b/pyo3-derive-backend/src/pyclass.rs @@ -157,7 +157,11 @@ impl PyClassArgs { } pub fn build_py_class(class: &mut syn::ItemStruct, attr: &PyClassArgs) -> syn::Result { - let doc = utils::get_doc(&class.attrs, true); + let text_signature = utils::parse_text_signature_attrs( + &mut class.attrs, + &get_class_python_name(&class.ident, attr), + )?; + let doc = utils::get_doc(&class.attrs, text_signature, true)?; let mut descriptors = Vec::new(); check_generics(class)?; @@ -245,16 +249,20 @@ fn impl_inventory(cls: &syn::Ident) -> TokenStream { } } +fn get_class_python_name(cls: &syn::Ident, attr: &PyClassArgs) -> TokenStream { + match &attr.name { + Some(name) => quote! { #name }, + None => quote! { #cls }, + } +} + fn impl_class( cls: &syn::Ident, attr: &PyClassArgs, doc: syn::Lit, descriptors: Vec<(syn::Field, Vec)>, ) -> TokenStream { - let cls_name = match &attr.name { - Some(name) => quote! { #name }.to_string(), - None => cls.to_string(), - }; + let cls_name = get_class_python_name(cls, attr); let extra = { if let Some(freelist) = &attr.freelist { diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index 9a0d3eff814..f8c5cb1adb7 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -12,8 +12,38 @@ pub fn gen_py_method( ) -> syn::Result { check_generic(name, sig)?; - let doc = utils::get_doc(&meth_attrs, true); - let spec = FnSpec::parse(name, sig, meth_attrs)?; + let spec = FnSpec::parse(name, sig, &mut *meth_attrs)?; + + let text_signature = match &spec.tp { + FnType::Fn | FnType::PySelf(_) | FnType::FnClass | FnType::FnStatic => { + utils::parse_text_signature_attrs(&mut *meth_attrs, name)? + } + FnType::FnNew => utils::parse_text_signature_attrs( + &mut *meth_attrs, + &syn::Ident::new("__new__", name.span()), + )?, + FnType::FnCall => utils::parse_text_signature_attrs( + &mut *meth_attrs, + &syn::Ident::new("__call__", name.span()), + )?, + FnType::Getter(get_set_name) | FnType::Setter(get_set_name) => { + // try to parse anyway to give better error messages + let get_set_name = match get_set_name { + None => name.clone(), + Some(get_set_name) => syn::Ident::new(get_set_name, name.span()), + }; + if let Some(type_signature) = + utils::parse_text_signature_attrs(&mut *meth_attrs, &get_set_name)? + { + return Err(syn::Error::new_spanned( + type_signature, + "type_signature not allowed on getters/setters", + )); + } + None + } + }; + let doc = utils::get_doc(&meth_attrs, text_signature, true)?; Ok(match spec.tp { FnType::Fn => impl_py_method_def(name, doc, &spec, &impl_wrap(cls, name, &spec, true)), diff --git a/pyo3-derive-backend/src/utils.rs b/pyo3-derive-backend/src/utils.rs index 31de7e9200a..db8624d93ac 100644 --- a/pyo3-derive-backend/src/utils.rs +++ b/pyo3-derive-backend/src/utils.rs @@ -2,6 +2,7 @@ use proc_macro2::Span; use proc_macro2::TokenStream; +use std::fmt::Display; pub fn print_err(msg: String, t: TokenStream) { println!("Error: {} in '{}'", msg, t.to_string()); @@ -20,9 +21,108 @@ pub fn if_type_is_python(ty: &syn::Type) -> bool { } } +pub fn is_text_signature_attr(attr: &syn::Attribute) -> bool { + attr.path.is_ident("text_signature") +} + +fn parse_text_signature_attr( + attr: &syn::Attribute, + python_name: &T, + text_signature: &mut Option, +) -> syn::Result> { + if !is_text_signature_attr(attr) { + return Ok(None); + } + if text_signature.is_some() { + return Err(syn::Error::new_spanned( + attr, + "text_signature attribute already specified previously", + )); + } + let value: String; + match attr.parse_meta()? { + syn::Meta::NameValue(syn::MetaNameValue { + lit: syn::Lit::Str(lit), + .. + }) => { + value = lit.value(); + *text_signature = Some(lit); + } + meta => { + return Err(syn::Error::new_spanned( + meta, + "text_signature must be of the form #[text_signature = \"\"]", + )); + } + }; + let python_name_str = python_name.to_string(); + let python_name_str = python_name_str + .rsplit('.') + .next() + .map(str::trim) + .filter(|v| !v.is_empty()) + .ok_or_else(|| { + syn::Error::new_spanned( + &python_name, + format!("failed to parse python name: {}", python_name), + ) + })?; + if !value.starts_with(&python_name_str) || !value[python_name_str.len()..].starts_with('(') { + return Err(syn::Error::new_spanned( + text_signature, + format!("text_signature must start with \"{}(\"", python_name_str), + )); + } + if !value.ends_with(')') { + return Err(syn::Error::new_spanned( + text_signature, + "text_signature must end with \")\"", + )); + } + Ok(Some(())) +} + +pub fn parse_text_signature_attrs( + attrs: &mut Vec, + python_name: &T, +) -> syn::Result> { + let mut parse_error: Option = None; + let mut text_signature = None; + attrs.retain(|attr| { + match parse_text_signature_attr(attr, python_name, &mut text_signature) { + Ok(None) => return true, + Ok(Some(_)) => {} + Err(err) => { + if let Some(parse_error) = &mut parse_error { + parse_error.combine(err); + } else { + parse_error = Some(err); + } + } + } + false + }); + if let Some(parse_error) = parse_error { + return Err(parse_error); + } + Ok(text_signature) +} + // FIXME(althonos): not sure the docstring formatting is on par here. -pub fn get_doc(attrs: &[syn::Attribute], null_terminated: bool) -> syn::Lit { +pub fn get_doc( + attrs: &[syn::Attribute], + text_signature: Option, + null_terminated: bool, +) -> syn::Result { let mut doc = Vec::new(); + let mut needs_terminating_newline = false; + + if let Some(text_signature) = text_signature { + doc.push(text_signature.value()); + doc.push("--".to_string()); + doc.push(String::new()); + needs_terminating_newline = true; + } // TODO(althonos): set span on produced doc str literal // let mut span = None; @@ -38,17 +138,22 @@ pub fn get_doc(attrs: &[syn::Attribute], null_terminated: bool) -> syn::Lit { } else { d }); + needs_terminating_newline = false; } else { - panic!("Invalid doc comment"); + return Err(syn::Error::new_spanned(metanv, "Invalid doc comment")); } } } } + if needs_terminating_newline { + doc.push(String::new()); + } + let mut docstr = doc.join("\n"); if null_terminated { docstr.push('\0'); } - syn::Lit::Str(syn::LitStr::new(&docstr, Span::call_site())) + Ok(syn::Lit::Str(syn::LitStr::new(&docstr, Span::call_site()))) } diff --git a/pyo3cls/src/lib.rs b/pyo3cls/src/lib.rs index bb0b6686ac9..e85a645adc4 100644 --- a/pyo3cls/src/lib.rs +++ b/pyo3cls/src/lib.rs @@ -27,7 +27,12 @@ pub fn pymodule(attr: TokenStream, input: TokenStream) -> TokenStream { process_functions_in_module(&mut ast); - let expanded = py_init(&ast.sig.ident, &modname, get_doc(&ast.attrs, false)); + let doc = match get_doc(&ast.attrs, None, false) { + Ok(doc) => doc, + Err(err) => return err.to_compile_error().into(), + }; + + let expanded = py_init(&ast.sig.ident, &modname, doc); quote!( #ast @@ -75,11 +80,11 @@ pub fn pymethods(_: TokenStream, input: TokenStream) -> TokenStream { #[proc_macro_attribute] pub fn pyfunction(attr: TokenStream, input: TokenStream) -> TokenStream { - let ast = parse_macro_input!(input as syn::ItemFn); + let mut ast = parse_macro_input!(input as syn::ItemFn); let args = parse_macro_input!(attr as PyFunctionAttr); let python_name = syn::Ident::new(&ast.sig.ident.unraw().to_string(), Span::call_site()); - let expanded = add_fn_to_module(&ast, &python_name, args.arguments); + let expanded = add_fn_to_module(&mut ast, &python_name, args.arguments); quote!( #ast From 2915f50fc4647367ce483007953ded399bdae7fb Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Wed, 27 Nov 2019 14:31:16 -0800 Subject: [PATCH 2/8] fix compile error --- pyo3-derive-backend/src/pyclass.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyo3-derive-backend/src/pyclass.rs b/pyo3-derive-backend/src/pyclass.rs index 311f5c7c593..b4a8d6fe346 100644 --- a/pyo3-derive-backend/src/pyclass.rs +++ b/pyo3-derive-backend/src/pyclass.rs @@ -262,7 +262,7 @@ fn impl_class( doc: syn::Lit, descriptors: Vec<(syn::Field, Vec)>, ) -> TokenStream { - let cls_name = get_class_python_name(cls, attr); + let cls_name = get_class_python_name(cls, attr).to_string(); let extra = { if let Some(freelist) = &attr.freelist { From af8c0d2531d8b5cc0b98fb882d0aed13477c2079 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Fri, 29 Nov 2019 12:19:56 -0800 Subject: [PATCH 3/8] switch text_signature to arguments only & add tests --- pyo3-derive-backend/src/pymethod.rs | 34 +++-- pyo3-derive-backend/src/utils.rs | 60 ++++----- tests/test_text_signature.rs | 186 ++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 37 deletions(-) create mode 100644 tests/test_text_signature.rs diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index f8c5cb1adb7..b7ade9c3b47 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -18,14 +18,32 @@ pub fn gen_py_method( FnType::Fn | FnType::PySelf(_) | FnType::FnClass | FnType::FnStatic => { utils::parse_text_signature_attrs(&mut *meth_attrs, name)? } - FnType::FnNew => utils::parse_text_signature_attrs( - &mut *meth_attrs, - &syn::Ident::new("__new__", name.span()), - )?, - FnType::FnCall => utils::parse_text_signature_attrs( - &mut *meth_attrs, - &syn::Ident::new("__call__", name.span()), - )?, + FnType::FnNew => { + // try to parse anyway to give better error messages + if let Some(type_signature) = utils::parse_text_signature_attrs( + &mut *meth_attrs, + &syn::Ident::new("__new__", name.span()), + )? { + return Err(syn::Error::new_spanned( + type_signature, + "type_signature not allowed on __new__, put it on the struct definition instead", + )); + } + None + } + FnType::FnCall => { + // try to parse anyway to give better error messages + if let Some(type_signature) = utils::parse_text_signature_attrs( + &mut *meth_attrs, + &syn::Ident::new("__call__", name.span()), + )? { + return Err(syn::Error::new_spanned( + type_signature, + "type_signature not allowed on __call__, put it on the struct definition instead", + )); + } + None + } FnType::Getter(get_set_name) | FnType::Setter(get_set_name) => { // try to parse anyway to give better error messages let get_set_name = match get_set_name { diff --git a/pyo3-derive-backend/src/utils.rs b/pyo3-derive-backend/src/utils.rs index db8624d93ac..cc90aa5fb23 100644 --- a/pyo3-derive-backend/src/utils.rs +++ b/pyo3-derive-backend/src/utils.rs @@ -28,25 +28,51 @@ pub fn is_text_signature_attr(attr: &syn::Attribute) -> bool { fn parse_text_signature_attr( attr: &syn::Attribute, python_name: &T, - text_signature: &mut Option, + text_signature_line: &mut Option, ) -> syn::Result> { if !is_text_signature_attr(attr) { return Ok(None); } - if text_signature.is_some() { + if text_signature_line.is_some() { return Err(syn::Error::new_spanned( attr, "text_signature attribute already specified previously", )); } - let value: String; + let python_name_str = python_name.to_string(); + let python_name_str = python_name_str + .rsplit('.') + .next() + .map(str::trim) + .filter(|v| !v.is_empty()) + .ok_or_else(|| { + syn::Error::new_spanned( + &python_name, + format!("failed to parse python name: {}", python_name), + ) + })?; match attr.parse_meta()? { syn::Meta::NameValue(syn::MetaNameValue { lit: syn::Lit::Str(lit), .. }) => { - value = lit.value(); - *text_signature = Some(lit); + let value = lit.value(); + if !value.starts_with('(') { + return Err(syn::Error::new_spanned( + lit, + "text_signature must start with \"(\"", + )); + } + if !value.ends_with(')') { + return Err(syn::Error::new_spanned( + lit, + "text_signature must end with \")\"", + )); + } + *text_signature_line = Some(syn::LitStr::new( + &(python_name_str.to_owned() + &value), + lit.span(), + )); } meta => { return Err(syn::Error::new_spanned( @@ -55,30 +81,6 @@ fn parse_text_signature_attr( )); } }; - let python_name_str = python_name.to_string(); - let python_name_str = python_name_str - .rsplit('.') - .next() - .map(str::trim) - .filter(|v| !v.is_empty()) - .ok_or_else(|| { - syn::Error::new_spanned( - &python_name, - format!("failed to parse python name: {}", python_name), - ) - })?; - if !value.starts_with(&python_name_str) || !value[python_name_str.len()..].starts_with('(') { - return Err(syn::Error::new_spanned( - text_signature, - format!("text_signature must start with \"{}(\"", python_name_str), - )); - } - if !value.ends_with(')') { - return Err(syn::Error::new_spanned( - text_signature, - "text_signature must end with \")\"", - )); - } Ok(Some(())) } diff --git a/tests/test_text_signature.rs b/tests/test_text_signature.rs new file mode 100644 index 00000000000..72df1fdf111 --- /dev/null +++ b/tests/test_text_signature.rs @@ -0,0 +1,186 @@ +use pyo3::prelude::*; +use pyo3::{types::PyType, wrap_pyfunction, wrap_pymodule}; + +mod common; + +#[test] +fn class_without_docs_or_signature() { + #[pyclass] + struct MyClass {} + + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + + py_assert!(py, typeobj, "typeobj.__doc__ is None"); + py_assert!(py, typeobj, "typeobj.__text_signature__ is None"); +} + +#[test] +fn class_with_docs() { + /// docs line1 + #[pyclass] + /// docs line2 + struct MyClass {} + + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + + py_assert!(py, typeobj, "typeobj.__doc__ == 'docs line1\\ndocs line2'"); + py_assert!(py, typeobj, "typeobj.__text_signature__ is None"); +} + +#[test] +fn class_with_docs_and_signature() { + /// docs line1 + #[pyclass] + /// docs line2 + #[text_signature = "(a, b=None, *, c=42)"] + /// docs line3 + struct MyClass {} + + #[pymethods] + impl MyClass { + #[new] + #[args(a, b = "None", "*", c = 42)] + fn __new__(obj: &PyRawObject, a: i32, b: Option, c: i32) { + let _ = (a, b, c); + obj.init(Self {}); + } + } + + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + + py_assert!( + py, + typeobj, + "typeobj.__doc__ == 'docs line1\\ndocs line2\\ndocs line3'" + ); + py_assert!( + py, + typeobj, + "typeobj.__text_signature__ == '(a, b=None, *, c=42)'" + ); +} + +#[test] +fn class_with_signature() { + #[pyclass] + #[text_signature = "(a, b=None, *, c=42)"] + struct MyClass {} + + #[pymethods] + impl MyClass { + #[new] + #[args(a, b = "None", "*", c = 42)] + fn __new__(obj: &PyRawObject, a: i32, b: Option, c: i32) { + let _ = (a, b, c); + obj.init(Self {}); + } + } + + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + + py_assert!(py, typeobj, "typeobj.__doc__ is None"); + py_assert!( + py, + typeobj, + "typeobj.__text_signature__ == '(a, b=None, *, c=42)'" + ); +} + +#[test] +fn test_function() { + #[pyfunction(a, b = "None", "*", c = 42)] + #[text_signature = "(a, b=None, *, c=42)"] + fn my_function(a: i32, b: Option, c: i32) { + let _ = (a, b, c); + } + + let gil = Python::acquire_gil(); + let py = gil.python(); + let f = wrap_pyfunction!(my_function)(py); + + py_assert!(py, f, "f.__text_signature__ == '(a, b=None, *, c=42)'"); +} + +#[test] +fn test_pyfn() { + #[pymodule] + fn my_module(_py: Python, m: &PyModule) -> PyResult<()> { + #[pyfn(m, "my_function", a, b = "None", "*", c = 42)] + #[text_signature = "(a, b=None, *, c=42)"] + fn my_function(a: i32, b: Option, c: i32) { + let _ = (a, b, c); + } + Ok(()) + } + + let gil = Python::acquire_gil(); + let py = gil.python(); + let m = wrap_pymodule!(my_module)(py); + + py_assert!( + py, + m, + "m.my_function.__text_signature__ == '(a, b=None, *, c=42)'" + ); +} + +#[test] +fn test_methods() { + #[pyclass] + struct MyClass {} + + #[pymethods] + impl MyClass { + #[text_signature = "($self, a)"] + fn method(&self, a: i32) { + let _ = a; + } + #[text_signature = "($self, b)"] + fn pyself_method(_this: PyRef, b: i32) { + let _ = b; + } + #[classmethod] + #[text_signature = "($cls, c)"] + fn class_method(_cls: &PyType, c: i32) { + let _ = c; + } + #[staticmethod] + #[text_signature = "(d)"] + fn static_method(d: i32) { + let _ = d; + } + } + + let gil = Python::acquire_gil(); + let py = gil.python(); + let typeobj = py.get_type::(); + + py_assert!( + py, + typeobj, + "typeobj.method.__text_signature__ == '($self, a)'" + ); + py_assert!( + py, + typeobj, + "typeobj.pyself_method.__text_signature__ == '($self, b)'" + ); + py_assert!( + py, + typeobj, + "typeobj.class_method.__text_signature__ == '($cls, c)'" + ); + py_assert!( + py, + typeobj, + "typeobj.static_method.__text_signature__ == '(d)'" + ); +} From 9a3c47e3cd59116d9b2f00cca123bfe859096e56 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Fri, 29 Nov 2019 13:22:31 -0800 Subject: [PATCH 4/8] add text_signature to documentation --- guide/src/function.md | 74 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) diff --git a/guide/src/function.md b/guide/src/function.md index f13482f054f..26d16a0ef73 100644 --- a/guide/src/function.md +++ b/guide/src/function.md @@ -74,15 +74,72 @@ fn module_with_functions(py: Python, m: &PyModule) -> PyResult<()> { # fn main() {} ``` -### Making the function signature available to Python +## Making the function signature available to Python In order to make the function signature available to Python to be retrieved via -`inspect.signature`, simply make sure the first line of your docstring is -formatted like in the example below. Please note that the newline after the +`inspect.signature`, use the `#[text_signature]` annotation as in the example +below. The `/` signifies the end of positional-only arguments. + +```rust +use pyo3::prelude::*; + +/// This function adds two unsigned 64-bit integers. +#[pyfunction] +#[text_signature = "(a, b, /)"] +fn add(a: u64, b: u64) -> u64 { + a + b +} +``` + +This also works for classes and methods: + +```rust +use pyo3::prelude::*; +use pyo3::types::PyType; + +// it works even if the item is not documented: + +#[pyclass] +#[text_signature = "(c, d, /)"] +struct MyClass {} + +#[pymethods] +impl MyClass { + // the signature for the constructor is attached + // to the struct definition instead. + #[new] + fn new(obj: &PyRawObject, c: i32, d: &str) { + obj.init(Self {}); + } + // the self argument should be written $self + #[text_signature = "($self, e, f)"] + fn my_method(&self, e: i32, f: i32) -> i32 { + e + f + } + #[classmethod] + #[text_signature = "(cls, e, f)"] + fn my_class_method(cls: &PyType, e: i32, f: i32) -> i32 { + e + f + } + #[staticmethod] + #[text_signature = "(e, f)"] + fn my_static_method(e: i32, f: i32) -> i32 { + e + f + } +} +``` + +### Making the function signature available to Python (old method) + +Alternatively, simply make sure the first line of your docstring is +formatted like in the following example. Please note that the newline after the `--` is mandatory. The `/` signifies the end of positional-only arguments. This is not a feature of this library in particular, but the general format used by CPython for annotating signatures of built-in functions. +`#[text_signature]` should be preferred, since it will override automatically +generated signatures when those are added in a future version of PyO3. + ```rust use pyo3::prelude::*; @@ -94,6 +151,17 @@ use pyo3::prelude::*; fn add(a: u64, b: u64) -> u64 { a + b } + +// a function with a signature but without docs. Both blank lines after the `--` are mandatory. + +/// sub(a, b, /) +/// -- +/// +/// +#[pyfunction] +fn sub(a: u64, b: u64) -> u64 { + a - b +} ``` When annotated like this, signatures are also correctly displayed in IPython. From 9ac66ac1eb773995e0517271bc3a501ae9776504 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Fri, 29 Nov 2019 13:31:21 -0800 Subject: [PATCH 5/8] add text_signature to changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21b1497490d..cce17c8163a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased +### Added + +* Support for `#[text_signature]` attribute. [#675](https://github.com/PyO3/pyo3/pull/675) + ## [0.8.3] ### Fixed From 07611b0358bc2fc49e0858051fb6d7f7f8221d9c Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Fri, 29 Nov 2019 13:34:04 -0800 Subject: [PATCH 6/8] remove unnecessary lifetime --- pyo3-derive-backend/src/method.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyo3-derive-backend/src/method.rs b/pyo3-derive-backend/src/method.rs index 75e78d460ad..f95fe820f25 100644 --- a/pyo3-derive-backend/src/method.rs +++ b/pyo3-derive-backend/src/method.rs @@ -47,10 +47,10 @@ pub fn get_return_info(output: &syn::ReturnType) -> syn::Type { impl<'a> FnSpec<'a> { /// Parser function signature and function attributes - pub fn parse<'b>( + pub fn parse( name: &'a syn::Ident, sig: &'a syn::Signature, - meth_attrs: &'b mut Vec, + meth_attrs: &mut Vec, ) -> syn::Result> { let (mut fn_type, fn_attrs) = parse_attributes(meth_attrs)?; From 95c2e2f5858c435a11870ac95bc60d615448a0a0 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Tue, 3 Dec 2019 16:42:48 -0800 Subject: [PATCH 7/8] clean up parse_text_signature_attrs and get_doc --- pyo3-derive-backend/src/utils.rs | 114 +++++++++++++------------------ 1 file changed, 48 insertions(+), 66 deletions(-) diff --git a/pyo3-derive-backend/src/utils.rs b/pyo3-derive-backend/src/utils.rs index cc90aa5fb23..84b445b6f1f 100644 --- a/pyo3-derive-backend/src/utils.rs +++ b/pyo3-derive-backend/src/utils.rs @@ -28,17 +28,10 @@ pub fn is_text_signature_attr(attr: &syn::Attribute) -> bool { fn parse_text_signature_attr( attr: &syn::Attribute, python_name: &T, - text_signature_line: &mut Option, -) -> syn::Result> { +) -> syn::Result> { if !is_text_signature_attr(attr) { return Ok(None); } - if text_signature_line.is_some() { - return Err(syn::Error::new_spanned( - attr, - "text_signature attribute already specified previously", - )); - } let python_name_str = python_name.to_string(); let python_name_str = python_name_str .rsplit('.') @@ -57,56 +50,46 @@ fn parse_text_signature_attr( .. }) => { let value = lit.value(); - if !value.starts_with('(') { - return Err(syn::Error::new_spanned( - lit, - "text_signature must start with \"(\"", - )); - } - if !value.ends_with(')') { - return Err(syn::Error::new_spanned( + if value.starts_with('(') && value.ends_with(')') { + Ok(Some(syn::LitStr::new( + &(python_name_str.to_owned() + &value), + lit.span(), + ))) + } else { + Err(syn::Error::new_spanned( lit, - "text_signature must end with \")\"", - )); + "text_signature must start with \"(\" and end with \")\"", + )) } - *text_signature_line = Some(syn::LitStr::new( - &(python_name_str.to_owned() + &value), - lit.span(), - )); } - meta => { - return Err(syn::Error::new_spanned( - meta, - "text_signature must be of the form #[text_signature = \"\"]", - )); - } - }; - Ok(Some(())) + meta => Err(syn::Error::new_spanned( + meta, + "text_signature must be of the form #[text_signature = \"\"]", + )), + } } pub fn parse_text_signature_attrs( attrs: &mut Vec, python_name: &T, ) -> syn::Result> { - let mut parse_error: Option = None; let mut text_signature = None; - attrs.retain(|attr| { - match parse_text_signature_attr(attr, python_name, &mut text_signature) { - Ok(None) => return true, - Ok(Some(_)) => {} - Err(err) => { - if let Some(parse_error) = &mut parse_error { - parse_error.combine(err); - } else { - parse_error = Some(err); - } + let mut attrs_out = Vec::with_capacity(attrs.len()); + for attr in attrs.drain(..) { + if let Some(value) = parse_text_signature_attr(&attr, python_name)? { + if text_signature.is_some() { + return Err(syn::Error::new_spanned( + attr, + "text_signature attribute already specified previously", + )); + } else { + text_signature = Some(value); } + } else { + attrs_out.push(attr); } - false - }); - if let Some(parse_error) = parse_error { - return Err(parse_error); } + *attrs = attrs_out; Ok(text_signature) } @@ -116,31 +99,35 @@ pub fn get_doc( text_signature: Option, null_terminated: bool, ) -> syn::Result { - let mut doc = Vec::new(); - let mut needs_terminating_newline = false; + let mut doc = String::new(); + let mut span = Span::call_site(); if let Some(text_signature) = text_signature { - doc.push(text_signature.value()); - doc.push("--".to_string()); - doc.push(String::new()); - needs_terminating_newline = true; + // create special doc string lines to set `__text_signature__` + span = text_signature.span(); + doc.push_str(&text_signature.value()); + doc.push_str("\n--\n\n"); } - // TODO(althonos): set span on produced doc str literal - // let mut span = None; + let mut separator = ""; + let mut first = true; for attr in attrs.iter() { if let Ok(syn::Meta::NameValue(ref metanv)) = attr.parse_meta() { if metanv.path.is_ident("doc") { - // span = Some(metanv.span()); if let syn::Lit::Str(ref litstr) = metanv.lit { + if first { + first = false; + span = litstr.span(); + } let d = litstr.value(); - doc.push(if d.starts_with(' ') { - d[1..d.len()].to_string() + doc.push_str(separator); + if d.starts_with(' ') { + doc.push_str(&d[1..d.len()]); } else { - d - }); - needs_terminating_newline = false; + doc.push_str(&d); + }; + separator = "\n"; } else { return Err(syn::Error::new_spanned(metanv, "Invalid doc comment")); } @@ -148,14 +135,9 @@ pub fn get_doc( } } - if needs_terminating_newline { - doc.push(String::new()); - } - - let mut docstr = doc.join("\n"); if null_terminated { - docstr.push('\0'); + doc.push('\0'); } - Ok(syn::Lit::Str(syn::LitStr::new(&docstr, Span::call_site()))) + Ok(syn::Lit::Str(syn::LitStr::new(&doc, span))) } From a676e6d5aaef84360b769ac3daab80eb6e8b6a26 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Fri, 6 Dec 2019 15:05:41 -0800 Subject: [PATCH 8/8] clean up gen_py_method and fix typos --- pyo3-derive-backend/src/pymethod.rs | 78 +++++++++++++---------------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index b7ade9c3b47..ba32667c606 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -14,52 +14,46 @@ pub fn gen_py_method( let spec = FnSpec::parse(name, sig, &mut *meth_attrs)?; + let mut parse_erroneous_text_signature = |alt_name: Option<&str>, error_msg: &str| { + let python_name; + let python_name = match alt_name { + None => name, + Some(alt_name) => { + python_name = syn::Ident::new(alt_name, name.span()); + &python_name + } + }; + // try to parse anyway to give better error messages + if let Some(text_signature) = + utils::parse_text_signature_attrs(&mut *meth_attrs, python_name)? + { + Err(syn::Error::new_spanned(text_signature, error_msg)) + } else { + Ok(None) + } + }; + let text_signature = match &spec.tp { FnType::Fn | FnType::PySelf(_) | FnType::FnClass | FnType::FnStatic => { utils::parse_text_signature_attrs(&mut *meth_attrs, name)? } - FnType::FnNew => { - // try to parse anyway to give better error messages - if let Some(type_signature) = utils::parse_text_signature_attrs( - &mut *meth_attrs, - &syn::Ident::new("__new__", name.span()), - )? { - return Err(syn::Error::new_spanned( - type_signature, - "type_signature not allowed on __new__, put it on the struct definition instead", - )); - } - None - } - FnType::FnCall => { - // try to parse anyway to give better error messages - if let Some(type_signature) = utils::parse_text_signature_attrs( - &mut *meth_attrs, - &syn::Ident::new("__call__", name.span()), - )? { - return Err(syn::Error::new_spanned( - type_signature, - "type_signature not allowed on __call__, put it on the struct definition instead", - )); - } - None - } - FnType::Getter(get_set_name) | FnType::Setter(get_set_name) => { - // try to parse anyway to give better error messages - let get_set_name = match get_set_name { - None => name.clone(), - Some(get_set_name) => syn::Ident::new(get_set_name, name.span()), - }; - if let Some(type_signature) = - utils::parse_text_signature_attrs(&mut *meth_attrs, &get_set_name)? - { - return Err(syn::Error::new_spanned( - type_signature, - "type_signature not allowed on getters/setters", - )); - } - None - } + FnType::FnNew => parse_erroneous_text_signature( + Some("__new__"), + "text_signature not allowed on __new__; if you want to add a signature on \ + __new__, put it on the struct definition instead", + )?, + FnType::FnCall => parse_erroneous_text_signature( + Some("__call__"), + "text_signature not allowed on __call__", + )?, + FnType::Getter(getter_name) => parse_erroneous_text_signature( + getter_name.as_ref().map(|v| &**v), + "text_signature not allowed on getter", + )?, + FnType::Setter(setter_name) => parse_erroneous_text_signature( + setter_name.as_ref().map(|v| &**v), + "text_signature not allowed on setter", + )?, }; let doc = utils::get_doc(&meth_attrs, text_signature, true)?;