Merge pull request #1077 from godot-rust/qol/register-docs-cleanup

XML doc generation: code cleanup

Author: Bromeon

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

@@ -94,7 +94,7 @@ pub fn transform_inherent_impl(
     let consts = process_godot_constants(&mut impl_block)?;
 
     #[cfg(all(feature = "register-docs", since_api = "4.3"))]
-    let docs = crate::docs::make_inherent_impl_docs(&funcs, &consts, &signals);
+    let docs = crate::docs::document_inherent_impl(&funcs, &consts, &signals);
     #[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
     let docs = quote! {};
 

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

@@ -34,7 +34,7 @@ pub fn transform_trait_impl(original_impl: venial::Impl) -> ParseResult<TokenStr
     let prv = quote! { ::godot::private };
 
     #[cfg(all(feature = "register-docs", since_api = "4.3"))]
-    let docs = crate::docs::make_virtual_impl_docs(&original_impl.body_items);
+    let docs = crate::docs::document_interface_trait_impl(&original_impl.body_items);
     #[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
     let docs = quote! {};
 

godot-macros/src/class/derive_godot_class.rs

@@ -65,11 +65,8 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
     }
     let base_ty = &struct_cfg.base_ty;
     #[cfg(all(feature = "register-docs", since_api = "4.3"))]
