Merge #167

167: Attempt to extract useful comments from function signatures r=matklad a=kjeremy

I'm trying to extract useful function comments for signature info. This will also be useful for hover.  This is a WIP (and actually works pretty well!) but I don't think it's the right approach long term so some guidance would be appreciated so that we could also get comments for say types and variable instances etc.

Currently `test_fn_signature_with_simple_doc` fails due to a bug in `extend` but we probably shouldn't use this approach anyway. Maybe comments should be attached to nodes somehow? I'm also thinking that maybe the markdown bits should live in the language server.

Thoughts?

Co-authored-by: Jeremy A. Kolb <jkolb@ara.com>

crates/ra_analysis/src/descriptors/function/mod.rs

@@ -1,8 +1,11 @@
 pub(super) mod imp;
 mod scope;
 
+use std::cmp::{min, max};
+
 use ra_syntax::{
-    ast::{self, AstNode, NameOwner}
+    ast::{self, AstNode, DocCommentsOwner, NameOwner},
+    TextRange, TextUnit
 };
 
 use crate::{
@@ -30,14 +33,17 @@ pub struct FnDescriptor {
     pub label: String,
     pub ret_type: Option<String>,
     pub params: Vec<String>,
+    pub doc: Option<String>
 }
 
 impl FnDescriptor {
     pub fn new(node: ast::FnDef) -> Option<Self> {
         let name = node.name()?.text().to_string();
 
+        let mut doc = None;
+
         // Strip the body out for the label.
-        let label: String = if let Some(body) = node.body() {
+        let mut label: String = if let Some(body) = node.body() {
             let body_range = body.syntax().range();
             let label: String = node
                 .syntax()
@@ -50,6 +56,36 @@ impl FnDescriptor {
             node.syntax().text().to_string()
         };
 
+        if let Some((comment_range, docs)) = FnDescriptor::extract_doc_comments(node) {
+            let comment_range = comment_range.checked_sub(node.syntax().range().start()).unwrap();
+            let start = comment_range.start().to_usize();
+            let end = comment_range.end().to_usize();
+
+            // Remove the comment from the label
+            label.replace_range(start..end, "");
+
+            // Massage markdown
+            let mut processed_lines = Vec::new();
+            let mut in_code_block = false;
+            for line in docs.lines() {
+                if line.starts_with("```") {
+                    in_code_block = !in_code_block;
+                }
+
+                let line = if in_code_block && line.starts_with("```") && !line.contains("rust") {
+                    "```rust".into()
+                } else {
+                    line.to_string()
+                };
+
+                processed_lines.push(line);
+            }
+
+            if !processed_lines.is_empty() {
+                doc = Some(processed_lines.join("\n"));
+            }
+        }
+
         let params = FnDescriptor::param_list(node);
         let ret_type = node.ret_type().map(|r| r.syntax().text().to_string());
 
@@ -57,10 +93,28 @@ impl FnDescriptor {
             name,
             ret_type,
             params,
-            label,
+            label: label.trim().to_owned(),
+            doc
         })
     }
 
+    fn extract_doc_comments(node: ast::FnDef) -> Option<(TextRange, String)> {
+        if node.doc_comments().count() == 0 {
+            return None;
+        }
+
+        let comment_text = node.doc_comment_text();
+
+        let (begin, end) = node.doc_comments()
+            .map(|comment| comment.syntax().range())
+            .map(|range| (range.start().to_usize(), range.end().to_usize()))
+            .fold((std::usize::MAX, std::usize::MIN), |acc, range| (min(acc.0, range.0), max(acc.1, range.1)));
+
+        let range = TextRange::from_to(TextUnit::from_usize(begin), TextUnit::from_usize(end));
+
+        Some((range, comment_text))
+    }
+
     fn param_list(node: ast::FnDef) -> Vec<String> {
         let mut res = vec![];
         if let Some(param_list) = node.param_list() {

crates/ra_analysis/tests/tests.rs

@@ -195,6 +195,153 @@ fn bar() {
     assert_eq!(param, Some(1));
 }
 
+#[test]
+fn test_fn_signature_with_docs_simple() {
+    let (desc, param) = get_signature(
+        r#"
+// test
+fn foo(j: u32) -> u32 {
+    j
+}
+
+fn bar() {
+    let _ = foo(<|>);
+}
+"#,
+    );
+
+    assert_eq!(desc.name, "foo".to_string());
+    assert_eq!(desc.params, vec!["j".to_string()]);
+    assert_eq!(desc.ret_type, Some("-> u32".to_string()));
+    assert_eq!(param, Some(0));
+    assert_eq!(desc.label, "fn foo(j: u32) -> u32".to_string());
+    assert_eq!(desc.doc, Some("test".into()));
+}
+
+#[test]
+fn test_fn_signature_with_docs() {
+    let (desc, param) = get_signature(
+        r#"
+/// Adds one to the number given.
+///
+/// # Examples
+///
+/// ```
+/// let five = 5;
+///
+/// assert_eq!(6, my_crate::add_one(5));
+/// ```
+pub fn add_one(x: i32) -> i32 {
+    x + 1
+}
+
+pub fn do() {
+    add_one(<|>
+}"#,
+    );
+
+    assert_eq!(desc.name, "add_one".to_string());
+    assert_eq!(desc.params, vec!["x".to_string()]);
+    assert_eq!(desc.ret_type, Some("-> i32".to_string()));
+    assert_eq!(param, Some(0));
+    assert_eq!(desc.label, "pub fn add_one(x: i32) -> i32".to_string());
+    assert_eq!(desc.doc, Some(
+r#"Adds one to the number given.
+
+# Examples
+
+```rust
+let five = 5;
+
+assert_eq!(6, my_crate::add_one(5));
+```"#.into()));
+}
+
+#[test]
+fn test_fn_signature_with_docs_impl() {
+    let (desc, param) = get_signature(
+        r#"
+struct addr;
+impl addr {
+    /// Adds one to the number given.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let five = 5;
+    ///
+    /// assert_eq!(6, my_crate::add_one(5));
+    /// ```
+    pub fn add_one(x: i32) -> i32 {
+        x + 1
+    }
+}
+
+pub fn do_it() {
+    addr {};
+    addr::add_one(<|>);
+}"#);
+
+    assert_eq!(desc.name, "add_one".to_string());
+    assert_eq!(desc.params, vec!["x".to_string()]);
+    assert_eq!(desc.ret_type, Some("-> i32".to_string()));
+    assert_eq!(param, Some(0));
+    assert_eq!(desc.label, "pub fn add_one(x: i32) -> i32".to_string());
+    assert_eq!(desc.doc, Some(
+r#"Adds one to the number given.
+
+# Examples
+
+```rust
+let five = 5;
+
+assert_eq!(6, my_crate::add_one(5));
+```"#.into()));
+}
+
+#[test]
+fn test_fn_signature_with_docs_from_actix() {
+    let (desc, param) = get_signature(
+        r#"
+pub trait WriteHandler<E>
+where
+    Self: Actor,
+    Self::Context: ActorContext,
+{
+    /// Method is called when writer emits error.
+    ///
+    /// If this method returns `ErrorAction::Continue` writer processing
+    /// continues otherwise stream processing stops.
+    fn error(&mut self, err: E, ctx: &mut Self::Context) -> Running {
+        Running::Stop
+    }
+
+    /// Method is called when writer finishes.
+    ///
+    /// By default this method stops actor's `Context`.
+    fn finished(&mut self, ctx: &mut Self::Context) {
+        ctx.stop()
+    }
+}
+
+pub fn foo() {
+    WriteHandler r;
+    r.finished(<|>);
+}
+
+"#);
+
+    assert_eq!(desc.name, "finished".to_string());
+    assert_eq!(desc.params, vec!["&mut self".to_string(), "ctx".to_string()]);
+    assert_eq!(desc.ret_type, None);
+    assert_eq!(param, Some(1));
+    assert_eq!(desc.doc, Some(
+r#"Method is called when writer finishes.
+
+By default this method stops actor's `Context`."#.into()));
+}
+
+
 fn get_all_refs(text: &str) -> Vec<(FileId, TextRange)> {
     let (analysis, position) = single_file_with_position(text);
     analysis.find_all_refs(position.file_id, position.offset).unwrap()

crates/ra_lsp_server/src/main_loop/handlers.rs

@@ -5,7 +5,7 @@ use languageserver_types::{
     CodeActionResponse, Command, CompletionItem, CompletionItemKind, Diagnostic,
     DiagnosticSeverity, DocumentSymbol, FoldingRange, FoldingRangeKind, FoldingRangeParams,
     InsertTextFormat, Location, Position, SymbolInformation, TextDocumentIdentifier, TextEdit,
-    RenameParams, WorkspaceEdit, PrepareRenameResponse
+    RenameParams, WorkspaceEdit, PrepareRenameResponse, Documentation, MarkupContent, MarkupKind
 };
 use gen_lsp_server::ErrorCode;
 use ra_analysis::{FileId, FoldKind, Query, RunnableKind};
@@ -465,9 +465,18 @@ pub fn handle_signature_help(
             })
             .collect();
 
+        let documentation = if let Some(doc) = descriptor.doc {
+            Some(Documentation::MarkupContent(MarkupContent {
+                kind: MarkupKind::Markdown,
+                value: doc
+            }))
+        } else {
+            None
+        };
+
         let sig_info = SignatureInformation {
             label: descriptor.label,
-            documentation: None,
+            documentation,
             parameters: Some(parameters),
         };
 

crates/ra_syntax/src/ast/generated.rs

@@ -864,6 +864,7 @@ impl<'a> AstNode<'a> for FnDef<'a> {
 impl<'a> ast::NameOwner<'a> for FnDef<'a> {}
 impl<'a> ast::TypeParamsOwner<'a> for FnDef<'a> {}
 impl<'a> ast::AttrsOwner<'a> for FnDef<'a> {}
+impl<'a> ast::DocCommentsOwner<'a> for FnDef<'a> {}
 impl<'a> FnDef<'a> {
     pub fn param_list(self) -> Option<ParamList<'a>> {
         super::child_opt(self)

crates/ra_syntax/src/ast/mod.rs

@@ -65,6 +65,24 @@ pub trait AttrsOwner<'a>: AstNode<'a> {
     }
 }
 
+pub trait DocCommentsOwner<'a>: AstNode<'a> {
+    fn doc_comments(self) -> AstChildren<'a, Comment<'a>> { children(self) }
+
+    /// Returns the textual content of a doc comment block as a single string.
+    /// That is, strips leading `///` and joins lines
+    fn doc_comment_text(self) -> String {
+        self.doc_comments()
+            .map(|comment| {
+                let prefix = comment.prefix();
+                let trimmed = comment.text().as_str()
+                    .trim()
+                    .trim_start_matches(prefix)
+                    .trim_start();
+                trimmed.to_owned()
+            }).join("\n")
+    }
+}
+
 impl<'a> FnDef<'a> {
     pub fn has_atom_attr(&self, atom: &str) -> bool {
         self.attrs().filter_map(|x| x.as_atom()).any(|x| x == atom)

crates/ra_syntax/src/grammar.ron

@@ -251,6 +251,7 @@ Grammar(
                 "NameOwner",
                 "TypeParamsOwner",
                 "AttrsOwner",
+                "DocCommentsOwner"
             ],
             options: [ "ParamList", ["body", "Block"], "RetType" ],
         ),