fix: Don't give empty documentation to starlark-rust

src/bazel.rs

@@ -1277,4 +1277,37 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    /// Empty summary in DocString strings break starlark-rust. See #41. Here
+    /// we ensure that instead of generating an empty summary, the whole
+    /// DocString is None.
+    fn no_empty_documentation_is_produced() -> anyhow::Result<()> {
+        let fixture = TestFixture::new("simple")?;
+        let context = fixture.context()?;
+
+        let module = context.get_environment(&LspUrl::File(PathBuf::from("/foo/bar/defs.bzl")));
+
+        fn check_doc_not_empty(doc: Option<&DocString>) {
+            if let Some(doc) = doc {
+                assert!(!doc.summary.trim().is_empty());
+            }
+        }
+
+        for (name, member) in module.members {
+            match member {
+                DocMember::Function(function) => {
+                    check_doc_not_empty(function.docs.as_ref());
+                    for param in function.params {
+                        check_doc_not_empty(param.get_doc_string());
+                    }
+                }
+                DocMember::Property(property) => {
+                    check_doc_not_empty(property.docs.as_ref());
+                }
+            }
+        }
+
+        Ok(())
+    }
 }

src/builtin.rs

@@ -18,19 +18,19 @@ pub fn build_language_to_doc_members<'a>(
 
 pub fn rule_to_doc_member(rule: &RuleDefinition) -> DocMember {
     DocMember::Function(DocFunction {
-        docs: rule.documentation.as_ref().map(|doc| DocString {
-            summary: doc.clone(),
-            details: None,
-        }),
+        docs: rule
+            .documentation
+            .as_ref()
+            .and_then(|doc| create_docstring(doc)),
         params: rule
             .attribute
             .iter()
             .map(|attribute| DocParam::Arg {
                 name: attribute.name.clone(),
-                docs: attribute.documentation.as_ref().map(|doc| DocString {
-                    summary: doc.clone(),
-                    details: None,
-                }),
+                docs: attribute
+                    .documentation
+                    .as_ref()
+                    .and_then(|doc| create_docstring(doc)),
                 typ: Ty::any(),
                 default_value: None,
             })
@@ -55,11 +55,8 @@ pub fn builtins_to_doc_members<'a>(
     })
 }
 
-pub fn value_to_doc_member(value: &Value) -> DocMember {
-    let docs = Some(DocString {
-        summary: value.doc.clone(),
-        details: None,
-    });
+fn value_to_doc_member(value: &Value) -> DocMember {
+    let docs = create_docstring(&value.doc);
 
     if let Some(callable) = &value.callable {
         DocMember::Function(DocFunction {
@@ -69,10 +66,7 @@ pub fn value_to_doc_member(value: &Value) -> DocMember {
                 .iter()
                 .map(|param| DocParam::Arg {
                     name: param.name.clone(),
-                    docs: Some(DocString {
-                        summary: param.doc.clone(),
-                        details: None,
-                    }),
+                    docs: create_docstring(&param.doc),
                     typ: Ty::any(),
                     default_value: if param.is_mandatory {
                         None
@@ -90,3 +84,16 @@ pub fn value_to_doc_member(value: &Value) -> DocMember {
         })
     }
 }
+
+fn create_docstring(summary: &str) -> Option<DocString> {
+    let summary = summary.trim();
+
+    if summary.is_empty() {
+        None
+    } else {
+        Some(DocString {
+            summary: summary.to_string(),
+            details: None,
+        })
+    }
+}