Refactor CallInfo function signatures to new FunctionSignature type

This is used by CallInfo to create a pretty printed function signature that can
be used with completions and other places as well.

8 files changed, 210 insertions, 69 deletions

diff --git a/crates/ra_hir/src/docs.rs b/crates/ra_hir/src/docs.rs
index 5db72c08a..e3a755b46 100644
--- a/crates/ra_hir/src/docs.rs
+++ b/crates/ra_hir/src/docs.rs
@@ -22,6 +22,12 @@ impl Into<String> for Documentation {
     }
 }
 
+impl<'a> Into<String> for &'a Documentation {
+    fn into(self) -> String {
+        self.contents().into()
+    }
+}
+
 pub trait Docs {
     fn docs(&self, db: &impl HirDatabase) -> Option<Documentation>;
 }
diff --git a/crates/ra_ide_api/src/call_info.rs b/crates/ra_ide_api/src/call_info.rs
index 29fa7d30b..a65119315 100644
--- a/crates/ra_ide_api/src/call_info.rs
+++ b/crates/ra_ide_api/src/call_info.rs
@@ -30,7 +30,7 @@ pub(crate) fn call_info(db: &RootDatabase, position: FilePosition) -> Option<Cal
     let mut call_info = CallInfo::new(db, function, fn_def)?;
 
     // If we have a calling expression let's find which argument we are on
-    let num_params = call_info.parameters.len();
+    let num_params = call_info.parameters().len();
     let has_self = fn_def.param_list().and_then(|l| l.self_param()).is_some();
 
     if num_params == 1 {
@@ -108,27 +108,26 @@ impl<'a> FnCallNode<'a> {
 
 impl CallInfo {
     fn new(db: &RootDatabase, function: hir::Function, node: &ast::FnDef) -> Option<Self> {
-        let label = crate::completion::function_label(node)?;
+        let sig = crate::completion::function_signature(node)?;
         let doc = function.docs(db);
+        let sig = sig.with_doc_opt(doc);
 
-        Some(CallInfo { parameters: param_list(node), label, doc, active_parameter: None })
+        Some(CallInfo { signature: sig, active_parameter: None })
     }
-}
 
-fn param_list(node: &ast::FnDef) -> Vec<String> {
-    let mut res = vec![];
-    if let Some(param_list) = node.param_list() {
-        if let Some(self_param) = param_list.self_param() {
-            res.push(self_param.syntax().text().to_string())
-        }
+    fn parameters(&self) -> &[String] {
+        &self.signature.parameters
+    }
 
-        // Maybe use param.pat here? See if we can just extract the name?
-        //res.extend(param_list.params().map(|p| p.syntax().text().to_string()));
-        res.extend(
-            param_list.params().filter_map(|p| p.pat()).map(|pat| pat.syntax().text().to_string()),
-        );
+    #[cfg(test)]
+    fn doc(&self) -> Option<&hir::Documentation> {
+        self.signature.doc.as_ref()
+    }
+
+    #[cfg(test)]
+    fn label(&self) -> String {
+        self.signature.to_string()
     }
-    res
 }
 
 #[cfg(test)]
@@ -151,7 +150,7 @@ mod tests {
 fn bar() { foo(<|>3, ); }"#,
         );
 
-        assert_eq!(info.parameters, vec!("x".to_string(), "y".to_string()));
+        assert_eq!(info.parameters(), ["x: u32", "y: u32"]);
         assert_eq!(info.active_parameter, Some(0));
     }
 
@@ -162,7 +161,7 @@ fn bar() { foo(<|>3, ); }"#,
 fn bar() { foo(3, <|>); }"#,
         );
 
-        assert_eq!(info.parameters, vec!("x".to_string(), "y".to_string()));
+        assert_eq!(info.parameters(), ["x: u32", "y: u32"]);
         assert_eq!(info.active_parameter, Some(1));
     }
 
@@ -173,7 +172,27 @@ fn bar() { foo(3, <|>); }"#,
 fn bar() { foo(<|>); }"#,
         );
 
-        assert_eq!(info.parameters, vec!("x".to_string(), "y".to_string()));
+        assert_eq!(info.parameters(), ["x: u32", "y: u32"]);
+        assert_eq!(info.active_parameter, Some(0));
+    }
+
+    #[test]
+    fn test_fn_signature_two_args_first_generics() {
+        let info = call_info(
+            r#"fn foo<T, U: Copy + Display>(x: T, y: U) -> u32 where T: Copy + Display, U: Debug {x + y}
+fn bar() { foo(<|>3, ); }"#,
+        );
+
+        assert_eq!(info.parameters(), ["x: T", "y: U"]);
+        assert_eq!(
+            info.label(),
+            r#"
+fn foo<T, U: Copy + Display>(x: T, y: U) -> u32
+where T: Copy + Display,
+      U: Debug
+    "#
+            .trim()
+        );
         assert_eq!(info.active_parameter, Some(0));
     }
 
