Repository: rust-analyzer/rust-analyzer
Pull request #167 (2018-10-30 18:17:35)

Attempt to extract useful comments from function signatures
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?

kjeremy: I decided not to restrict to doc comments since internally people frequently comment with `//` but that can certainly be changed.
kjeremy: Note that tests won't pass until #168 is merged.
matklad: bors try

Patch:
diff --git a/crates/ra_analysis/src/descriptors/function/mod.rs b/crates/ra_analysis/src/descriptors/function/mod.rs
index bb68b0ce75..ae40f3e8f8 100644
--- a/crates/ra_analysis/src/descriptors/function/mod.rs
+++ b/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() {
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index b3c094041c..ab795a647e 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -102,8 +102,6 @@ impl AnalysisHostImpl {
             .set(WORKSPACE, Default::default());
         db.query(crate::input::CrateGraphQuery)
             .set((), Default::default());
-        db.query(crate::input::LibrariesQuery)
-            .set((), Default::default());
         AnalysisHostImpl { db }
     }
     pub fn analysis(&self) -> AnalysisImpl {
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 4ac08e527f..20cb5f7728 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/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),
         };
 
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index d0cd060d30..b6a15dbdc1 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/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)
diff --git a/crates/ra_syntax/src/ast/mod.rs b/crates/ra_syntax/src/ast/mod.rs
index c033263a1b..3aa11b9dda 100644
--- a/crates/ra_syntax/src/ast/mod.rs
+++ b/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)
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index c1c215e0d6..6951db010a 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -251,6 +251,7 @@ Grammar(
                 "NameOwner",
                 "TypeParamsOwner",
                 "AttrsOwner",
+                "DocCommentsOwner"
             ],
             options: [ "ParamList", ["body", "Block"], "RetType" ],
         ),


Test patch:
diff --git a/crates/ra_analysis/tests/tests.rs b/crates/ra_analysis/tests/tests.rs
index f5683aec5b..03e2df48e2 100644
--- a/crates/ra_analysis/tests/tests.rs
+++ b/crates/ra_analysis/tests/tests.rs
@@ -185,6 +185,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 (offset, code) = extract_offset(text);
     let code = code.as_str();