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

@@ -488,9 +488,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.
@@ -507,7 +507,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

@@ -2,7 +2,7 @@ use crate::attributes::{
     self, get_pyo3_options, CrateAttribute, DefaultAttribute, FromPyWithAttribute,
     RenameAllAttribute, RenamingRule,
 };
-use crate::utils::{self, Ctx};
+use crate::utils::{self, deprecated_from_py_with, Ctx};
 use proc_macro2::TokenStream;
 use quote::{format_ident, quote, quote_spanned, ToTokens};
 use syn::{
@@ -304,10 +304,13 @@ impl<'a> Container<'a> {
                 value: expr_path,
             }) = from_py_with
             {
+                let deprecation = deprecated_from_py_with(expr_path).unwrap_or_default();
+
                 let extractor = quote_spanned! { kw.span =>
                     { let from_py_with: fn(_) -> _ = #expr_path; from_py_with }
                 };
                 quote! {
+                    #deprecation
                     Ok(#self_ty {
                         #ident: #pyo3_path::impl_::frompyobject::extract_struct_field_with(#extractor, obj, #struct_name, #field_name)?
                     })
@@ -324,10 +327,13 @@ impl<'a> Container<'a> {
             value: expr_path,
         }) = from_py_with
         {
+            let deprecation = deprecated_from_py_with(expr_path).unwrap_or_default();
+
             let extractor = quote_spanned! { kw.span =>
                 { let from_py_with: fn(_) -> _ = #expr_path; from_py_with }
             };
             quote! {
+                #deprecation
                 #pyo3_path::impl_::frompyobject::extract_tuple_struct_field_with(#extractor, obj, #struct_name, 0).map(#self_ty)
             }
         } else {
@@ -361,7 +367,14 @@ impl<'a> Container<'a> {
             }}
         });
 
+        let deprecations = struct_fields
+            .iter()
+            .filter_map(|fields| fields.from_py_with.as_ref())
+            .filter_map(|kw| deprecated_from_py_with(&kw.value))
+            .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),
@@ -435,7 +448,13 @@ 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_map(|kw| deprecated_from_py_with(&kw.value))
+            .collect::<TokenStream>();
+
+        quote!(#d ::std::result::Result::Ok(#self_ty{#fields}))
     }
 }
 

pyo3-macros-backend/src/params.rs

@@ -1,4 +1,4 @@
-use crate::utils::Ctx;
+use crate::utils::{deprecated_from_py_with, Ctx};
 use crate::{
     attributes::FromPyWithAttribute,
     method::{FnArg, FnSpec, RegularArg},
@@ -62,7 +62,9 @@ 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 = deprecated_from_py_with(from_py_with).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

@@ -4,7 +4,7 @@ use std::ffi::CString;
 use crate::attributes::{FromPyWithAttribute, NameAttribute, RenamingRule};
 use crate::method::{CallingConvention, ExtractErrorMode, PyArg};
 use crate::params::{impl_regular_arg_param, Holders};
-use crate::utils::PythonDoc;
+use crate::utils::{deprecated_from_py_with, PythonDoc};
 use crate::utils::{Ctx, LitCStr};
 use crate::{
     method::{FnArg, FnSpec, FnType, SelfType},
@@ -660,8 +660,10 @@ pub fn impl_py_setter_def(
             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 = deprecated_from_py_with(from_py_with).unwrap_or_default();
                     (
                         quote_spanned! { from_py_with.span() =>
+                            #d
                             let #ident = #from_py_with;
                         },
                         ident,

pyo3-macros-backend/src/utils.rs

@@ -1,6 +1,6 @@
-use crate::attributes::{CrateAttribute, RenamingRule};
+use crate::attributes::{CrateAttribute, ExprPathWrap, RenamingRule};
 use proc_macro2::{Span, TokenStream};
-use quote::{quote, ToTokens};
+use quote::{quote, quote_spanned, ToTokens};
 use std::ffi::CString;
 use syn::spanned::Spanned;
 use syn::{punctuated::Punctuated, Token};
@@ -323,3 +323,17 @@ pub(crate) fn has_attribute_with_namespace(
             .eq(attr.path().segments.iter().map(|v| &v.ident))
     })
 }
+
+pub(crate) fn deprecated_from_py_with(expr_path: &ExprPathWrap) -> Option<TokenStream> {
+    let path = quote!(#expr_path).to_string();
+    let msg =
+        format!("remove the quotes from the literal\n= help: use `{path}` instead of `\"{path}\"`");
+    expr_path.from_lit_str.then(|| {
+        quote_spanned! { expr_path.span() =>
+            #[deprecated(since = "0.24.0", note = #msg)]
+            #[allow(dead_code)]
+            const LIT_STR_DEPRECATION: () = ();
+            let _: () = LIT_STR_DEPRECATION;
+        }
+    })
+}

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

@@ -70,4 +70,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

@@ -628,7 +628,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,
 }
 
@@ -653,7 +653,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]
@@ -693,10 +693,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,
     ),
 }
 
@@ -717,7 +717,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,
 }
 
@@ -815,7 +815,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
     }