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 <[email protected]>

2 files changed, 204 insertions, 3 deletions

diff --git a/crates/ra_analysis/src/descriptors/function/mod.rs b/crates/ra_analysis/src/descriptors/function/mod.rs
index bb68b0ce7..ae40f3e8f 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/tests/tests.rs b/crates/ra_analysis/tests/tests.rs
index 94e025677..9e2478d9e 100644
--- a/crates/ra_analysis/tests/tests.rs
+++ b/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()