-    let docs = crate::docs::make_definition_docs(
-        base_ty.to_string(),
-        &class.attributes,
-        &fields.all_fields,
-    );
+    let docs =
+        crate::docs::document_struct(base_ty.to_string(), &class.attributes, &fields.all_fields);
     #[cfg(not(all(feature = "register-docs", since_api = "4.3")))]
     let docs = quote! {};
     let base_class = quote! { ::godot::classes::#base_ty };

godot-macros/src/docs.rs

@@ -10,22 +10,24 @@ mod markdown_converter;
 use crate::class::{ConstDefinition, Field, FuncDefinition, SignalDefinition};
 use proc_macro2::{Ident, TokenStream};
 use quote::{quote, ToTokens};
-use venial::*;
 
-pub fn make_definition_docs(
+/// Returns code containing the doc information of a `#[derive(GodotClass)] struct MyClass` declaration.
+pub fn document_struct(
     base: String,
-    description: &[Attribute],
-    members: &[Field],
+    description: &[venial::Attribute],
+    fields: &[Field],
 ) -> TokenStream {
     let base_escaped = xml_escape(base);
-    let Some(desc_escaped) = make_docs_from_attributes(description).map(xml_escape) else {
+    let Some(desc_escaped) = attribute_docs_to_bbcode(description).map(xml_escape) else {
         return quote! { None };
     };
-    let members = members
+
+    let members = fields
         .iter()
-        .filter(|x| x.var.is_some() | x.export.is_some())
-        .filter_map(member)
+        .filter(|field| field.var.is_some() || field.export.is_some())
+        .filter_map(format_member_xml)
         .collect::<String>();
+
     quote! {
         Some(
             ::godot::docs::StructDocs {
@@ -37,79 +39,71 @@ pub fn make_definition_docs(
     }
 }
 
-pub fn make_inherent_impl_docs(
+/// Returns code containing the doc information of a `#[godot_api] impl MyClass` declaration.
+pub fn document_inherent_impl(
     functions: &[FuncDefinition],
     constants: &[ConstDefinition],
     signals: &[SignalDefinition],
 ) -> TokenStream {
-    /// Generates TokenStream containing field definitions for documented methods and documentation blocks for constants and signals.
-    fn pieces(
-        functions: &[FuncDefinition],
-        signals: &[SignalDefinition],
-        constants: &[ConstDefinition],
-    ) -> TokenStream {
-        let to_tagged = |s: String, tag: &str| -> String {
-            if s.is_empty() {
-                s
-            } else {
-                format!("<{tag}>{s}</{tag}>")
-            }
-        };
-
-        let signals_block = to_tagged(
-            signals
-                .iter()
-                .filter_map(make_signal_docs)
-                .collect::<String>(),
-            "signals",
-        );
-        let constants_block = to_tagged(
-            constants
-                .iter()
-                .map(|ConstDefinition { raw_constant }| raw_constant)
-                .filter_map(make_constant_docs)
-                .collect::<String>(),
-            "constants",
-        );
-
-        let methods = functions
-            .iter()
-            .filter_map(make_method_docs)
-            .collect::<String>();
-
-        quote! {
-            ::godot::docs::InherentImplDocs {
-                methods: #methods,
-                signals_block: #signals_block,
-                constants_block: #constants_block,
-            }
+    let group_xml_block = |s: String, tag: &str| -> String {
+        if s.is_empty() {
+            s
+        } else {
+            format!("<{tag}>{s}</{tag}>")
+        }
+    };
+
+    let signal_xml_elems = signals
+        .iter()
+        .filter_map(format_signal_xml)
+        .collect::<String>();
+    let signals_block = group_xml_block(signal_xml_elems, "signals");
+
+    let constant_xml_elems = constants
+        .iter()
+        .map(|ConstDefinition { raw_constant }| raw_constant)
+        .filter_map(format_constant_xml)
+        .collect::<String>();
+    let constants_block = group_xml_block(constant_xml_elems, "constants");
+
+    let method_xml_elems = functions
+        .iter()
+        .filter_map(format_method_xml)
+        .collect::<String>();
+
+    quote! {
+        ::godot::docs::InherentImplDocs {
+            methods: #method_xml_elems,
+            signals_block: #signals_block,
+            constants_block: #constants_block,
         }
     }
-    pieces(functions, signals, constants)
 }
 
-pub fn make_virtual_impl_docs(vmethods: &[ImplMember]) -> TokenStream {
-    let virtual_methods = vmethods
+/// Returns code containing the doc information of a `#[godot_api] impl ITrait for MyClass` declaration.
+pub fn document_interface_trait_impl(impl_members: &[venial::ImplMember]) -> TokenStream {
+    let interface_methods = impl_members
         .iter()
         .filter_map(|x| match x {
             venial::ImplMember::AssocFunction(f) => Some(f.clone()),
             _ => None,
         })
-        .filter_map(make_virtual_method_docs)
+        .filter_map(format_virtual_method_xml)
         .collect::<String>();
 
-    quote! { #virtual_methods }
+    quote! { #interface_methods }
 }
 
 /// `///` is expanded to `#[doc = "…"]`.
-/// This function goes through and extracts the …
-fn siphon_docs_from_attributes(doc: &[Attribute]) -> impl Iterator<Item = String> + '_ {
+///
+/// This function goes through and extracts the "…" part.
+fn extract_docs_from_attributes(doc: &[venial::Attribute]) -> impl Iterator<Item = String> + '_ {
     doc.iter()
-        // find #[doc]
+        // Find #[doc].
         .filter(|x| x.get_single_path_segment().is_some_and(|x| x == "doc"))
-        // #[doc = "…"]
+        // Limit to occurrences with syntax #[doc = "…"].
         .filter_map(|x| match &x.value {
-            AttributeValue::Equals(_, doc) => Some(doc),
+            venial::AttributeValue::Equals(_, doc) => Some(doc),
             _ => None,
         })
         .flat_map(|doc| {
@@ -122,7 +116,7 @@ fn siphon_docs_from_attributes(doc: &[Attribute]) -> impl Iterator<Item = String
 }
 
 fn xml_escape(value: String) -> String {
-    // Most strings have no special characters, so this check helps avoid unnecessary string copying
+    // Most strings have no special characters, so this check helps avoid unnecessary string copying.
     if !value.contains(['&', '<', '>', '"', '\'']) {
         return value;
     }
@@ -143,29 +137,33 @@ fn xml_escape(value: String) -> String {
     result
 }
 
-/// Calls [`siphon_docs_from_attributes`] and converts the result to BBCode
-/// for Godot's consumption.
-fn make_docs_from_attributes(doc: &[Attribute]) -> Option<String> {
-    let doc = siphon_docs_from_attributes(doc)
+/// Calls [`extract_docs_from_attributes`] and converts the result to BBCode for Godot's consumption.
+fn attribute_docs_to_bbcode(doc: &[venial::Attribute]) -> Option<String> {
+    let doc = extract_docs_from_attributes(doc)
         .collect::<Vec<String>>()
         .join("\n");
 
     (!doc.is_empty()).then(|| markdown_converter::to_bbcode(&doc))
 }
 
-fn make_signal_docs(signal: &SignalDefinition) -> Option<String> {
+fn format_venial_params_xml(params: &venial::Punctuated<venial::FnParam>) -> String {
+    let non_receiver_params = params.iter().filter_map(|(param, _punct)| match param {
+        venial::FnParam::Receiver(_) => None,
+        venial::FnParam::Typed(p) => Some((&p.name, &p.ty)),
+    });
+
+    format_params_xml(non_receiver_params)
+}
+
+fn format_signal_xml(signal: &SignalDefinition) -> Option<String> {
     let name = &signal.fn_signature.name;
-    let params = params(
-        signal
-            .fn_signature
-            .params
-            .iter()
-            .filter_map(|(x, _)| match x {
-                FnParam::Receiver(_) => None,
-                FnParam::Typed(y) => Some((&y.name, &y.ty)),
-            }),
-    );
-    let desc = make_docs_from_attributes(&signal.external_attributes)?;
+    let name = xml_escape(name.to_string());
+
+    let params = format_venial_params_xml(&signal.fn_signature.params);
+
+    let desc = attribute_docs_to_bbcode(&signal.external_attributes)?;
+    let desc = xml_escape(desc);
+
     Some(format!(
         r#"
 <signal name="{name}">
@@ -174,14 +172,12 @@ fn make_signal_docs(signal: &SignalDefinition) -> Option<String> {
   {desc}
   </description>
 </signal>
-"#,
-        name = xml_escape(name.to_string()),
-        desc = xml_escape(desc),
+"#
     ))
 }
 
-fn make_constant_docs(constant: &Constant) -> Option<String> {
-    let docs = make_docs_from_attributes(&constant.attributes)?;
+fn format_constant_xml(constant: &venial::Constant) -> Option<String> {
+    let docs = attribute_docs_to_bbcode(&constant.attributes)?;
     let name = constant.name.to_string();
     let value = constant
         .initializer
@@ -197,11 +193,12 @@ fn make_constant_docs(constant: &Constant) -> Option<String> {
     ))
 }
 
-pub fn member(member: &Field) -> Option<String> {
-    let docs = make_docs_from_attributes(&member.attributes)?;
+pub fn format_member_xml(member: &Field) -> Option<String> {
+    let docs = attribute_docs_to_bbcode(&member.attributes)?;
     let name = &member.name;
     let ty = member.ty.to_token_stream().to_string();
     let default = member.default_val.to_token_stream().to_string();
+
     Some(format!(
         r#"<member name="{name}" type="{ty}" default="{default}">{docs}</member>"#,
         name = xml_escape(name.to_string()),
@@ -211,76 +208,79 @@ pub fn member(member: &Field) -> Option<String> {
     ))
 }
 
-fn params<'a, 'b>(params: impl Iterator<Item = (&'a Ident, &'b TypeExpr)>) -> String {
+fn format_params_xml<'a, 'b>(
+    params: impl Iterator<Item = (&'a Ident, &'b venial::TypeExpr)>,
+) -> String {
+    use std::fmt::Write;
+
     let mut output = String::new();
     for (index, (name, ty)) in params.enumerate() {
-        output.push_str(&format!(
+        write!(
+            output,
             r#"<param index="{index}" name="{name}" type="{ty}" />"#,
             name = xml_escape(name.to_string()),
             ty = xml_escape(ty.to_token_stream().to_string()),
-        ));
+        )
+        .expect("write to string failed");
     }
     output
 }
 
-pub fn make_virtual_method_docs(method: Function) -> Option<String> {
-    let desc = make_docs_from_attributes(&method.attributes)?;
+fn format_virtual_method_xml(method: venial::Function) -> Option<String> {
+    let desc = attribute_docs_to_bbcode(&method.attributes)?;
+    let desc = xml_escape(desc);
+
     let name = method.name.to_string();
-    let ret = method
+    let name = xml_escape(name);
+
+    let return_ty = method
         .return_ty
-        .map(|x| x.to_token_stream().to_string())
+        .map(|ty| ty.to_token_stream().to_string())
         .unwrap_or_else(|| "void".to_string());
+    let return_ty = xml_escape(return_ty);
+
+    let params = format_venial_params_xml(&method.params);
 
-    let params = params(method.params.iter().filter_map(|(x, _)| match x {
-        FnParam::Receiver(_) => None,
-        FnParam::Typed(y) => Some((&y.name, &y.ty)),
-    }));
     Some(format!(
         r#"
 <method name="_{name}">
-  <return type="{ret}" />
+  <return type="{return_ty}" />
   {params}
   <description>
   {desc}
   </description>
 </method>
-"#,
-        name = xml_escape(name),
-        ret = xml_escape(ret),
-        desc = xml_escape(desc),
+"#
     ))
 }
 
-pub fn make_method_docs(method: &FuncDefinition) -> Option<String> {
-    let desc = make_docs_from_attributes(&method.external_attributes)?;
+fn format_method_xml(method: &FuncDefinition) -> Option<String> {
+    let desc = attribute_docs_to_bbcode(&method.external_attributes)?;
+    let desc = xml_escape(desc);
+
     let name = method
         .registered_name
         .clone()
         .unwrap_or_else(|| method.rust_ident().to_string());
-    let ret = method
-        .signature_info
-        .return_type
-        .to_token_stream()
-        .to_string();
-    let params = params(
-        method
-            .signature_info
-            .param_idents
-            .iter()
-            .zip(&method.signature_info.param_types),
-    );
+    let name = xml_escape(name);
+
+    let signature = &method.signature_info;
+
+    let return_ty = signature.return_type.to_token_stream().to_string();
+    let return_ty = xml_escape(return_ty);
+
+    let param_names_and_types = signature.param_idents.iter().zip(&signature.param_types);
+    let params = format_params_xml(param_names_and_types);
+
     Some(format!(
         r#"
 <method name="{name}">
-  <return type="{ret}" />
+  <return type="{return_ty}" />
   {params}
   <description>
   {desc}
   </description>
 </method>
-"#,
-        name = xml_escape(name),
-        ret = xml_escape(ret),
-        desc = xml_escape(desc),
+"#
     ))
 }