@@ -184,7 +203,7 @@ fn bar() { foo(<|>); }"#,
 fn bar() {let _ : F = F::new(<|>);}"#,
         );
 
-        assert_eq!(info.parameters, Vec::<String>::new());
+        assert!(info.parameters().is_empty());
         assert_eq!(info.active_parameter, None);
     }
 
@@ -206,7 +225,7 @@ fn bar() {
 }"#,
         );
 
-        assert_eq!(info.parameters, vec!["&self".to_string()]);
+        assert_eq!(info.parameters(), ["&self"]);
         assert_eq!(info.active_parameter, None);
     }
 
@@ -228,7 +247,7 @@ fn bar() {
 }"#,
         );
 
-        assert_eq!(info.parameters, vec!["&self".to_string(), "x".to_string()]);
+        assert_eq!(info.parameters(), ["&self", "x: i32"]);
         assert_eq!(info.active_parameter, Some(1));
     }
 
@@ -248,10 +267,10 @@ fn bar() {
 "#,
         );
 
-        assert_eq!(info.parameters, vec!["j".to_string()]);
+        assert_eq!(info.parameters(), ["j: u32"]);
         assert_eq!(info.active_parameter, Some(0));
-        assert_eq!(info.label, "fn foo(j: u32) -> u32".to_string());
-        assert_eq!(info.doc.map(|it| it.into()), Some("test".to_string()));
+        assert_eq!(info.label(), "fn foo(j: u32) -> u32");
+        assert_eq!(info.doc().map(|it| it.into()), Some("test".to_string()));
     }
 
     #[test]
@@ -276,11 +295,11 @@ pub fn do() {
 }"#,
         );
 
-        assert_eq!(info.parameters, vec!["x".to_string()]);
+        assert_eq!(info.parameters(), ["x: i32"]);
         assert_eq!(info.active_parameter, Some(0));
