Embedded doc mode using HTML comments to fix attribute order issues (fixes #5269)

newsfragments/5288.added.md

@@ -0,0 +1 @@
+Embedded doc mode using HTML comments to fix attribute order issues.

pyo3-macros-backend/src/attributes.rs

@@ -18,8 +18,10 @@ pub mod kw {
     syn::custom_keyword!(cancel_handle);
     syn::custom_keyword!(constructor);
     syn::custom_keyword!(dict);
+    syn::custom_keyword!(doc_mode);
     syn::custom_keyword!(eq);
     syn::custom_keyword!(eq_int);
+    syn::custom_keyword!(end_doc_mode);
     syn::custom_keyword!(extends);
     syn::custom_keyword!(freelist);
     syn::custom_keyword!(from_py_with);

pyo3-macros-backend/src/method.rs

@@ -976,7 +976,7 @@ impl<'a> FnSpec<'a> {
     }
 
     /// Forwards to [utils::get_doc] with the text signature of this spec.
-    pub fn get_doc(&self, attrs: &[syn::Attribute], ctx: &Ctx) -> syn::Result<PythonDoc> {
+    pub fn get_doc(&self, attrs: &mut Vec<syn::Attribute>, ctx: &Ctx) -> syn::Result<PythonDoc> {
         let text_signature = self
             .text_signature_call_signature()
             .map(|sig| format!("{}{}", self.python_name, sig));

pyo3-macros-backend/src/module.rs

@@ -453,7 +453,7 @@ pub fn pymodule_function_impl(
         .name
         .map_or_else(|| ident.unraw(), |name| name.value.0);
     let vis = &function.vis;
-    let doc = get_doc(&function.attrs, None, ctx)?;
+    let doc = get_doc(&mut function.attrs, None, ctx)?;
 
     let initialization = module_initialization(
         &name,

pyo3-macros-backend/src/pyclass.rs

@@ -250,7 +250,7 @@ pub fn build_py_class(
     args.options.take_pyo3_options(&mut class.attrs)?;
 
     let ctx = &Ctx::new(&args.options.krate, None);
-    let doc = utils::get_doc(&class.attrs, None, ctx)?;
+    let doc = utils::get_doc(&mut class.attrs, None, ctx)?;
 
     if let Some(lt) = class.generics.lifetimes().next() {
         bail_spanned!(
@@ -446,11 +446,11 @@ fn impl_class(
         ctx,
     )?;
 
-    let (default_class_geitem, default_class_geitem_method) =
-        pyclass_class_geitem(&args.options, &syn::parse_quote!(#cls), ctx)?;
+    let (default_class_getitem, default_class_getitem_method) =
+        pyclass_class_getitem(&args.options, &syn::parse_quote!(#cls), ctx)?;
 
-    if let Some(default_class_geitem_method) = default_class_geitem_method {
-        default_methods.push(default_class_geitem_method);
+    if let Some(default_class_getitem_method) = default_class_getitem_method {
+        default_methods.push(default_class_getitem_method);
     }
 
     let (default_str, default_str_slot) =
@@ -484,7 +484,7 @@ fn impl_class(
             #default_richcmp
             #default_hash
             #default_str
-            #default_class_geitem
+            #default_class_getitem
         }
     })
 }
@@ -531,7 +531,7 @@ pub fn build_py_enum(
         bail_spanned!(generic.span() => "enums do not support #[pyclass(generic)]");
     }
 
-    let doc = utils::get_doc(&enum_.attrs, None, ctx)?;
+    let doc = utils::get_doc(&mut enum_.attrs, None, ctx)?;
     let enum_ = PyClassEnum::new(enum_)?;
     impl_enum(enum_, &args, doc, method_type, ctx)
 }
@@ -1768,7 +1768,7 @@ fn complex_enum_variant_field_getter<'a>(
     let property_type = crate::pymethod::PropertyType::Function {
         self_type: &self_type,
         spec: &spec,
-        doc: crate::get_doc(&[], None, ctx)?,
+        doc: PythonDoc::empty(ctx),
     };
 
     let getter = crate::pymethod::impl_py_getter_def(variant_cls_type, property_type, ctx)?;
@@ -2036,7 +2036,7 @@ fn pyclass_hash(
     }
 }
 
-fn pyclass_class_geitem(
+fn pyclass_class_getitem(
     options: &PyClassPyO3Options,
     cls: &syn::Type,
     ctx: &Ctx,
@@ -2045,7 +2045,7 @@ fn pyclass_class_geitem(
     match options.generic {
         Some(_) => {
             let ident = format_ident!("__class_getitem__");
-            let mut class_geitem_impl: syn::ImplItemFn = {
+            let mut class_getitem_impl: syn::ImplItemFn = {
                 parse_quote! {
                     #[classmethod]
                     fn #ident<'py>(
@@ -2058,19 +2058,19 @@ fn pyclass_class_geitem(
             };
 
             let spec = FnSpec::parse(
-                &mut class_geitem_impl.sig,
-                &mut class_geitem_impl.attrs,
+                &mut class_getitem_impl.sig,
+                &mut class_getitem_impl.attrs,
                 Default::default(),
             )?;
 
-            let class_geitem_method = crate::pymethod::impl_py_method_def(
+            let class_getitem_method = crate::pymethod::impl_py_method_def(
                 cls,
                 &spec,
-                &spec.get_doc(&class_geitem_impl.attrs, ctx)?,
+                &spec.get_doc(&mut class_getitem_impl.attrs, ctx)?,
                 Some(quote!(#pyo3_path::ffi::METH_CLASS)),
                 ctx,
             )?;
-            Ok((Some(class_geitem_impl), Some(class_geitem_method)))
+            Ok((Some(class_getitem_impl), Some(class_getitem_method)))
         }
         None => Ok((None, None)),
     }

pyo3-macros-backend/src/pyfunction.rs

@@ -425,7 +425,7 @@ pub fn impl_wrap_pyfunction(
         );
     }
     let wrapper = spec.get_wrapper_function(&wrapper_ident, None, ctx)?;
-    let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&func.attrs, ctx)?, ctx);
+    let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&mut func.attrs, ctx)?, ctx);
 
     let wrapped_pyfunction = quote! {
         // Create a module with the same name as the `#[pyfunction]` - this way `use <the function>`

pyo3-macros-backend/src/pyimpl.rs

@@ -139,7 +139,7 @@ pub fn impl_methods(
                     let method = PyMethod::parse(&mut meth.sig, &mut meth.attrs, fun_options)?;
                     #[cfg(feature = "experimental-inspect")]
                     extra_fragments.push(method_introspection_code(&method.spec, ty, ctx));
-                    match pymethod::gen_py_method(ty, method, &meth.attrs, ctx)? {
+                    match pymethod::gen_py_method(ty, method, &mut meth.attrs, ctx)? {
                         GeneratedPyMethod::Method(MethodAndMethodDef {
                             associated_method,
                             method_def,

pyo3-macros-backend/src/pymethod.rs

@@ -191,7 +191,7 @@ pub fn is_proto_method(name: &str) -> bool {
 pub fn gen_py_method(
     cls: &syn::Type,
     method: PyMethod<'_>,
-    meth_attrs: &[syn::Attribute],
+    meth_attrs: &mut Vec<syn::Attribute>,
     ctx: &Ctx,
 ) -> Result<GeneratedPyMethod> {
     let spec = &method.spec;
@@ -233,50 +233,59 @@ pub fn gen_py_method(
             }
         }
         // ordinary functions (with some specialties)
-        (_, FnType::Fn(_)) => GeneratedPyMethod::Method(impl_py_method_def(
-            cls,
-            spec,
-            &spec.get_doc(meth_attrs, ctx)?,
-            None,
-            ctx,
-        )?),
-        (_, FnType::FnClass(_)) => GeneratedPyMethod::Method(impl_py_method_def(
-            cls,
-            spec,
-            &spec.get_doc(meth_attrs, ctx)?,
-            Some(quote!(#pyo3_path::ffi::METH_CLASS)),
-            ctx,
-        )?),
-        (_, FnType::FnStatic) => GeneratedPyMethod::Method(impl_py_method_def(
-            cls,
-            spec,
-            &spec.get_doc(meth_attrs, ctx)?,
-            Some(quote!(#pyo3_path::ffi::METH_STATIC)),
-            ctx,
-        )?),
+        (_, FnType::Fn(_)) => {
+            let doc = spec.get_doc(meth_attrs, ctx)?;
+            GeneratedPyMethod::Method(impl_py_method_def(cls, spec, &doc, None, ctx)?)
+        }
+        (_, FnType::FnClass(_)) => {
+            let doc = spec.get_doc(meth_attrs, ctx)?;
+            GeneratedPyMethod::Method(impl_py_method_def(
+                cls,
+                spec,
+                &doc,
+                Some(quote!(#pyo3_path::ffi::METH_CLASS)),
+                ctx,
+            )?)
+        }
+        (_, FnType::FnStatic) => {
+            let doc = spec.get_doc(meth_attrs, ctx)?;
+            GeneratedPyMethod::Method(impl_py_method_def(
+                cls,
+                spec,
+                &doc,
+                Some(quote!(#pyo3_path::ffi::METH_STATIC)),
+                ctx,
+            )?)
+        }
         // special prototypes
         (_, FnType::FnNew) | (_, FnType::FnNewClass(_)) => {
             GeneratedPyMethod::Proto(impl_py_method_def_new(cls, spec, ctx)?)
         }
 
-        (_, FnType::Getter(self_type)) => GeneratedPyMethod::Method(impl_py_getter_def(
-            cls,
-            PropertyType::Function {
-                self_type,
-                spec,
-                doc: spec.get_doc(meth_attrs, ctx)?,
-            },
-            ctx,
-        )?),
-        (_, FnType::Setter(self_type)) => GeneratedPyMethod::Method(impl_py_setter_def(
-            cls,
-            PropertyType::Function {
-                self_type,
-                spec,
-                doc: spec.get_doc(meth_attrs, ctx)?,
-            },
-            ctx,
-        )?),
+        (_, FnType::Getter(self_type)) => {
+            let doc = spec.get_doc(meth_attrs, ctx)?;
+            GeneratedPyMethod::Method(impl_py_getter_def(
+                cls,
+                PropertyType::Function {
+                    self_type,
+                    spec,
+                    doc,
+                },
+                ctx,
+            )?)
+        }
+        (_, FnType::Setter(self_type)) => {
+            let doc = spec.get_doc(meth_attrs, ctx)?;
+            GeneratedPyMethod::Method(impl_py_setter_def(
+                cls,
+                PropertyType::Function {
+                    self_type,
+                    spec,
+                    doc,
+                },
+                ctx,
+            )?)
+        }
         (_, FnType::FnModule(_)) => {
             unreachable!("methods cannot be FnModule")
         }
@@ -978,12 +987,16 @@ impl PropertyType<'_> {
     }
 
     fn doc(&self, ctx: &Ctx) -> Result<Cow<'_, PythonDoc>> {
-        match self {
+        let doc = match self {
             PropertyType::Descriptor { field, .. } => {
-                utils::get_doc(&field.attrs, None, ctx).map(Cow::Owned)
+                // FIXME: due to the clone this will not properly strip Rust documentation, maybe
+                // need to parse the field and doc earlier in the process?
+                let mut attrs = field.attrs.clone();
+                Cow::Owned(utils::get_doc(&mut attrs, None, ctx)?)
             }
-            PropertyType::Function { doc, .. } => Ok(Cow::Borrowed(doc)),
-        }
+            PropertyType::Function { doc, .. } => Cow::Borrowed(doc),
+        };
+        Ok(doc)
     }
 }