Improve default value for None in text_signature

PyO3 · Mar 29, 2023 · 57dbc94 · 57dbc94
1 parent 90d50da
commit 57dbc94
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 19 deletions.
diff --git a/guide/src/function/signature.md b/guide/src/function/signature.md
@@ -148,7 +148,7 @@ fn increment(x: u64, amount: Option<u64>) -> u64 {
 #             .extract()?;
 #
 #         #[cfg(Py_3_8)]  // on 3.7 the signature doesn't render b, upstream bug?
-#         assert_eq!(sig, "(x, amount=Ellipsis)");
+#         assert_eq!(sig, "(x, amount=None)");
 #
 #         Ok(())
 #     })

diff --git a/newsfragments/3066.changed.md b/newsfragments/3066.changed.md
@@ -0,0 +1 @@
+Improve default value for `None` in `text_signature`.
diff --git a/pyo3-macros-backend/src/pyfunction/signature.rs b/pyo3-macros-backend/src/pyfunction/signature.rs
@@ -588,21 +588,36 @@ impl<'a> FunctionSignature<'a> {
     fn default_value_for_parameter(&self, parameter: &str) -> String {
         let mut default = "...".to_string();
         if let Some(fn_arg) = self.arguments.iter().find(|arg| arg.name == parameter) {
-            if let Some(syn::Expr::Lit(syn::ExprLit { lit, .. })) = fn_arg.default.as_ref() {
-                match lit {
-                    syn::Lit::Str(s) => default = s.token().to_string(),
-                    syn::Lit::Char(c) => default = c.token().to_string(),
-                    syn::Lit::Int(i) => default = i.base10_digits().to_string(),
-                    syn::Lit::Float(f) => default = f.base10_digits().to_string(),
-                    syn::Lit::Bool(b) => {
-                        default = if b.value() {
-                            "True".to_string()
-                        } else {
-                            "False".to_string()
+            if let Some(arg_default) = fn_arg.default.as_ref() {
+                match arg_default {
+                    // literal values
+                    syn::Expr::Lit(syn::ExprLit { lit, .. }) => match lit {
+                        syn::Lit::Str(s) => default = s.token().to_string(),
+                        syn::Lit::Char(c) => default = c.token().to_string(),
+                        syn::Lit::Int(i) => default = i.base10_digits().to_string(),
+                        syn::Lit::Float(f) => default = f.base10_digits().to_string(),
+                        syn::Lit::Bool(b) => {
+                            default = if b.value() {
+                                "True".to_string()
+                            } else {
+                                "False".to_string()
+                            }
                         }
+                        _ => {}
+                    },
+                    // None
+                    syn::Expr::Path(syn::ExprPath {
+                        qself: None, path, ..
+                    }) if path.is_ident("None") => {
+                        default = "None".to_string();
                     }
+                    // others, unsupported yet so defaults to `...`
                     _ => {}
                 }
+            } else if fn_arg.optional.is_some() {
+                // functions without a `#[pyo3(signature = (...))]` option
+                // will treat trailing `Option<T>` arguments as having a default of `None`
+                default = "None".to_string();
             }
         }
         default

diff --git a/tests/test_text_signature.rs b/tests/test_text_signature.rs
@@ -152,28 +152,52 @@ fn test_auto_test_signature_function() {
         let _ = (a, b, c, d, e, f, h);
     }
 
+    #[pyfunction]
+    fn my_function_6(a: i32, b: Option<i32>, c: Option<i32>) {
+        let _ = (a, b, c);
+    }
+
     Python::with_gil(|py| {
         let f = wrap_pyfunction!(my_function)(py).unwrap();
-        py_assert!(py, f, "f.__text_signature__ == '(a, b, c)'");
+        py_assert!(
+            py,
+            f,
+            "f.__text_signature__ == '(a, b, c)', f.__text_signature__"
+        );
 
         let f = wrap_pyfunction!(my_function_2)(py).unwrap();
-        py_assert!(py, f, "f.__text_signature__ == '($module, a, b, c)'");
+        py_assert!(
+            py,
+            f,
+            "f.__text_signature__ == '($module, a, b, c)', f.__text_signature__"
+        );
 
         let f = wrap_pyfunction!(my_function_3)(py).unwrap();
-        py_assert!(py, f, "f.__text_signature__ == '(a, /, b=..., *, c=5)'");
+        py_assert!(
+            py,
+            f,
+            "f.__text_signature__ == '(a, /, b=None, *, c=5)', f.__text_signature__"
+        );
 
         let f = wrap_pyfunction!(my_function_4)(py).unwrap();
         py_assert!(
             py,
             f,
-            "f.__text_signature__ == '(a, /, b=..., *args, c, d=5, **kwargs)'"
+            "f.__text_signature__ == '(a, /, b=None, *args, c, d=5, **kwargs)', f.__text_signature__"
         );
 
         let f = wrap_pyfunction!(my_function_5)(py).unwrap();
         py_assert!(
             py,
             f,
-            "f.__text_signature__ == '(a=1, /, b=..., c=1.5, d=5, e=\"pyo3\", f=\\'f\\', h=True)', f.__text_signature__"
+            "f.__text_signature__ == '(a=1, /, b=None, c=1.5, d=5, e=\"pyo3\", f=\\'f\\', h=True)', f.__text_signature__"
+        );
+
+        let f = wrap_pyfunction!(my_function_6)(py).unwrap();
+        py_assert!(
+            py,
+            f,
+            "f.__text_signature__ == '(a, b=None, c=None)', f.__text_signature__"
         );
     });
 }
@@ -228,12 +252,12 @@ fn test_auto_test_signature_method() {
         py_assert!(
             py,
             cls,
-            "cls.method_2.__text_signature__ == '($self, a, /, b=..., *, c=5)'"
+            "cls.method_2.__text_signature__ == '($self, a, /, b=None, *, c=5)'"
         );
         py_assert!(
             py,
             cls,
-            "cls.method_3.__text_signature__ == '($self, a, /, b=..., *args, c, d=5, **kwargs)'"
+            "cls.method_3.__text_signature__ == '($self, a, /, b=None, *args, c, d=5, **kwargs)'"
         );
         py_assert!(
             py,