-        assert_eq!(info.label, "pub fn add_one(x: i32) -> i32".to_string());
+        assert_eq!(info.label(), "pub fn add_one(x: i32) -> i32");
         assert_eq!(
-            info.doc.map(|it| it.into()),
+            info.doc().map(|it| it.into()),
             Some(
                 r#"Adds one to the number given.
 
@@ -322,11 +341,11 @@ pub fn do_it() {
 }"#,
         );
 
-        assert_eq!(info.parameters, vec!["x".to_string()]);
+        assert_eq!(info.parameters(), ["x: i32"]);
         assert_eq!(info.active_parameter, Some(0));
-        assert_eq!(info.label, "pub fn add_one(x: i32) -> i32".to_string());
+        assert_eq!(info.label(), "pub fn add_one(x: i32) -> i32");
         assert_eq!(
-            info.doc.map(|it| it.into()),
+            info.doc().map(|it| it.into()),
             Some(
                 r#"Adds one to the number given.
 
@@ -375,10 +394,10 @@ pub fn foo() {
 "#,
         );
 
-        assert_eq!(info.parameters, vec!["&mut self".to_string(), "ctx".to_string()]);
+        assert_eq!(info.parameters(), ["&mut self", "ctx: &mut Self::Context"]);
         assert_eq!(info.active_parameter, Some(1));
         assert_eq!(
-            info.doc.map(|it| it.into()),
+            info.doc().map(|it| it.into()),
             Some(
                 r#"Method is called when writer finishes.
 
diff --git a/crates/ra_ide_api/src/completion.rs b/crates/ra_ide_api/src/completion.rs
index a846a7a3c..d8e4410b2 100644
--- a/crates/ra_ide_api/src/completion.rs
+++ b/crates/ra_ide_api/src/completion.rs
@@ -13,11 +13,12 @@ mod complete_scope;
 mod complete_postfix;
 
 use ra_db::SourceDatabase;
-use ra_syntax::{ast::{self, AstNode}, SyntaxKind::{ATTR, COMMENT}};
+use ra_syntax::{ast::{self, AstNode, NameOwner, VisibilityOwner, TypeParamsOwner}, SyntaxKind::{ATTR, COMMENT}};
 
 use crate::{
     db,
     FilePosition,
+    FunctionSignature,
     completion::{
         completion_item::{Completions, CompletionKind},
         completion_context::CompletionContext,
@@ -71,22 +72,52 @@ pub(crate) fn completions(db: &db::RootDatabase, position: FilePosition) -> Opti
     Some(acc)
 }
 
-pub fn function_label(node: &ast::FnDef) -> Option<String> {
-    let label: String = if let Some(body) = node.body() {
-        let body_range = body.syntax().range();
-        let label: String = node
-            .syntax()
-            .children_with_tokens()
-            .filter(|child| !child.range().is_subrange(&body_range)) // Filter out body
-            .filter(|child| !(child.kind() == COMMENT || child.kind() == ATTR)) // Filter out comments and attrs
-            .map(|node| node.to_string())
-            .collect();
-        label
-    } else {
-        node.syntax().text().to_string()
+pub fn generic_parameters<N: TypeParamsOwner>(node: &N) -> Vec<String> {
+    let mut res = vec![];
+    if let Some(type_params) = node.type_param_list() {
+        res.extend(type_params.lifetime_params().map(|p| p.syntax().text().to_string()));
+        res.extend(type_params.type_params().map(|p| p.syntax().text().to_string()));
+    }
+    res
+}
+
+pub fn where_predicates<N: TypeParamsOwner>(node: &N) -> Vec<String> {
+    let mut res = vec![];
+    if let Some(clause) = node.where_clause() {
+        res.extend(clause.predicates().map(|p| p.syntax().text().to_string()));
+    }
+    res
+}
+
+pub fn function_signature(node: &ast::FnDef) -> Option<FunctionSignature> {
+    fn param_list(node: &ast::FnDef) -> Vec<String> {
+        let mut res = vec![];
+        if let Some(param_list) = node.param_list() {
+            if let Some(self_param) = param_list.self_param() {
+                res.push(self_param.syntax().text().to_string())
+            }
+
+            res.extend(param_list.params().map(|param| param.syntax().text().to_string()));
+        }
+        res
+    }
+
+    let sig = FunctionSignature {
+        visibility: node.visibility().map(|n| n.syntax().text().to_string()),
+        name: node.name().map(|n| n.text().to_string()),
+        ret_type: node.ret_type().and_then(|r| r.type_ref()).map(|n| n.syntax().text().to_string()),
+        parameters: param_list(node),
+        generic_parameters: generic_parameters(node),
+        where_predicates: where_predicates(node),
+        // docs are processed separately
+        doc: None,
     };
 
-    Some(label.trim().to_owned())
+    Some(sig)
+}
+
+pub fn function_label(node: &ast::FnDef) -> Option<String> {
+    function_signature(node).map(|n| n.to_string())
 }
 
 pub fn const_label(node: &ast::ConstDef) -> String {
diff --git a/crates/ra_ide_api/src/completion/complete_scope.rs b/crates/ra_ide_api/src/completion/complete_scope.rs
index 6146b7bb6..9d82f2270 100644
--- a/crates/ra_ide_api/src/completion/complete_scope.rs
+++ b/crates/ra_ide_api/src/completion/complete_scope.rs
@@ -145,7 +145,7 @@ mod tests {
         check_reference_completion(
             "dont_show_both_completions_for_shadowing",
             r"
-            fn foo() -> {
+            fn foo() {
                 let bar = 92;
                 {
                     let bar = 62;
diff --git a/crates/ra_ide_api/src/display.rs b/crates/ra_ide_api/src/display.rs
new file mode 100644
index 000000000..60fa72f1b
--- /dev/null
+++ b/crates/ra_ide_api/src/display.rs
@@ -0,0 +1,51 @@
+use super::*;

+use std::fmt::{self, Display};

+

+impl Display for FunctionSignature {

+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {

+        if let Some(t) = &self.visibility {

+            write!(f, "{} ", t)?;

+        }

+

+        if let Some(name) = &self.name {

+            write!(f, "fn {}", name)?;

+        }

+

+        if !self.generic_parameters.is_empty() {

+            write!(f, "<")?;

+            write_joined(f, &self.generic_parameters, ", ")?;

+            write!(f, ">")?;

+        }

+

+        write!(f, "(")?;

+        write_joined(f, &self.parameters, ", ")?;

+        write!(f, ")")?;

+

+        if let Some(t) = &self.ret_type {

+            write!(f, " -> {}", t)?;

+        }

+

+        if !self.where_predicates.is_empty() {

+            write!(f, "\nwhere ")?;

+            write_joined(f, &self.where_predicates, ",\n      ")?;

+        }

+

+        Ok(())

+    }

+}

+

+fn write_joined<T: Display>(

+    f: &mut fmt::Formatter,

+    items: impl IntoIterator<Item = T>,

+    sep: &str,

+) -> fmt::Result {

+    let mut first = true;

+    for e in items {

+        if !first {

+            write!(f, "{}", sep)?;

+        }

+        first = false;

+        write!(f, "{}", e)?;

+    }

+    Ok(())

+}

diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs
index 9063f78a9..7f8f454bc 100644
--- a/crates/ra_ide_api/src/lib.rs
+++ b/crates/ra_ide_api/src/lib.rs
@@ -37,6 +37,7 @@ mod join_lines;
 mod structure;
 mod typing;
 mod matching_brace;
+mod display;
 
 #[cfg(test)]
 mod marks;
@@ -243,10 +244,34 @@ impl<T> RangeInfo<T> {
 
 #[derive(Debug)]
 pub struct CallInfo {
-    pub label: String,
+    pub signature: FunctionSignature,
+    pub active_parameter: Option<usize>,
+}
+
+/// Contains information about a function signature
+#[derive(Debug)]
+pub struct FunctionSignature {
+    /// Optional visibility
+    pub visibility: Option<String>,
+    /// Name of the function
+    pub name: Option<String>,
+    /// Documentation for the function
     pub doc: Option<Documentation>,
+    /// Generic parameters
+    pub generic_parameters: Vec<String>,
+    /// Parameters of the function
     pub parameters: Vec<String>,
-    pub active_parameter: Option<usize>,
+    /// Optional return type
+    pub ret_type: Option<String>,
+    /// Where predicates
+    pub where_predicates: Vec<String>,
+}
+
+impl FunctionSignature {
+    pub(crate) fn with_doc_opt(mut self, doc: Option<Documentation>) -> Self {
+        self.doc = doc;
+        self
+    }
 }
 
 /// `AnalysisHost` stores the current state of the world.
diff --git a/crates/ra_lsp_server/src/conv.rs b/crates/ra_lsp_server/src/conv.rs
index 74e91c236..4d6ede316 100644
--- a/crates/ra_lsp_server/src/conv.rs
+++ b/crates/ra_lsp_server/src/conv.rs
@@ -174,6 +174,28 @@ impl Conv for ra_ide_api::Documentation {
     }
 }
 
+impl Conv for ra_ide_api::FunctionSignature {
+    type Output = lsp_types::SignatureInformation;
+    fn conv(self) -> Self::Output {
+        use lsp_types::{ParameterInformation, ParameterLabel, SignatureInformation};
+
+        let label = self.to_string();
+
+        let documentation = self.doc.map(|it| it.conv());
+
+        let parameters: Vec<ParameterInformation> = self
+            .parameters
+            .into_iter()
+            .map(|param| ParameterInformation {
+                label: ParameterLabel::Simple(param),
+                documentation: None,
+            })
+            .collect();
+
+        SignatureInformation { label, documentation, parameters: Some(parameters) }
+    }
+}
+
 impl ConvWith for TextEdit {
     type Ctx = LineIndex;
     type Output = Vec<lsp_types::TextEdit>;
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 89e96a33a..b96deb061 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -3,8 +3,8 @@ use lsp_types::{
     CodeActionResponse, CodeLens, Command, Diagnostic, DiagnosticSeverity, CodeAction,
     DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange,
     FoldingRangeKind, FoldingRangeParams, Hover, HoverContents, Location, MarkupContent,
-    MarkupKind, ParameterInformation, ParameterLabel, Position, PrepareRenameResponse, Range,
-    RenameParams, SignatureInformation, SymbolInformation, TextDocumentIdentifier, TextEdit,
+    MarkupKind, Position, PrepareRenameResponse, Range,
+    RenameParams,SymbolInformation, TextDocumentIdentifier, TextEdit,
     WorkspaceEdit,
 };
 use ra_ide_api::{
@@ -403,26 +403,13 @@ pub fn handle_signature_help(
 ) -> Result<Option<req::SignatureHelp>> {
     let position = params.try_conv_with(&world)?;
     if let Some(call_info) = world.analysis().call_info(position)? {
-        let parameters: Vec<ParameterInformation> = call_info
-            .parameters
-            .into_iter()
-            .map(|param| ParameterInformation {
-                label: ParameterLabel::Simple(param.clone()),
-                documentation: None,
-            })
-            .collect();
+        let active_parameter = call_info.active_parameter.map(|it| it as i64);
+        let sig_info = call_info.signature.conv();
 
-        let documentation = call_info.doc.map(|it| it.conv());
-
-        let sig_info = SignatureInformation {
-            label: call_info.label,
-            documentation,
-            parameters: Some(parameters),
-        };
         Ok(Some(req::SignatureHelp {
             signatures: vec![sig_info],
             active_signature: Some(0),
-            active_parameter: call_info.active_parameter.map(|it| it as i64),
+            active_parameter,
         }))
     } else {
         Ok(None)