Merge pull request #861 from andreymal/docs-valid-html

Generate valid XML from doc comments

Author: Bromeon

godot-core/src/docs.rs

@@ -18,6 +18,7 @@ use std::collections::HashMap;
 ///    x: f32,
 /// }
 /// ```
+/// All fields are XML parts, escaped where necessary.
 #[derive(Clone, Copy, Debug, Default)]
 pub struct StructDocs {
     pub base: &'static str,
@@ -41,6 +42,7 @@ pub struct StructDocs {
 ///
 /// }
 /// ```
+/// All fields are XML parts, escaped where necessary.
 #[derive(Clone, Copy, Debug, Default)]
 pub struct InherentImplDocs {
     pub methods: &'static str,
@@ -114,8 +116,7 @@ pub fn gather_xml_docs() -> impl Iterator<Item = String> {
                 .map(|(x, _)| x)
                 .unwrap_or_default();
 
-            format!(r#"
-<?xml version="1.0" encoding="UTF-8"?>
+            format!(r#"<?xml version="1.0" encoding="UTF-8"?>
 <class name="{class}" inherits="{base}" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd">
 <brief_description>{brief}</brief_description>
 <description>{description}</description>

godot-macros/src/docs.rs

@@ -17,16 +17,17 @@ pub fn make_definition_docs(
     members: &[Field],
 ) -> TokenStream {
     (|| {
-        let desc = make_docs_from_attributes(description)?;
+        let base_escaped = xml_escape(base);
+        let desc_escaped = xml_escape(make_docs_from_attributes(description)?);
         let members = members
             .into_iter()
             .filter(|x| x.var.is_some() | x.export.is_some())
             .filter_map(member)
             .collect::<String>();
         Some(quote! {
             docs: ::godot::docs::StructDocs {
-                base: #base,
-                description: #desc,
+                base: #base_escaped,
+                description: #desc_escaped,
                 members: #members,
             }.into()
         })
@@ -122,6 +123,28 @@ 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
+    if !value.contains(&['&', '<', '>', '"', '\'']) {
+        return value;
+    }
+
+    let mut result = String::with_capacity(value.len());
+
+    for c in value.chars() {
+        match c {
+            '&' => result.push_str("&amp;"),
+            '<' => result.push_str("&lt;"),
+            '>' => result.push_str("&gt;"),
+            '"' => result.push_str("&quot;"),
+            '\'' => result.push_str("&#39;"),
+            c => result.push(c),
+        }
+    }
+
+    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> {
@@ -146,7 +169,9 @@ fn make_signal_docs(signal: &SignalDefinition) -> Option<String> {
   {desc}
   </description>
 </signal>
-"#
+"#,
+        name = xml_escape(name.to_string()),
+        desc = xml_escape(desc),
     ))
 }
 
@@ -159,7 +184,10 @@ fn make_constant_docs(constant: &Constant) -> Option<String> {
         .map(|x| x.to_token_stream().to_string())
         .unwrap_or("null".into());
     Some(format!(
-        r#"<constant name="{name}" value="{value}">{docs}</constant>"#
+        r#"<constant name="{name}" value="{value}">{docs}</constant>"#,
+        name = xml_escape(name),
+        value = xml_escape(value),
+        docs = xml_escape(docs),
     ))
 }
 
@@ -169,7 +197,11 @@ pub fn member(member: &Field) -> Option<String> {
     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>"#
+        r#"<member name="{name}" type="{ty}" default="{default}">{docs}</member>"#,
+        name = xml_escape(name.to_string()),
+        ty = xml_escape(ty),
+        default = xml_escape(default),
+        docs = xml_escape(docs),
     ))
 }
 
@@ -178,7 +210,8 @@ fn params<'a, 'b>(params: impl Iterator<Item = (&'a Ident, &'b TypeExpr)>) -> St
     for (index, (name, ty)) in params.enumerate() {
         output.push_str(&format!(
             r#"<param index="{index}" name="{name}" type="{ty}" />"#,
-            ty = ty.to_token_stream()
+            name = xml_escape(name.to_string()),
+            ty = xml_escape(ty.to_token_stream().to_string()),
         ));
     }
     output
@@ -204,7 +237,10 @@ pub fn make_virtual_method_docs(method: Function) -> Option<String> {
   {desc}
   </description>
 </method>
-"#
+"#,
+        name = xml_escape(name),
+        ret = xml_escape(ret),
+        desc = xml_escape(desc),
     ))
 }
 
@@ -214,7 +250,7 @@ pub fn make_method_docs(method: &FuncDefinition) -> Option<String> {
         .rename
         .clone()
         .unwrap_or_else(|| method.signature_info.method_name.to_string());
-    let ret = method.signature_info.ret_type.to_token_stream();
+    let ret = method.signature_info.ret_type.to_token_stream().to_string();
     let params = params(
         method
             .signature_info
@@ -231,6 +267,9 @@ pub fn make_method_docs(method: &FuncDefinition) -> Option<String> {
   {desc}
   </description>
 </method>
-"#
+"#,
+        name = xml_escape(name),
+        ret = xml_escape(ret),
+        desc = xml_escape(desc),
     ))
 }

godot-macros/src/docs/markdown_converter.rs

@@ -62,6 +62,7 @@ fn walk_node(node: &Node, definitions: &HashMap<&str, &str>) -> Option<String> {
         List(_) | BlockQuote(_) | FootnoteReference(_) | FootnoteDefinition(_) | Table(_) => {
             "".into()
         }
+        Html(html) => html.value.clone(),
         _ => walk_nodes(&node.children()?, definitions, ""),
     };
     Some(bbcode)

godot/tests/docs.rs

@@ -7,7 +7,7 @@
  */
 use godot::prelude::*;
 
-/// *documented* ~ **documented** ~ [AABB] [pr](https://github.com/godot-rust/gdext/pull/748)
+/// *documented* ~ **documented** ~ [AABB] < [pr](https://github.com/godot-rust/gdext/pull/748)
 ///
 /// a few tests:
 ///
@@ -72,6 +72,22 @@ use godot::prelude::*;
 /// static main: u64 = 0x31c0678b10;
 /// ```
 ///
+/// Some HTML to make sure it's properly escaped:
+///
+/// <br/> <- this is inline HTML
+///
+/// &lt;br/&gt; <- not considered HTML (manually escaped)
+///
+/// `inline<br/>code`
+///
+/// ```html
+/// <div>
+///   code&nbsp;block
+/// </div>
+/// ```
+///
+/// [Google: 2 + 2 < 5](https://www.google.com/search?q=2+%2B+2+<+5)
+///
 /// connect
 /// these
 #[derive(GodotClass)]
@@ -83,6 +99,9 @@ pub struct FairlyDocumented {
     /// is it documented?
     #[var]
     item_2: i64,
+    #[var]
+    /// this docstring has < a special character
+    item_xml: GString,
     /// this isnt documented
     _other_item: (),
     /// nor this
@@ -97,6 +116,7 @@ impl INode for FairlyDocumented {
             base,
             item: 883.0,
             item_2: 25,
+            item_xml: "".into(),
             _other_item: {},
         }
     }
@@ -111,6 +131,10 @@ impl FairlyDocumented {
     #[constant]
     const PURPOSE: i64 = 42;
 
+    /// this docstring has < a special character
+    #[constant]
+    const XML: i64 = 1;
+
     #[func]
     fn totally_undocumented_function(&self) -> i64 {
         5
@@ -122,6 +146,12 @@ impl FairlyDocumented {
         self.item
     }
 
+    /// Function with lots of special characters (`Gd<Node>`)
+    #[func]
+    fn process_node(&self, node: Gd<Node>) -> Gd<Node> {
+        node
+    }
+
     #[func(gd_self, virtual)]
     fn virtual_undocumented(_s: Gd<Self>) {
         panic!("no implementation")
@@ -130,8 +160,10 @@ impl FairlyDocumented {
     /// some virtual function that should be overridden by a user
     ///
     /// some multiline doc
+    ///
+    /// The `Gd<Node>` param should be properly escaped
     #[func(gd_self, virtual)]
-    fn virtual_documented(_s: Gd<Self>) {
+    fn virtual_documented(_s: Gd<Self>, _node: Gd<Node>) {
         panic!("please provide user implementation")
     }
 
@@ -149,8 +181,10 @@ impl FairlyDocumented {
     /// some user signal
     ///
     /// some multiline doc
+    ///
+    /// The `Gd<Node>` param should be properly escaped
     #[signal]
-    fn documented_signal(p: Vector3, w: f64);
+    fn documented_signal(p: Vector3, w: f64, node: Gd<Node>);
 }
 
 #[test]

godot/tests/test_data/docs.xml

@@ -1,11 +1,12 @@
-
 <?xml version="1.0" encoding="UTF-8"?>
 <class name="FairlyDocumented" inherits="Node" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../class.xsd">
-<brief_description>[i]documented[/i] ~ [b]documented[/b] ~ [AABB] [url=https://github.com/godot-rust/gdext/pull/748]pr[/url]</brief_description>
-<description>[i]documented[/i] ~ [b]documented[/b] ~ [AABB] [url=https://github.com/godot-rust/gdext/pull/748]pr[/url][br][br]a few tests:[br][br]headings:[br][br]Some heading[br][br]lists:[br][br][br][br][br][br]links with back-references:[br][br]Blah blah [br][br][br][br]footnotes:[br][br]We cannot florbinate the glorb[br][br][br][br]task lists:[br][br]We must ensure that we've completed[br][br][br][br]tables:[br][br][br][br]images:[br][br][img]http://url/a.png[/img][br][br]blockquotes:[br][br][br][br]ordered list:[br][br][br][br]Something here < this is technically header syntax[br][br]And here[br][br]smart punctuation[br][br]codeblocks:[br][br][codeblock]#![no_main]
-#[link_section=\".text\"]
+<brief_description>[i]documented[/i] ~ [b]documented[/b] ~ [AABB] &lt; [url=https://github.com/godot-rust/gdext/pull/748]pr[/url]</brief_description>
+<description>[i]documented[/i] ~ [b]documented[/b] ~ [AABB] &lt; [url=https://github.com/godot-rust/gdext/pull/748]pr[/url][br][br]a few tests:[br][br]headings:[br][br]Some heading[br][br]lists:[br][br][br][br][br][br]links with back-references:[br][br]Blah blah [br][br][br][br]footnotes:[br][br]We cannot florbinate the glorb[br][br][br][br]task lists:[br][br]We must ensure that we&#39;ve completed[br][br][br][br]tables:[br][br][br][br]images:[br][br][img]http://url/a.png[/img][br][br]blockquotes:[br][br][br][br]ordered list:[br][br][br][br]Something here &lt; this is technically header syntax[br][br]And here[br][br]smart punctuation[br][br]codeblocks:[br][br][codeblock]#![no_main]
+#[link_section=\&quot;.text\&quot;]
 #[no_mangle]
-static main: u64 = 0x31c0678b10;[/codeblock][br][br]connect
+static main: u64 = 0x31c0678b10;[/codeblock][br][br]Some HTML to make sure it&#39;s properly escaped:[br][br]&lt;br/&gt; &lt;- this is inline HTML[br][br]&lt;br/&gt; &lt;- not considered HTML (manually escaped)[br][br][code]inline&lt;br/&gt;code[/code][br][br][codeblock]&lt;div&gt;
+  code&amp;nbsp;block
+&lt;/div&gt;[/codeblock][br][br][url=https://www.google.com/search?q=2+%2B+2+&lt;+5]Google: 2 + 2 &lt; 5[/url][br][br]connect
 these</description>
 <methods>
 <method name="ye">
@@ -16,16 +17,24 @@ these</description>
   </description>
 </method>
 
+<method name="process_node">
+  <return type="Gd &lt; Node &gt;" />
+  <param index="0" name="node" type="Gd &lt; Node &gt;" />
+  <description>
+  Function with lots of special characters ([code]Gd&lt;Node&gt;[/code])
+  </description>
+</method>
+
 <method name="virtual_documented">
   <return type="()" />
-  
+  <param index="0" name="node" type="Gd &lt; Node &gt;" />
   <description>
-  some virtual function that should be overridden by a user[br][br]some multiline doc
+  some virtual function that should be overridden by a user[br][br]some multiline doc[br][br]The [code]Gd&lt;Node&gt;[/code] param should be properly escaped
   </description>
 </method>
 
 <method name="ne">
-  <return type="Gd < FairlyDocumented >" />
+  <return type="Gd &lt; FairlyDocumented &gt;" />
   <param index="0" name="x" type="f32" />
   <description>
   wow[br][br]some multiline doc
@@ -34,20 +43,20 @@ these</description>
 
 <method name="_init">
   <return type="Self" />
-  <param index="0" name="base" type="Base < Node >" />
+  <param index="0" name="base" type="Base &lt; Node &gt;" />
   <description>
   initialize this
   </description>
 </method>
 </methods>
-<constants><constant name="RANDOM" value="4">Documentation.</constant></constants>
+<constants><constant name="RANDOM" value="4">Documentation.</constant><constant name="XML" value="1">this docstring has &lt; a special character</constant></constants>
 <signals>
 <signal name="documented_signal">
-  <param index="0" name="p" type="Vector3" /><param index="1" name="w" type="f64" />
+  <param index="0" name="p" type="Vector3" /><param index="1" name="w" type="f64" /><param index="2" name="node" type="Gd &lt; Node &gt;" />
   <description>
-  some user signal[br][br]some multiline doc
+  some user signal[br][br]some multiline doc[br][br]The [code]Gd&lt;Node&gt;[/code] param should be properly escaped
   </description>
 </signal>
 </signals>
-<members><member name="item" type="f32" default="">this is very documented</member><member name="item_2" type="i64" default="">is it documented?</member></members>
+<members><member name="item" type="f32" default="">this is very documented</member><member name="item_2" type="i64" default="">is it documented?</member><member name="item_xml" type="GString" default="">this docstring has &lt; a special character</member></members>
 </class>