diff --git a/Lib/test/test_pydoc/test_pydoc.py b/Lib/test/test_pydoc/test_pydoc.py index d206e5a910d..f44c56da652 100644 --- a/Lib/test/test_pydoc/test_pydoc.py +++ b/Lib/test/test_pydoc/test_pydoc.py @@ -1582,7 +1582,6 @@ def test_module_level_callable(self): self.assertEqual(self._get_summary_line(os.stat), "stat(path, *, dir_fd=None, follow_symlinks=True)") - @unittest.expectedFailure # TODO: RUSTPYTHON def test_module_level_callable_noargs(self): self.assertEqual(self._get_summary_line(time.time), "time()") diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index 809d3164b4a..c612572d3dc 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -1096,7 +1096,10 @@ where args.attrs.push(allow_attr); } - let doc = args.attrs.doc().map(|doc| format_doc(&sig_doc, &doc)); + let doc = args.attrs.doc().map(|doc| match &sig_doc { + Some(sig_doc) => format_doc(sig_doc, &doc), + None => doc, + }); args.context.method_items.add_item(MethodNurseryItem { py_name, cfgs: args.cfgs.to_vec(), diff --git a/crates/derive-impl/src/pymodule.rs b/crates/derive-impl/src/pymodule.rs index 32d7a0fa6bf..20c94abe94f 100644 --- a/crates/derive-impl/src/pymodule.rs +++ b/crates/derive-impl/src/pymodule.rs @@ -524,7 +524,7 @@ struct FunctionNurseryItem { py_names: Vec, cfgs: Vec, ident: Ident, - doc: String, + doc: Option, call_flags: TokenStream, } @@ -556,8 +556,10 @@ impl ToTokens for ValidatedFunctionNursery { let cfgs = &item.cfgs; let cfgs = quote!(#(#cfgs)*); let py_names = &item.py_names; - let doc = &item.doc; - let doc = quote!(Some(#doc)); + let doc = match &item.doc { + Some(doc) => quote!(Some(#doc)), + None => quote!(None), + }; let flags = &item.call_flags; inner_tokens.extend(quote![ @@ -671,10 +673,10 @@ impl ModuleItem for FunctionItem { .copied() .map(str::to_owned) }); - let doc = if let Some(doc) = doc { - format_doc(&sig_doc, &doc) - } else { - sig_doc + let doc = match (sig_doc, doc) { + (Some(sig_doc), Some(doc)) => Some(format_doc(&sig_doc, &doc)), + (Some(sig_doc), None) => Some(sig_doc), + (None, doc) => doc, }; let py_names = { diff --git a/crates/derive-impl/src/util.rs b/crates/derive-impl/src/util.rs index 1ee878c1313..1e734d94a50 100644 --- a/crates/derive-impl/src/util.rs +++ b/crates/derive-impl/src/util.rs @@ -732,13 +732,20 @@ where // Best effort attempt to generate a template from which a // __text_signature__ can be created. -pub(crate) fn text_signature(sig: &Signature, name: &str) -> String { - let signature = func_sig(sig); - if signature.starts_with("$self") { +// +// Unlike CPython, a `#[pyfunction]` doesn't take the module as an argument, +// so there's no module to mark with `$module`. +pub(crate) fn text_signature(sig: &Signature, name: &str) -> Option { + let signature = func_sig(sig)?; + // Arguments bind through `FuncArgs::take_positional`, which never consults + // the keyword map, so they are positional-only. `*args`/`**kwargs` cannot be + // followed by `/`, and an empty parameter list has nothing to mark. + let signature = if signature.is_empty() || signature.contains('*') { format!("{name}({signature})") } else { - format!("{}({}, {})", name, "$module", signature) - } + format!("{name}({signature}, /)") + }; + Some(signature) } pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) -> TokenStream { @@ -812,37 +819,45 @@ pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) } } -fn func_sig(sig: &Signature) -> String { - sig.inputs - .iter() - .filter_map(|arg| { - let arg = match arg { - FnArg::Typed(typed) => typed, - FnArg::Receiver(_) => return Some("$self".to_owned()), - }; - let ty = arg.ty.as_ref(); - let ty = quote!(#ty).to_string(); - if ty == "FuncArgs" { - return Some("*args, **kwargs".to_owned()); - } - if ty.starts_with('&') && ty.ends_with("VirtualMachine") { - return None; - } - let ident = match arg.pat.as_ref() { - syn::Pat::Ident(p) => p.ident.to_string(), - // FIXME: other => unreachable!("function arg pattern must be ident but found `{}`", quote!(fn #ident(.. #other ..))), - other => quote!(#other).to_string(), - }; - if ident == "zelf" { - return Some("$self".to_owned()); - } - if ident == "vm" { - unreachable!("type &VirtualMachine(`{ty}`) must be filtered already"); +/// Returns None when an argument has no name to report, in which case no +/// signature can be generated for the function. +fn func_sig(sig: &Signature) -> Option { + let mut params = Vec::new(); + for arg in &sig.inputs { + let arg = match arg { + FnArg::Typed(typed) => typed, + FnArg::Receiver(_) => { + params.push("$self".to_owned()); + continue; } - Some(ident) - }) - .collect::>() - .join(", ") + }; + let ty = arg.ty.as_ref(); + let ty = quote!(#ty).to_string(); + if ty == "FuncArgs" { + params.push("*args, **kwargs".to_owned()); + continue; + } + if ty.starts_with('&') && ty.ends_with("VirtualMachine") { + continue; + } + // An argument bound by a destructuring pattern, e.g. + // `fn round(RoundArgs { number, ndigits }: RoundArgs, ..)`, has no name + // to report. Stringifying the pattern would emit Rust syntax, which + // makes inspect.signature() raise "invalid signature". + let syn::Pat::Ident(pat) = arg.pat.as_ref() else { + return None; + }; + let ident = pat.ident.to_string(); + if ident == "zelf" { + params.push("$self".to_owned()); + continue; + } + if ident == "vm" { + unreachable!("type &VirtualMachine(`{ty}`) must be filtered already"); + } + params.push(ident); + } + Some(params.join(", ")) } pub(crate) fn format_doc(sig: &str, doc: &str) -> String { diff --git a/crates/vm/src/stdlib/builtins.rs b/crates/vm/src/stdlib/builtins.rs index 35f404f0f3b..e5d4854434a 100644 --- a/crates/vm/src/stdlib/builtins.rs +++ b/crates/vm/src/stdlib/builtins.rs @@ -74,8 +74,8 @@ mod builtins { } #[pyfunction] - fn bin(x: PyIntRef) -> String { - let x = x.as_bigint(); + fn bin(number: PyIntRef) -> String { + let x = number.as_bigint(); if x.is_negative() { format!("-0b{:b}", x.abs()) } else { @@ -391,11 +391,11 @@ mod builtins { } #[pyfunction] - fn delattr(obj: PyObjectRef, attr: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let attr = attr.try_to_ref::(vm).map_err(|_e| { + fn delattr(obj: PyObjectRef, name: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { + let attr = name.try_to_ref::(vm).map_err(|_e| { vm.new_type_error(format!( "attribute name must be string, not '{}'", - attr.class().name() + name.class().name() )) })?; obj.del_attr(attr, vm) @@ -407,8 +407,8 @@ mod builtins { } #[pyfunction] - fn divmod(a: PyObjectRef, b: PyObjectRef, vm: &VirtualMachine) -> PyResult { - vm._divmod(&a, &b) + fn divmod(x: PyObjectRef, y: PyObjectRef, vm: &VirtualMachine) -> PyResult { + vm._divmod(&x, &y) } #[derive(FromArgs)] @@ -712,11 +712,11 @@ mod builtins { } #[pyfunction] - fn hasattr(obj: PyObjectRef, attr: PyObjectRef, vm: &VirtualMachine) -> PyResult { - let attr = attr.try_to_ref::(vm).map_err(|_e| { + fn hasattr(obj: PyObjectRef, name: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let attr = name.try_to_ref::(vm).map_err(|_e| { vm.new_type_error(format!( "attribute name must be string, not '{}'", - attr.class().name() + name.class().name() )) })?; Ok(vm.get_attribute_opt(obj, attr)?.is_some()) @@ -818,13 +818,21 @@ mod builtins { } #[pyfunction] - fn isinstance(obj: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { - obj.is_instance(&typ, vm) + fn isinstance( + obj: PyObjectRef, + class_or_tuple: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + obj.is_instance(&class_or_tuple, vm) } #[pyfunction] - fn issubclass(subclass: PyObjectRef, typ: PyObjectRef, vm: &VirtualMachine) -> PyResult { - subclass.is_subclass(&typ, vm) + fn issubclass( + cls: PyObjectRef, + class_or_tuple: PyObjectRef, + vm: &VirtualMachine, + ) -> PyResult { + cls.is_subclass(&class_or_tuple, vm) } #[pyfunction] @@ -845,8 +853,8 @@ mod builtins { } #[pyfunction] - fn aiter(iter_target: PyObjectRef, vm: &VirtualMachine) -> PyResult { - iter_target.get_aiter(vm) + fn aiter(async_iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult { + async_iterable.get_aiter(vm) } #[pyfunction] @@ -994,8 +1002,8 @@ mod builtins { } #[pyfunction] - fn ord(string: Either, vm: &VirtualMachine) -> PyResult { - match string { + fn ord(character: Either, vm: &VirtualMachine) -> PyResult { + match character { Either::A(bytes) => bytes.with_ref(|bytes| { let bytes_len = bytes.len(); if bytes_len != 1 { @@ -1138,14 +1146,14 @@ mod builtins { #[pyfunction] fn setattr( obj: PyObjectRef, - attr: PyObjectRef, + name: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<()> { - let attr = attr.try_to_ref::(vm).map_err(|_e| { + let attr = name.try_to_ref::(vm).map_err(|_e| { vm.new_type_error(format!( "attribute name must be string, not '{}'", - attr.class().name() + name.class().name() )) })?; obj.set_attr(attr, value, vm)?; diff --git a/extra_tests/snippets/builtin_signature.py b/extra_tests/snippets/builtin_signature.py new file mode 100644 index 00000000000..320a395d882 --- /dev/null +++ b/extra_tests/snippets/builtin_signature.py @@ -0,0 +1,66 @@ +import inspect +import sys + +# __text_signature__ is generated from the Rust parameter list, so it must not +# describe parameters the function does not actually take, and must mark the +# ones it does take as positional-only. + +# No phantom `module` parameter. RustPython's #[pyfunction]s take no module +# argument, so `__self__` is None and inspect has nothing to strip. +for f in (len, abs, hash, id, repr, bin, ord, divmod, hex, oct, chr, callable): + assert "module" not in inspect.signature(f).parameters, f.__name__ + +# Plain arguments bind through take_positional(), so they are positional-only. +try: + len(obj=[1, 2]) +except TypeError: + pass +else: + raise AssertionError("len() should not accept keyword arguments") + +assert str(inspect.signature(len)) == "(obj, /)" +assert str(inspect.signature(abs)) == "(x, /)" +assert str(inspect.signature(hash)) == "(obj, /)" +assert str(inspect.signature(chr)) == "(i, /)" +assert str(inspect.signature(callable)) == "(obj, /)" + +assert ( + inspect.signature(len).parameters["obj"].kind == inspect.Parameter.POSITIONAL_ONLY +) + +# *args/**kwargs cannot be followed by `/`. The parameter names themselves still +# differ from CPython here, which is out of scope. +breakpoint_kinds = [p.kind for p in inspect.signature(breakpoint).parameters.values()] +assert breakpoint_kinds == [ + inspect.Parameter.VAR_POSITIONAL, + inspect.Parameter.VAR_KEYWORD, +], breakpoint_kinds + +# Parameter names follow CPython, so signatures are directly comparable. +assert str(inspect.signature(bin)) == "(number, /)" +assert str(inspect.signature(ord)) == "(character, /)" +assert str(inspect.signature(divmod)) == "(x, y, /)" +assert str(inspect.signature(hasattr)) == "(obj, name, /)" +assert str(inspect.signature(setattr)) == "(obj, name, value, /)" +assert str(inspect.signature(delattr)) == "(obj, name, /)" +assert str(inspect.signature(isinstance)) == "(obj, class_or_tuple, /)" +assert str(inspect.signature(issubclass)) == "(cls, class_or_tuple, /)" +assert str(inspect.signature(aiter)) == "(async_iterable, /)" + +if sys.implementation.name == "rustpython": + # Functions whose Rust arguments are destructuring patterns rather than + # plain names get no signature at all, instead of emitting text that is not + # valid Python and makes inspect.signature() raise "invalid signature". + # + # CPython does have signatures for these, hand-written via Argument Clinic. + # We cannot derive them until FromArgs reports the parameters of its own + # structs, so until then we report no signature, which is at least how + # CPython behaves for the builtins it has no signature for. + for f in (round, sum): + assert f.__text_signature__ is None, f.__name__ + try: + inspect.signature(f) + except ValueError as e: + assert "no signature found" in str(e), str(e) + else: + raise AssertionError(f"{f.__name__} should have no signature")