Merge pull request #1355 from Yarwin/fix-docs-in-godot-api-secondary

Class Docs – register docs in `#[godot_api(secondary)]`, simplify docs registration logic

Author: Bromeon

godot-core/src/docs.rs

@@ -8,7 +8,41 @@
 use std::collections::HashMap;
 
 use crate::meta::ClassId;
-use crate::registry::plugin::{ITraitImpl, InherentImpl, PluginItem, Struct};
+use crate::obj::GodotClass;
+
+/// Piece of information that is gathered by the self-registration ("plugin") system.
+///
+/// You should not manually construct this struct, but rather use [`DocsPlugin::new()`].
+#[derive(Debug)]
+pub struct DocsPlugin {
+    /// The name of the class to register docs for.
+    class_name: ClassId,
+
+    /// The actual item being registered.
+    item: DocsItem,
+}
+
+impl DocsPlugin {
+    /// Creates a new `DocsPlugin`, automatically setting the `class_name` to the values defined in [`GodotClass`].
+    pub fn new<T: GodotClass>(item: DocsItem) -> Self {
+        Self {
+            class_name: T::class_id(),
+            item,
+        }
+    }
+}
+
+type ITraitImplDocs = &'static str;
+
+#[derive(Debug)]
+pub enum DocsItem {
+    /// Docs for `#[derive(GodotClass)] struct MyClass`.
+    Struct(StructDocs),
+    /// Docs for `#[godot_api] impl MyClass`.
+    InherentImpl(InherentImplDocs),
+    /// Docs for `#[godot_api] impl ITrait for MyClass`.
+    ITraitImpl(ITraitImplDocs),
+}
 
 /// Created for documentation on
 /// ```ignore
@@ -29,7 +63,7 @@ pub struct StructDocs {
     pub members: &'static str,
 }
 
-/// Keeps documentation for inherent `impl` blocks, such as:
+/// Keeps documentation for inherent `impl` blocks (primary and secondary), such as:
 /// ```ignore
 /// #[godot_api]
 /// impl Struct {
