allow path in from_py_with and deprecate string literal

guide/src/class/numeric.md

@@ -27,7 +27,7 @@ OverflowError: Python int too large to convert to C long
 ```
 
 Instead of relying on the default [`FromPyObject`] extraction to parse arguments, we can specify our
-own extraction function, using the `#[pyo3(from_py_with = "...")]` attribute. Unfortunately PyO3
+own extraction function, using the `#[pyo3(from_py_with = ...)]` attribute. Unfortunately PyO3
 doesn't provide a way to wrap Python integers out of the box, but we can do a Python call to mask it
 and cast it to an `i32`.
 
@@ -62,7 +62,7 @@ struct Number(i32);
 #[pymethods]
 impl Number {
     #[new]
-    fn new(#[pyo3(from_py_with = "wrap")] value: i32) -> Self {
+    fn new(#[pyo3(from_py_with = wrap)] value: i32) -> Self {
         Self(value)
     }
 }
@@ -225,7 +225,7 @@ struct Number(i32);
 #[pymethods]
 impl Number {
     #[new]
-    fn new(#[pyo3(from_py_with = "wrap")] value: i32) -> Self {
+    fn new(#[pyo3(from_py_with = wrap)] value: i32) -> Self {
         Self(value)
     }
 

guide/src/conversions/traits.md

@@ -484,9 +484,9 @@ If the input is neither a string nor an integer, the error message will be:
 - `pyo3(item)`, `pyo3(item("key"))`
     - retrieve the field from a mapping, possibly with the custom key specified as an argument.
     - can be any literal that implements `ToBorrowedObject`
-- `pyo3(from_py_with = "...")`
+- `pyo3(from_py_with = ...)`
     - apply a custom function to convert the field from Python the desired Rust type.
-    - the argument must be the name of the function as a string.
+    - the argument must be the path to the function.
     - the function signature must be `fn(&Bound<PyAny>) -> PyResult<T>` where `T` is the Rust type of the argument.
 - `pyo3(default)`, `pyo3(default = ...)`
   - if the argument is set, uses the given default value.
@@ -503,7 +503,7 @@ use pyo3::prelude::*;
 
 #[derive(FromPyObject)]
 struct RustyStruct {
-    #[pyo3(item("value"), default, from_py_with = "Bound::<'_, PyAny>::len")]
+    #[pyo3(item("value"), default, from_py_with = Bound::<'_, PyAny>::len)]
     len: usize,
     #[pyo3(item)]
     other: usize,

guide/src/function.md

@@ -101,7 +101,7 @@ The `#[pyo3]` attribute can be used to modify properties of the generated Python
 
 The `#[pyo3]` attribute can be used on individual arguments to modify properties of them in the generated function. It can take any combination of the following options:
 
-  - <a id="from_py_with"></a> `#[pyo3(from_py_with = "...")]`
+  - <a id="from_py_with"></a> `#[pyo3(from_py_with = ...)]`
 
     Set this on an option to specify a custom function to convert the function argument from Python to the desired Rust type, instead of using the default `FromPyObject` extraction. The function signature must be `fn(&Bound<'_, PyAny>) -> PyResult<T>` where `T` is the Rust type of the argument.
 
@@ -115,7 +115,7 @@ The `#[pyo3]` attribute can be used on individual arguments to modify properties
     }
 
     #[pyfunction]
-    fn object_length(#[pyo3(from_py_with = "get_length")] argument: usize) -> usize {
+    fn object_length(#[pyo3(from_py_with = get_length)] argument: usize) -> usize {
         argument
     }
 

newsfragments/4860.changed.md

@@ -0,0 +1 @@
+`#[pyo3(from_py_with = ...)]` now take a path rather than a string literal

pyo3-macros-backend/src/attributes.rs

@@ -349,7 +349,37 @@ impl<K: ToTokens, V: ToTokens> ToTokens for OptionalKeywordAttribute<K, V> {
     }
 }
 
-pub type FromPyWithAttribute = KeywordAttribute<kw::from_py_with, LitStrValue<ExprPath>>;
+#[derive(Debug, Clone)]
+pub struct ExprPathWrap {
+    pub from_lit_str: bool,
+    pub expr_path: ExprPath,
+}
+
+impl Parse for ExprPathWrap {
+    fn parse(input: ParseStream<'_>) -> Result<Self> {
+        match input.parse::<ExprPath>() {
+            Ok(expr_path) => Ok(ExprPathWrap {
+                from_lit_str: false,
+                expr_path,
+            }),
+            Err(e) => match input.parse::<LitStrValue<ExprPath>>() {
+                Ok(LitStrValue(expr_path)) => Ok(ExprPathWrap {
+                    from_lit_str: true,
+                    expr_path,
+                }),
+                Err(_) => Err(e),
+            },
+        }
+    }
+}
+
+impl ToTokens for ExprPathWrap {
+    fn to_tokens(&self, tokens: &mut TokenStream) {
+        self.expr_path.to_tokens(tokens)
+    }
+}
+
+pub type FromPyWithAttribute = KeywordAttribute<kw::from_py_with, ExprPathWrap>;
 
 pub type DefaultAttribute = OptionalKeywordAttribute<Token![default], Expr>;
 

pyo3-macros-backend/src/frompyobject.rs

@@ -3,7 +3,7 @@ use crate::attributes::{
 };
 use crate::utils::Ctx;
 use proc_macro2::TokenStream;
-use quote::{format_ident, quote, ToTokens};
+use quote::{format_ident, quote, quote_spanned, ToTokens};
 use syn::{
     ext::IdentExt,
     parenthesized,
@@ -284,11 +284,23 @@ impl<'a> Container<'a> {
                 },
                 Some(FromPyWithAttribute {
                     value: expr_path, ..
-                }) => quote! {
-                    Ok(#self_ty {
-                        #ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
-                    })
-                },
+                }) => {
+                    let deprecation = expr_path.from_lit_str.then(|| {
+                        quote_spanned! { expr_path.span() =>
+                            #[deprecated(since = "0.24.0", note = "`from_py_with` string literals is deprecated. Use the function path instead.")]
+                            #[allow(dead_code)]
+                            const LIT_STR_DEPRECATION: () = ();
+                            let _: () = LIT_STR_DEPRECATION;
+                        }
+                    }).unwrap_or_default();
+
+                    quote! {
+                        #deprecation
+                        Ok(#self_ty {
+                            #ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, #field_name)?
+                        })
+                    }
+                }
             }
         } else {
             match from_py_with {
@@ -298,9 +310,20 @@ impl<'a> Container<'a> {
 
                 Some(FromPyWithAttribute {
                     value: expr_path, ..
-                }) => quote! {
-                    #pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
-                },
+                }) => {
+                    let deprecation = expr_path.from_lit_str.then(|| {
+                        quote_spanned! { expr_path.span() =>
+                            #[deprecated(since = "0.24.0", note = "`from_py_with` string literals is deprecated. Use the function path instead.")]
+                            #[allow(dead_code)]
+                            const LIT_STR_DEPRECATION: () = ();
+                            let _: () = LIT_STR_DEPRECATION;
+                        }
+                    }).unwrap_or_default();
+                    quote! {
+                        #deprecation
+                        #pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#expr_path as fn(_) -> _, obj, #struct_name, 0).map(#self_ty)
+                    }
+                }
             }
         }
     }
