From 908cebf35c4e9be1fa91cb558389b7e80b6d0b77 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Mon, 19 Dec 2022 13:23:51 +0300
Subject: fix(tvix/builtin-macros): parse multi-line docstrings correctly

Having a multi-line docstring yields multiple doc-attributes in order,
however we were previously discarding all but the first one.

This reduces them into a single string instead, which can then be
displayed as multi-line documentation.

Change-Id: I1f237956cdea2e4c746d3f13744e0373c1c645a6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7594
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
---
 tvix/eval/builtin-macros/src/lib.rs     | 20 +++++++++++++++++---
 tvix/eval/builtin-macros/tests/tests.rs | 13 +++++++++++--
 2 files changed, 28 insertions(+), 5 deletions(-)

(limited to 'tvix/eval/builtin-macros')

diff --git a/tvix/eval/builtin-macros/src/lib.rs b/tvix/eval/builtin-macros/src/lib.rs
index e815749ec62c..ac994954ba60 100644
--- a/tvix/eval/builtin-macros/src/lib.rs
+++ b/tvix/eval/builtin-macros/src/lib.rs
@@ -22,11 +22,14 @@ impl Parse for BuiltinArgs {
     }
 }
 
-fn extract_docstring(attrs: &[Attribute]) -> Option<LitStr> {
+fn extract_docstring(attrs: &[Attribute]) -> Option<String> {
     // Rust docstrings are transparently written pre-macro expansion into an attribute that looks
     // like:
     //
     // #[doc = "docstring here"]
+    //
+    // Multi-line docstrings yield multiple attributes in order, which we assemble into a single
+    // string below.
 
     #[allow(dead_code)]
     #[derive(Debug)]
@@ -47,8 +50,19 @@ fn extract_docstring(attrs: &[Attribute]) -> Option<LitStr> {
     attrs
         .iter()
         .filter(|attr| attr.path.get_ident().into_iter().any(|id| id == "doc"))
-        .find_map(|attr| parse2::<Docstring>(attr.tokens.clone()).ok())
-        .map(|docstring| docstring.doc)
+        .filter_map(|attr| parse2::<Docstring>(attr.tokens.clone()).ok())
+        .map(|docstring| docstring.doc.value())
+        .reduce(|mut fst, snd| {
+            if snd.is_empty() {
+                // An empty string represents a spacing newline that was added in the
+                // original doc comment.
+                fst.push_str("\n\n");
+            } else {
+                fst.push_str(&snd);
+            }
+
+            fst
+        })
 }
 
 /// Mark the annotated module as a module for defining Nix builtins.
diff --git a/tvix/eval/builtin-macros/tests/tests.rs b/tvix/eval/builtin-macros/tests/tests.rs
index b270594b0f33..5705d9202f6e 100644
--- a/tvix/eval/builtin-macros/tests/tests.rs
+++ b/tvix/eval/builtin-macros/tests/tests.rs
@@ -7,7 +7,9 @@ mod builtins {
     use tvix_eval::internal::VM;
     use tvix_eval::{ErrorKind, Value};
 
-    /// Test docstring
+    /// Test docstring.
+    ///
+    /// It has multiple lines!
     #[builtin("identity")]
     pub fn builtin_identity(_vm: &mut VM, x: Value) -> Result<Value, ErrorKind> {
         Ok(x)
@@ -25,5 +27,12 @@ fn builtins() {
     assert_eq!(builtins.len(), 2);
 
     let identity = builtins.iter().find(|b| b.name() == "identity").unwrap();
-    assert_eq!(identity.documentation(), Some(" Test docstring"));
+    assert_eq!(
+        identity.documentation(),
+        Some(
+            r#" Test docstring.
+
+ It has multiple lines!"#
+        )
+    );
 }
-- 
cgit 1.4.1