@@ -46,18 +80,19 @@ pub struct StructDocs {
 /// }
 /// ```
 /// All fields are XML parts, escaped where necessary.
-#[derive(Default, Copy, Clone, Debug)]
+#[derive(Default, Clone, Debug)]
 pub struct InherentImplDocs {
     pub methods: &'static str,
-    pub signals_block: &'static str,
-    pub constants_block: &'static str,
+    pub signals: &'static str,
+    pub constants: &'static str,
 }
 
 #[derive(Default)]
 struct DocPieces {
     definition: StructDocs,
-    inherent: InherentImplDocs,
-    virtual_methods: &'static str,
+    methods: Vec<&'static str>,
+    signals: Vec<&'static str>,
+    constants: Vec<&'static str>,
 }
 
 /// This function scours the registered plugins to find their documentation pieces,
@@ -76,24 +111,27 @@ struct DocPieces {
 #[doc(hidden)]
 pub fn gather_xml_docs() -> impl Iterator<Item = String> {
     let mut map = HashMap::<ClassId, DocPieces>::new();
-    crate::private::iterate_plugins(|x| {
+    crate::private::iterate_docs_plugins(|x| {
         let class_name = x.class_name;
-
-        match x.item {
-            PluginItem::InherentImpl(InherentImpl { docs, .. }) => {
-                map.entry(class_name).or_default().inherent = docs
+        match &x.item {
+            DocsItem::Struct(s) => {
+                map.entry(class_name).or_default().definition = *s;
             }
-
-            PluginItem::ITraitImpl(ITraitImpl {
-                virtual_method_docs,
-                ..
-            }) => map.entry(class_name).or_default().virtual_methods = virtual_method_docs,
-
-            PluginItem::Struct(Struct { docs, .. }) => {
-                map.entry(class_name).or_default().definition = docs
+            DocsItem::InherentImpl(trait_docs) => {
+                let InherentImplDocs {
+                    methods,
+                    constants,
+                    signals,
+                } = trait_docs;
+                map.entry(class_name).or_default().methods.push(methods);
+                map.entry(class_name)
+                    .and_modify(|pieces| pieces.constants.push(constants));
+                map.entry(class_name)
+                    .and_modify(|pieces| pieces.signals.push(signals));
+            }
+            DocsItem::ITraitImpl(methods) => {
+                map.entry(class_name).or_default().methods.push(methods);
             }
-
-            _ => (),
         }
     });
 
@@ -106,22 +144,31 @@ pub fn gather_xml_docs() -> impl Iterator<Item = String> {
                 members,
             } = pieces.definition;
 
-            let InherentImplDocs {
-                methods,
-                signals_block,
-                constants_block,
-            } = pieces.inherent;
-
-            let virtual_methods = pieces.virtual_methods;
-            let methods_block = (virtual_methods.is_empty() && methods.is_empty())
-                .then(String::new)
-                .unwrap_or_else(|| format!("<methods>{methods}{virtual_methods}</methods>"));
 
+            let method_docs = String::from_iter(pieces.methods);
+            let signal_docs = String::from_iter(pieces.signals);
+            let constant_docs = String::from_iter(pieces.constants);
+
+            let methods_block = if method_docs.is_empty() {
+                String::new()
+            } else {
+                format!("<methods>{method_docs}</methods>")
+            };
+            let signals_block = if signal_docs.is_empty() {
+                String::new()
+            } else {
+                format!("<signals>{signal_docs}</signals>")
+            };
+            let constants_block = if constant_docs.is_empty() {
+                String::new()
+            } else {
+                format!("<constants>{constant_docs}</constants>")
+            };
             let (brief, description) = match description
                 .split_once("[br]") {
-                    Some((brief, description)) => (brief, description.trim_start_matches("[br]")),
-                    None => (description, ""),
-                };
+                Some((brief, description)) => (brief, description.trim_start_matches("[br]")),
+                None => (description, ""),
+            };
 
             format!(r#"<?xml version="1.0" encoding="UTF-8"?>
 <class name="{class}" inherits="{base}"{deprecated}{experimental} xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd">

godot-core/src/private.rs

@@ -22,6 +22,8 @@ use crate::{classes, sys};
 // Public re-exports
 
 mod reexport_pub {
+    #[cfg(all(since_api = "4.3", feature = "register-docs"))]
+    pub use crate::docs::{DocsItem, DocsPlugin, InherentImplDocs, StructDocs};
     pub use crate::gen::classes::class_macros;
     #[cfg(feature = "trace")]
     pub use crate::meta::trace;
@@ -52,6 +54,8 @@ static CALL_ERRORS: Global<CallErrors> = Global::default();
 static ERROR_PRINT_LEVEL: atomic::AtomicU8 = atomic::AtomicU8::new(2);
 
 sys::plugin_registry!(pub __GODOT_PLUGIN_REGISTRY: ClassPlugin);
+#[cfg(all(since_api = "4.3", feature = "register-docs"))]
+sys::plugin_registry!(pub __GODOT_DOCS_REGISTRY: DocsPlugin);
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Call error handling
@@ -146,6 +150,11 @@ pub(crate) fn iterate_plugins(mut visitor: impl FnMut(&ClassPlugin)) {
     sys::plugin_foreach!(__GODOT_PLUGIN_REGISTRY; visitor);
 }
 
+#[cfg(all(since_api = "4.3", feature = "register-docs"))]
+pub(crate) fn iterate_docs_plugins(mut visitor: impl FnMut(&DocsPlugin)) {
+    sys::plugin_foreach!(__GODOT_DOCS_REGISTRY; visitor);
+}
+
 #[cfg(feature = "codegen-full")] // Remove if used in other scenarios.
 pub(crate) fn find_inherent_impl(class_name: crate::meta::ClassId) -> Option<InherentImpl> {
     // We do this manually instead of using `iterate_plugins()` because we want to break as soon as we find a match.

godot-core/src/registry/class.rs

@@ -424,8 +424,6 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
             is_editor_plugin,
             is_internal,
             is_instantiable,
-            #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-                docs: _,
             reference_fn,
             unreference_fn,
         }) => {
@@ -473,8 +471,6 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
         PluginItem::InherentImpl(InherentImpl {
             register_methods_constants_fn,
             register_rpcs_fn: _,
-            #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-                docs: _,
         }) => {
             c.register_methods_constants_fn = Some(register_methods_constants_fn);
         }
@@ -492,8 +488,6 @@ fn fill_class_info(item: PluginItem, c: &mut ClassRegistrationInfo) {
             user_free_property_list_fn,
             user_property_can_revert_fn,
             user_property_get_revert_fn,
-            #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-                virtual_method_docs: _,
             validate_property_fn,
         }) => {
             c.user_register_fn = user_register_fn;

godot-core/src/registry/plugin.rs

@@ -8,8 +8,6 @@
 use std::any::Any;
 use std::{any, fmt};
 
-#[cfg(all(since_api = "4.3", feature = "register-docs"))]
-use crate::docs::*;
 use crate::init::InitLevel;
 use crate::meta::ClassId;
 use crate::obj::{bounds, cap, Bounds, DynGd, Gd, GodotClass, Inherits, UserClass};
@@ -197,16 +195,10 @@ pub struct Struct {
 
     /// Whether the class has a default constructor.
     pub(crate) is_instantiable: bool,
-
-    /// Documentation extracted from the struct's RustDoc.
-    #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-    pub(crate) docs: StructDocs,
 }
 
 impl Struct {
-    pub fn new<T: GodotClass + cap::ImplementsGodotExports>(
-        #[cfg(all(since_api = "4.3", feature = "register-docs"))] docs: StructDocs,
-    ) -> Self {
+    pub fn new<T: GodotClass + cap::ImplementsGodotExports>() -> Self {
         let refcounted = <T::Memory as bounds::Memory>::IS_REF_COUNTED;
 
         Self {
@@ -222,8 +214,6 @@ impl Struct {
             is_editor_plugin: false,
             is_internal: false,
             is_instantiable: false,
-            #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-            docs,
             // While Godot doesn't do anything with these callbacks for non-RefCounted classes, we can avoid instantiating them in Rust.
             reference_fn: refcounted.then_some(callbacks::reference::<T>),
             unreference_fn: refcounted.then_some(callbacks::unreference::<T>),
@@ -294,34 +284,23 @@ pub struct InherentImpl {
     // This field is only used during codegen-full.
     #[cfg_attr(not(feature = "codegen-full"), expect(dead_code))]
     pub(crate) register_rpcs_fn: Option<ErasedRegisterRpcsFn>,
-
-    #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-    pub docs: InherentImplDocs,
 }
 
 impl InherentImpl {
-    pub fn new<T: cap::ImplementsGodotApi>(
-        #[cfg(all(since_api = "4.3", feature = "register-docs"))] docs: InherentImplDocs,
-    ) -> Self {
+    pub fn new<T: cap::ImplementsGodotApi>() -> Self {
         Self {
             register_methods_constants_fn: ErasedRegisterFn {
                 raw: callbacks::register_user_methods_constants::<T>,
             },
             register_rpcs_fn: Some(ErasedRegisterRpcsFn {
                 raw: callbacks::register_user_rpcs::<T>,
             }),
-            #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-            docs,
         }
     }
 }
 
 #[derive(Default, Clone, Debug)]
 pub struct ITraitImpl {
-    #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-    /// Virtual method documentation.
-    pub(crate) virtual_method_docs: &'static str,
-
     /// Callback to user-defined `register_class` function.
     pub(crate) user_register_fn: Option<ErasedRegisterFn>,
 
@@ -436,12 +415,8 @@ pub struct ITraitImpl {
 }
 
 impl ITraitImpl {
-    pub fn new<T: GodotClass + cap::ImplementsGodotVirtual>(
-        #[cfg(all(since_api = "4.3", feature = "register-docs"))] virtual_method_docs: &'static str,
-    ) -> Self {
+    pub fn new<T: GodotClass + cap::ImplementsGodotVirtual>() -> Self {
         Self {
-            #[cfg(all(since_api = "4.3", feature = "register-docs"))]
-            virtual_method_docs,
             get_virtual_fn: Some(callbacks::get_virtual::<T>),
             ..Default::default()
         }

godot-macros/src/class/data_models/inherent_impl.rs

@@ -91,10 +91,8 @@ pub fn transform_inherent_impl(
     let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block, meta.secondary)?;
     let consts = process_godot_constants(&mut impl_block)?;
 
-    #[cfg(all(feature = "register-docs", since_api = "4.3"))]
-    let docs = crate::docs::document_inherent_impl(&funcs, &consts, &signals);
-    #[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
-    let docs = quote! {};
+    let inherent_impl_docs =
+        crate::docs::make_trait_docs_registration(&funcs, &consts, &signals, &class_name, &prv);
 
     // Container struct holding names of all registered #[func]s.
     // The struct is declared by #[derive(GodotClass)].
@@ -127,17 +125,20 @@ pub fn transform_inherent_impl(
     let method_storage_name = format_ident!("__registration_methods_{class_name}");
     let constants_storage_name = format_ident!("__registration_constants_{class_name}");
 
-    let fill_storage = quote! {
-        ::godot::sys::plugin_execute_pre_main!({
-            #method_storage_name.lock().unwrap().push(|| {
-                #( #method_registrations )*
-                #( #signal_registrations )*
-            });
+    let fill_storage = {
+        quote! {
+            ::godot::sys::plugin_execute_pre_main!({
+                #method_storage_name.lock().unwrap().push(|| {
+                    #( #method_registrations )*
+                    #( #signal_registrations )*
+                });
+
+                #constants_storage_name.lock().unwrap().push(|| {
+                    #constant_registration
+                });
 
-            #constants_storage_name.lock().unwrap().push(|| {
-                #constant_registration
             });
-        });
+        }
     };
 
     if !meta.secondary {
@@ -175,7 +176,7 @@ pub fn transform_inherent_impl(
 
         let class_registration = quote! {
             ::godot::sys::plugin_add!(#prv::__GODOT_PLUGIN_REGISTRY; #prv::ClassPlugin::new::<#class_name>(
-                #prv::PluginItem::InherentImpl(#prv::InherentImpl::new::<#class_name>(#docs))
+                #prv::PluginItem::InherentImpl(#prv::InherentImpl::new::<#class_name>())
             ));
         };
 
@@ -189,6 +190,7 @@ pub fn transform_inherent_impl(
                 #( #func_name_constants )*
             }
             #signal_symbol_types
+            #inherent_impl_docs
         };
 
         Ok(result)
@@ -202,6 +204,7 @@ pub fn transform_inherent_impl(
             impl #funcs_collection {
                 #( #func_name_constants )*
             }
+            #inherent_impl_docs
         };
 
         Ok(result)