@@ -325,7 +348,21 @@ impl<'a> Container<'a> {
             }
         });
 
+        let deprecations = struct_fields
+            .iter()
+            .filter_map(|fields| fields.from_py_with.as_ref()).filter(|f|f.value.from_lit_str)
+            .map(|f| {
+                quote_spanned! { f.value.span() => {
+                    #[deprecated(since = "0.24.0", note = "`from_py_with` string literals is deprecated. Use the function path instead.")]
+                    #[allow(dead_code)]
+                    const LIT_STR_DEPRECATION: () = ();
+                    let _: () = LIT_STR_DEPRECATION;
+                }}
+            })
+            .collect::<TokenStream>();
+
         quote!(
+            #deprecations
             match #pyo3_path::types::PyAnyMethods::extract(obj) {
                 ::std::result::Result::Ok((#(#field_idents),*)) => ::std::result::Result::Ok(#self_ty(#(#fields),*)),
                 ::std::result::Result::Err(err) => ::std::result::Result::Err(err),
@@ -387,7 +424,18 @@ impl<'a> Container<'a> {
             fields.push(quote!(#ident: #extracted));
         }
 
-        quote!(::std::result::Result::Ok(#self_ty{#fields}))
+        let d = struct_fields
+            .iter()
+            .filter_map(|field| field.from_py_with.as_ref())
+            .filter(|f| f.value.from_lit_str)
+            .map(|f| quote_spanned! { f.value.span() => {
+                #[deprecated(since = "0.24.0", note = "`from_py_with` string literals is deprecated. Use the function path instead.")]
+                #[allow(dead_code)]
+                const LIT_STR_DEPRECATION: () = ();
+                let _: () = LIT_STR_DEPRECATION;
+            }}).collect::<TokenStream>();
+
+        quote!(#d ::std::result::Result::Ok(#self_ty{#fields}))
     }
 }
 

pyo3-macros-backend/src/params.rs

@@ -61,7 +61,17 @@ pub fn impl_arg_params(
         .filter_map(|(i, arg)| {
             let from_py_with = &arg.from_py_with()?.value;
             let from_py_with_holder = format_ident!("from_py_with_{}", i);
+            let d = from_py_with
+                .from_lit_str
+                .then(|| quote_spanned! { from_py_with.span() =>
+                    #[deprecated(since = "0.24.0", note = "`from_py_with` string literals is deprecated. Use the function path instead.")]
+                    #[allow(dead_code)]
+                    const LIT_STR_DEPRECATION: () = ();
+                    let _: () = LIT_STR_DEPRECATION;
+                })
+                .unwrap_or_default();
             Some(quote_spanned! { from_py_with.span() =>
+                #d
                 let #from_py_with_holder = #from_py_with;
             })
         })

pyo3-macros-backend/src/pymethod.rs

@@ -657,18 +657,26 @@ pub fn impl_py_setter_def(
         PropertyType::Function { spec, .. } => {
             let (_, args) = split_off_python_arg(&spec.signature.arguments);
             let value_arg = &args[0];
-            let (from_py_with, ident) =
-                if let Some(from_py_with) = &value_arg.from_py_with().as_ref().map(|f| &f.value) {
-                    let ident = syn::Ident::new("from_py_with", from_py_with.span());
-                    (
-                        quote_spanned! { from_py_with.span() =>
-                            let #ident = #from_py_with;
-                        },
-                        ident,
-                    )
-                } else {
-                    (quote!(), syn::Ident::new("dummy", Span::call_site()))
-                };
+            let (from_py_with, ident) = if let Some(from_py_with) =
+                &value_arg.from_py_with().as_ref().map(|f| &f.value)
+            {
+                let ident = syn::Ident::new("from_py_with", from_py_with.span());
+                let d = from_py_with.from_lit_str.then(|| quote_spanned! { from_py_with.span() =>
+                        #[deprecated(since = "0.24.0", note = "`from_py_with` string literals is deprecated. Use the function path instead.")]
+                        #[allow(dead_code)]
+                        const LIT_STR_DEPRECATION: () = ();
+                        let _: () = LIT_STR_DEPRECATION;
+                    }).unwrap_or_default();
+                (
+                    quote_spanned! { from_py_with.span() =>
+                        #d
+                        let #ident = #from_py_with;
+                    },
+                    ident,
+                )
+            } else {
+                (quote!(), syn::Ident::new("dummy", Span::call_site()))
+            };
 
             let arg = if let FnArg::Regular(arg) = &value_arg {
                 arg

tests/test_class_basics.rs

@@ -357,23 +357,23 @@ struct ClassWithFromPyWithMethods {}
 
 #[pymethods]
 impl ClassWithFromPyWithMethods {
-    fn instance_method(&self, #[pyo3(from_py_with = "get_length")] argument: usize) -> usize {
+    fn instance_method(&self, #[pyo3(from_py_with = get_length)] argument: usize) -> usize {
         argument
     }
     #[classmethod]
     fn classmethod(
         _cls: &Bound<'_, PyType>,
-        #[pyo3(from_py_with = "Bound::<'_, PyAny>::len")] argument: usize,
+        #[pyo3(from_py_with = Bound::<'_, PyAny>::len)] argument: usize,
     ) -> usize {
         argument
     }
 
     #[staticmethod]
-    fn staticmethod(#[pyo3(from_py_with = "get_length")] argument: usize) -> usize {
+    fn staticmethod(#[pyo3(from_py_with = get_length)] argument: usize) -> usize {
         argument
     }
 
-    fn __contains__(&self, #[pyo3(from_py_with = "is_even")] obj: bool) -> bool {
+    fn __contains__(&self, #[pyo3(from_py_with = is_even)] obj: bool) -> bool {
         obj
     }
 }

tests/test_compile_error.rs

@@ -69,4 +69,5 @@ fn test_compile_errors() {
     t.compile_fail("tests/ui/invalid_base_class.rs");
     t.pass("tests/ui/ambiguous_associated_items.rs");
     t.pass("tests/ui/pyclass_probe.rs");
+    t.compile_fail("tests/ui/deprecated.rs");
 }

tests/test_frompyobject.rs

@@ -543,7 +543,7 @@ pub struct Zap {
     #[pyo3(item)]
     name: String,
 
-    #[pyo3(from_py_with = "Bound::<'_, PyAny>::len", item("my_object"))]
+    #[pyo3(from_py_with = Bound::<'_, PyAny>::len, item("my_object"))]
     some_object_length: usize,
 }
 
@@ -568,7 +568,7 @@ fn test_from_py_with() {
 #[derive(Debug, FromPyObject)]
 pub struct ZapTuple(
     String,
-    #[pyo3(from_py_with = "Bound::<'_, PyAny>::len")] usize,
+    #[pyo3(from_py_with = Bound::<'_, PyAny>::len)] usize,
 );
 
 #[test]
@@ -608,10 +608,10 @@ fn test_from_py_with_tuple_struct_error() {
 
 #[derive(Debug, FromPyObject, PartialEq, Eq)]
 pub enum ZapEnum {
-    Zip(#[pyo3(from_py_with = "Bound::<'_, PyAny>::len")] usize),
+    Zip(#[pyo3(from_py_with = Bound::<'_, PyAny>::len)] usize),
     Zap(
         String,
-        #[pyo3(from_py_with = "Bound::<'_, PyAny>::len")] usize,
+        #[pyo3(from_py_with = Bound::<'_, PyAny>::len)] usize,
     ),
 }
 
@@ -632,7 +632,7 @@ fn test_from_py_with_enum() {
 #[derive(Debug, FromPyObject, PartialEq, Eq)]
 #[pyo3(transparent)]
 pub struct TransparentFromPyWith {
-    #[pyo3(from_py_with = "Bound::<'_, PyAny>::len")]
+    #[pyo3(from_py_with = Bound::<'_, PyAny>::len)]
     len: usize,
 }
 
@@ -730,7 +730,7 @@ fn test_with_explicit_default_item() {
 
 #[derive(Debug, FromPyObject, PartialEq, Eq)]
 pub struct WithDefaultItemAndConversionFunction {
-    #[pyo3(item, default, from_py_with = "Bound::<'_, PyAny>::len")]
+    #[pyo3(item, default, from_py_with = Bound::<'_, PyAny>::len)]
     opt: usize,
     #[pyo3(item)]
     value: usize,

tests/test_getter_setter.rs

@@ -43,7 +43,7 @@ impl ClassWithProperties {
     }
 
     #[setter]
-    fn set_from_len(&mut self, #[pyo3(from_py_with = "extract_len")] value: i32) {
+    fn set_from_len(&mut self, #[pyo3(from_py_with = extract_len)] value: i32) {
         self.num = value;
     }
 

tests/test_methods.rs

@@ -1198,7 +1198,7 @@ fn test_issue_2988() {
         _data: Vec<i32>,
         // The from_py_with here looks a little odd, we just need some way
         // to encourage the macro to expand the from_py_with default path too
-        #[pyo3(from_py_with = "<Bound<'_, _> as PyAnyMethods>::extract")] _data2: Vec<i32>,
+        #[pyo3(from_py_with = <Bound<'_, _> as PyAnyMethods>::extract)] _data2: Vec<i32>,
     ) {
     }
 }

tests/test_pyfunction.rs

@@ -143,7 +143,7 @@ fn datetime_to_timestamp(dt: &Bound<'_, PyAny>) -> PyResult<i64> {
 #[cfg(not(Py_LIMITED_API))]
 #[pyfunction]
 fn function_with_custom_conversion(
-    #[pyo3(from_py_with = "datetime_to_timestamp")] timestamp: i64,
+    #[pyo3(from_py_with = datetime_to_timestamp)] timestamp: i64,
 ) -> i64 {
     timestamp
 }
@@ -196,13 +196,13 @@ fn test_from_py_with_defaults() {
     // issue 2280 combination of from_py_with and Option<T> did not compile
     #[pyfunction]
     #[pyo3(signature = (int=None))]
-    fn from_py_with_option(#[pyo3(from_py_with = "optional_int")] int: Option<i32>) -> i32 {
+    fn from_py_with_option(#[pyo3(from_py_with = optional_int)] int: Option<i32>) -> i32 {
         int.unwrap_or(0)
     }
 
     #[pyfunction(signature = (len=0))]
     fn from_py_with_default(
-        #[pyo3(from_py_with = "<Bound<'_, _> as PyAnyMethods>::len")] len: usize,
+        #[pyo3(from_py_with = <Bound<'_, _> as PyAnyMethods>::len)] len: usize,
     ) -> usize {
         len
     }