Merge #1110

1110: Introduce display module and implement new FunctionSignature for CallInfo's r=matklad a=vipentti

This introduces a new module `display` in `ra_ide_api` that contains UI-related things, in addition this refactors CallInfo's function signatures into a new `FunctionSignature` type, which implements `Display` and can be converted into `lsp_types::SignatureInformation` in the `conv` layer. 

Currently only `CallInfo` uses the `FunctionSignature` directly, but `function_label` now uses the same signature and returns it as a string, using the `Display` implementation.

This also fixes #960 

I think this similar structure could be applied to other UI-displayable items, so instead of the `ra_ide_api` returning `Strings` we could return some intermediate structures that can be converted into a UI-displayable `String` easily, but that could also provide some additional information.

Co-authored-by: Ville Penttinen <[email protected]>

16 files changed, 391 insertions, 231 deletions

diff --git a/crates/ra_ide_api/src/call_info.rs b/crates/ra_ide_api/src/call_info.rs
index 29fa7d30b..dbb3853d0 100644
--- a/crates/ra_ide_api/src/call_info.rs
+++ b/crates/ra_ide_api/src/call_info.rs
@@ -6,9 +6,8 @@ use ra_syntax::{
     ast::{self, ArgListOwner},
     algo::find_node_at_offset,
 };
-use hir::Docs;
 
-use crate::{FilePosition, CallInfo, db::RootDatabase};
+use crate::{FilePosition, CallInfo, FunctionSignature, db::RootDatabase};
 
 /// Computes parameter information for the given call expression.
 pub(crate) fn call_info(db: &RootDatabase, position: FilePosition) -> Option<CallInfo> {
@@ -27,10 +26,10 @@ pub(crate) fn call_info(db: &RootDatabase, position: FilePosition) -> Option<Cal
     let fn_def = ast::FnDef::cast(fn_def).unwrap();
     let function = hir::source_binder::function_from_source(db, symbol.file_id, fn_def)?;
 
-    let mut call_info = CallInfo::new(db, function, fn_def)?;
+    let mut call_info = CallInfo::new(db, function);
 
     // 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 {
@@ -107,28 +106,15 @@ 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 doc = function.docs(db);
+    fn new(db: &RootDatabase, function: hir::Function) -> Self {
+        let signature = FunctionSignature::from_hir(db, function);
 
-        Some(CallInfo { parameters: param_list(node), label, doc, active_parameter: None })
+        CallInfo { signature, 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())
-        }
 
-        // 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()),
-        );
+    fn parameters(&self) -> &[String] {
+        &self.signature.parameters
     }
-    res
 }
 
 #[cfg(test)]
@@ -139,6 +125,17 @@ mod tests {
 
     use super::*;
 
+    // These are only used when testing
+    impl CallInfo {
+        fn doc(&self) -> Option<hir::Documentation> {
+            self.signature.doc.clone()
+        }
+
+        fn label(&self) -> String {
+            self.signature.to_string()
+        }
+    }
+
     fn call_info(text: &str) -> CallInfo {
         let (analysis, position) = single_file_with_position(text);
         analysis.call_info(position).unwrap().unwrap()
@@ -151,7 +148,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 +159,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,18 +170,57 @@ 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));
     }
 
     #[test]
+    fn test_fn_signature_no_params() {
+        let info = call_info(
+            r#"fn foo<T>() -> T where T: Copy + Display {}
+fn bar() { foo(<|>); }"#,
+        );
+
+        assert!(info.parameters().is_empty());
+        assert_eq!(
+            info.label(),
+            r#"
+fn foo<T>() -> T
+where T: Copy + Display
+    "#
+            .trim()
+        );
+        assert!(info.active_parameter.is_none());
+    }
+
+    #[test]
     fn test_fn_signature_for_impl() {
         let info = call_info(
             r#"struct F; impl F { pub fn new() { F{}} }
 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 +242,7 @@ fn bar() {
 }"#,
         );
 
-        assert_eq!(info.parameters, vec!["&self".to_string()]);
+        assert_eq!(info.parameters(), ["&self"]);
         assert_eq!(info.active_parameter, None);
     }
 
@@ -228,7 +264,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 +284,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 +312,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 +358,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 +411,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..deff59cd3 100644
--- a/crates/ra_ide_api/src/completion.rs
+++ b/crates/ra_ide_api/src/completion.rs
@@ -13,7 +13,6 @@ mod complete_scope;
 mod complete_postfix;
 
 use ra_db::SourceDatabase;
-use ra_syntax::{ast::{self, AstNode}, SyntaxKind::{ATTR, COMMENT}};
 
 use crate::{
     db,
@@ -70,43 +69,3 @@ pub(crate) fn completions(db: &db::RootDatabase, position: FilePosition) -> Opti
     complete_postfix::complete_postfix(&mut acc, &ctx);
     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()
-    };
-
-    Some(label.trim().to_owned())
-}
-
-pub fn const_label(node: &ast::ConstDef) -> String {
-    let label: String = node
-        .syntax()
-        .children_with_tokens()
-        .filter(|child| !(child.kind() == COMMENT || child.kind() == ATTR))
-        .map(|node| node.to_string())
-        .collect();
-
-    label.trim().to_owned()
-}
-
-pub fn type_label(node: &ast::TypeAliasDef) -> String {
-    let label: String = node
-        .syntax()
-        .children_with_tokens()
-        .filter(|child| !(child.kind() == COMMENT || child.kind() == ATTR))
-        .map(|node| node.to_string())
-        .collect();
-
-    label.trim().to_owned()
-}
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/completion/presentation.rs b/crates/ra_ide_api/src/completion/presentation.rs
index 28c8f83ab..9aa346688 100644
--- a/crates/ra_ide_api/src/completion/presentation.rs
+++ b/crates/ra_ide_api/src/completion/presentation.rs
@@ -6,6 +6,9 @@ use ra_syntax::ast::NameOwner;
 
 use crate::completion::{
     Completions, CompletionKind, CompletionItemKind, CompletionContext, CompletionItem,
+};
+
+use crate::display::{
     function_label, const_label, type_label,
 };
 
@@ -101,7 +104,7 @@ impl Completions {
                 CompletionItemKind::Function
             })
             .set_documentation(func.docs(ctx.db))
-            .set_detail(detail);
+            .detail(detail);
         // If not an import, add parenthesis automatically.
         if ctx.use_item_syntax.is_none() && !ctx.is_call {
             tested_by!(inserts_parens_for_function_calls);
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_show_both_completions_for_shadowing.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_show_both_completions_for_shadowing.snap
index 87691b304..34adcda6c 100644
--- a/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_show_both_completions_for_shadowing.snap
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__dont_show_both_completions_for_shadowing.snap
@@ -1,23 +1,23 @@
 ---
-created: "2019-02-18T09:22:24.188564584Z"
-creator: insta@0.6.2
+created: "2019-04-04T14:52:24.531844100Z"
+creator: insta@0.7.4
 source: crates/ra_ide_api/src/completion/completion_item.rs
 expression: kind_completions
 ---
 [
     CompletionItem {
         label: "bar",
-        source_range: [129; 129),
-        delete: [129; 129),
+        source_range: [126; 126),
+        delete: [126; 126),
         insert: "bar",
         kind: Binding
     },
     CompletionItem {
         label: "foo",
-        source_range: [129; 129),
-        delete: [129; 129),
+        source_range: [126; 126),
+        delete: [126; 126),
         insert: "foo()$0",
         kind: Function,
-        detail: "fn foo() ->"
+        detail: "fn foo()"
     }
 ]
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__return_type.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__return_type.snap
index 0738cf466..ff36df707 100644
--- a/crates/ra_ide_api/src/completion/snapshots/completion_item__return_type.snap
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__return_type.snap
@@ -1,6 +1,6 @@
 ---
-created: "2019-02-18T09:22:24.182964414Z"
-creator: insta@0.6.2
+created: "2019-04-04T14:52:24.525395600Z"
+creator: insta@0.7.4
 source: crates/ra_ide_api/src/completion/completion_item.rs
 expression: kind_completions
 ---
@@ -18,6 +18,6 @@ expression: kind_completions
         delete: [47; 47),
         insert: "x()$0",
         kind: Function,
-        detail: "fn x() ->"
+        detail: "fn x()"
     }
 ]
diff --git a/crates/ra_ide_api/src/display.rs b/crates/ra_ide_api/src/display.rs
new file mode 100644
index 000000000..1b06abf94
--- /dev/null
+++ b/crates/ra_ide_api/src/display.rs
@@ -0,0 +1,82 @@
+//! This module contains utilities for turning SyntaxNodes and HIR types
+//! into types that may be used to render in a UI.
+
+mod function_signature;
+mod navigation_target;
+mod structure;
+
+use crate::db::RootDatabase;
+use ra_syntax::{ast::{self, AstNode, TypeParamsOwner}, SyntaxKind::{ATTR, COMMENT}};
+
+pub use navigation_target::NavigationTarget;
+pub use structure::{StructureNode, file_structure};
+pub use function_signature::FunctionSignature;
+
+pub(crate) fn function_label(node: &ast::FnDef) -> String {
+    FunctionSignature::from(node).to_string()
+}
+
+pub(crate) fn const_label(node: &ast::ConstDef) -> String {
+    let label: String = node
+        .syntax()
+        .children_with_tokens()
+        .filter(|child| !(child.kind() == COMMENT || child.kind() == ATTR))
+        .map(|node| node.to_string())
+        .collect();
+
+    label.trim().to_owned()
+}
+
+pub(crate) fn type_label(node: &ast::TypeAliasDef) -> String {
+    let label: String = node
+        .syntax()
+        .children_with_tokens()
+        .filter(|child| !(child.kind() == COMMENT || child.kind() == ATTR))
+        .map(|node| node.to_string())
+        .collect();
+
+    label.trim().to_owned()
+}
+
+pub(crate) 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(crate) 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(crate) fn rust_code_markup<CODE: AsRef<str>>(val: CODE) -> String {
+    rust_code_markup_with_doc::<_, &str>(val, None)
+}
+
+pub(crate) fn rust_code_markup_with_doc<CODE, DOC>(val: CODE, doc: Option<DOC>) -> String
+where
+    CODE: AsRef<str>,
+    DOC: AsRef<str>,
+{
+    if let Some(doc) = doc {
+        format!("```rust\n{}\n```\n\n{}", val.as_ref(), doc.as_ref())
+    } else {
+        format!("```rust\n{}\n```", val.as_ref())
+    }
+}
+
+// FIXME: this should not really use navigation target. Rather, approximately
+// resolved symbol should return a `DefId`.
+pub(crate) fn doc_text_for(db: &RootDatabase, nav: NavigationTarget) -> Option<String> {
+    match (nav.description(db), nav.docs(db)) {
+        (Some(desc), docs) => Some(rust_code_markup_with_doc(desc, docs)),
+        (None, Some(docs)) => Some(docs),
+        _ => None,
+    }
+}
diff --git a/crates/ra_ide_api/src/display/function_signature.rs b/crates/ra_ide_api/src/display/function_signature.rs
new file mode 100644
index 000000000..d09950bce
--- /dev/null
+++ b/crates/ra_ide_api/src/display/function_signature.rs
@@ -0,0 +1,101 @@
+use super::{where_predicates, generic_parameters};
+use crate::db;
+use std::fmt::{self, Display};
+use join_to_string::join;
+use ra_syntax::ast::{self, AstNode, NameOwner, VisibilityOwner};
+use std::convert::From;
+use hir::{Docs, Documentation};
+
+/// 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>,
+    /// 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
+    }
+
+    pub(crate) fn from_hir(db: &db::RootDatabase, function: hir::Function) -> Self {
+        let doc = function.docs(db);
+        let (_, ast_node) = function.source(db);
+        FunctionSignature::from(&*ast_node).with_doc_opt(doc)
+    }
+}
+
+impl From<&'_ ast::FnDef> for FunctionSignature {
+    fn from(node: &ast::FnDef) -> 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
+        }
+
+        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,
+        }
+    }
+}
+
+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() {
+            join(self.generic_parameters.iter())
+                .separator(", ")
+                .surround_with("<", ">")
+                .to_fmt(f)?;
+        }
+
+        join(self.parameters.iter()).separator(", ").surround_with("(", ")").to_fmt(f)?;
+
+        if let Some(t) = &self.ret_type {
+            write!(f, " -> {}", t)?;
+        }
+
+        if !self.where_predicates.is_empty() {
+            write!(f, "\nwhere ")?;
+            join(self.where_predicates.iter()).separator(",\n      ").to_fmt(f)?;
+        }
+
+        Ok(())
+    }
+}
diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/display/navigation_target.rs
index f6d7f3192..3c518faf5 100644
--- a/crates/ra_ide_api/src/navigation_target.rs
+++ b/crates/ra_ide_api/src/display/navigation_target.rs
@@ -1,7 +1,9 @@
-use ra_db::FileId;
+use ra_db::{FileId, SourceDatabase};
 use ra_syntax::{
-    SyntaxNode, SyntaxNodePtr, AstNode, SmolStr, TextRange, ast,
+    SyntaxNode, SyntaxNodePtr, AstNode, SmolStr, TextRange, TreeArc,
     SyntaxKind::{self, NAME},
+    ast::{self, NameOwner, VisibilityOwner, TypeAscriptionOwner},
+    algo::visit::{visitor, Visitor},
 };
 use hir::{ModuleSource, FieldSource, Name, ImplItem};
 
@@ -248,4 +250,80 @@ impl NavigationTarget {
             container_name: None,
         }
     }
+
+    pub(crate) fn node(&self, db: &RootDatabase) -> Option<TreeArc<SyntaxNode>> {
+        let source_file = db.parse(self.file_id());
+        let source_file = source_file.syntax();
+        let node = source_file
+            .descendants()
+            .find(|node| node.kind() == self.kind() && node.range() == self.full_range())?
+            .to_owned();
+        Some(node)
+    }
+
+    pub(crate) fn docs(&self, db: &RootDatabase) -> Option<String> {
+        let node = self.node(db)?;
+        fn doc_comments<N: ast::DocCommentsOwner>(node: &N) -> Option<String> {
+            node.doc_comment_text()
+        }
+
+        visitor()
+            .visit(doc_comments::<ast::FnDef>)
+            .visit(doc_comments::<ast::StructDef>)
+            .visit(doc_comments::<ast::EnumDef>)
+            .visit(doc_comments::<ast::TraitDef>)
+            .visit(doc_comments::<ast::Module>)
+            .visit(doc_comments::<ast::TypeAliasDef>)
+            .visit(doc_comments::<ast::ConstDef>)
+            .visit(doc_comments::<ast::StaticDef>)
+            .visit(doc_comments::<ast::NamedFieldDef>)
+            .visit(doc_comments::<ast::EnumVariant>)
+            .accept(&node)?
+    }
+
+    /// Get a description of this node.
+    ///
+    /// e.g. `struct Name`, `enum Name`, `fn Name`
+    pub(crate) fn description(&self, db: &RootDatabase) -> Option<String> {
+        // FIXME: After type inference is done, add type information to improve the output
+        let node = self.node(db)?;
+
+        fn visit_ascribed_node<T>(node: &T, prefix: &str) -> Option<String>
+        where
+            T: NameOwner + VisibilityOwner + TypeAscriptionOwner,
+        {
+            let mut string = visit_node(node, prefix)?;
+
+            if let Some(type_ref) = node.ascribed_type() {
+                string.push_str(": ");
+                type_ref.syntax().text().push_to(&mut string);
+            }
+
+            Some(string)
+        }
+
+        fn visit_node<T>(node: &T, label: &str) -> Option<String>
+        where
+            T: NameOwner + VisibilityOwner,
+        {
+            let mut string =
+                node.visibility().map(|v| format!("{} ", v.syntax().text())).unwrap_or_default();
+            string.push_str(label);
+            string.push_str(node.name()?.text().as_str());
+            Some(string)
+        }
+
+        visitor()
+            .visit(|node: &ast::FnDef| Some(crate::display::function_label(node)))
+            .visit(|node: &ast::StructDef| visit_node(node, "struct "))
+            .visit(|node: &ast::EnumDef| visit_node(node, "enum "))
+            .visit(|node: &ast::TraitDef| visit_node(node, "trait "))
+            .visit(|node: &ast::Module| visit_node(node, "mod "))
+            .visit(|node: &ast::TypeAliasDef| visit_node(node, "type "))
+            .visit(|node: &ast::ConstDef| visit_ascribed_node(node, "const "))
+            .visit(|node: &ast::StaticDef| visit_ascribed_node(node, "static "))
+            .visit(|node: &ast::NamedFieldDef| visit_ascribed_node(node, ""))
+            .visit(|node: &ast::EnumVariant| Some(node.name()?.text().to_string()))
+            .accept(&node)?
+    }
 }
diff --git a/crates/ra_ide_api/src/snapshots/tests__file_structure.snap b/crates/ra_ide_api/src/display/snapshots/tests__file_structure.snap
index 2efa8e22c..32dd99484 100644
--- a/crates/ra_ide_api/src/snapshots/tests__file_structure.snap
+++ b/crates/ra_ide_api/src/display/snapshots/tests__file_structure.snap
@@ -1,7 +1,7 @@
 ---
-created: "2019-02-05T22:03:50.763530100Z"
-creator: insta@0.6.1
-source: crates/ra_ide_api/src/structure.rs
+created: "2019-04-08T09:44:50.196004400Z"
+creator: insta@0.7.4
+source: crates/ra_ide_api/src/display/structure.rs
 expression: structure
 ---
 [
diff --git a/crates/ra_ide_api/src/structure.rs b/crates/ra_ide_api/src/display/structure.rs
index ec2c9bbc6..ec2c9bbc6 100644
--- a/crates/ra_ide_api/src/structure.rs
+++ b/crates/ra_ide_api/src/display/structure.rs
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index bfa7cd67a..3a8c93b99 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -1,11 +1,11 @@
 use ra_db::SourceDatabase;
 use ra_syntax::{
-    AstNode, SyntaxNode, TreeArc, ast::{self, NameOwner, VisibilityOwner, TypeAscriptionOwner},
-    algo::{find_covering_element, find_node_at_offset, find_token_at_offset, visit::{visitor, Visitor}},
+    AstNode, ast,
+    algo::{find_covering_element, find_node_at_offset, find_token_at_offset},
 };
 use hir::HirDisplay;
 
-use crate::{db::RootDatabase, RangeInfo, FilePosition, FileRange, NavigationTarget};
+use crate::{db::RootDatabase, RangeInfo, FilePosition, FileRange, display::{rust_code_markup, doc_text_for}};
 
 /// Contains the results when hovering over an item
 #[derive(Debug, Clone)]
@@ -145,110 +145,6 @@ pub(crate) fn type_of(db: &RootDatabase, frange: FileRange) -> Option<String> {
     }
 }
 
-fn rust_code_markup<CODE: AsRef<str>>(val: CODE) -> String {
-    rust_code_markup_with_doc::<_, &str>(val, None)
-}
-
-fn rust_code_markup_with_doc<CODE, DOC>(val: CODE, doc: Option<DOC>) -> String
-where
-    CODE: AsRef<str>,
-    DOC: AsRef<str>,
-{
-    if let Some(doc) = doc {
-        format!("```rust\n{}\n```\n\n{}", val.as_ref(), doc.as_ref())
-    } else {
-        format!("```rust\n{}\n```", val.as_ref())
-    }
-}
-
-// FIXME: this should not really use navigation target. Rather, approximately
-// resolved symbol should return a `DefId`.
-fn doc_text_for(db: &RootDatabase, nav: NavigationTarget) -> Option<String> {
-    match (nav.description(db), nav.docs(db)) {
-        (Some(desc), docs) => Some(rust_code_markup_with_doc(desc, docs)),
-        (None, Some(docs)) => Some(docs),
-        _ => None,
-    }
-}
-
-impl NavigationTarget {
-    fn node(&self, db: &RootDatabase) -> Option<TreeArc<SyntaxNode>> {
-        let source_file = db.parse(self.file_id());
-        let source_file = source_file.syntax();
-        let node = source_file
-            .descendants()
-            .find(|node| node.kind() == self.kind() && node.range() == self.full_range())?
-            .to_owned();
-        Some(node)
-    }
-
-    fn docs(&self, db: &RootDatabase) -> Option<String> {
-        let node = self.node(db)?;
-        fn doc_comments<N: ast::DocCommentsOwner>(node: &N) -> Option<String> {
-            node.doc_comment_text()
-        }
-
-        visitor()
-            .visit(doc_comments::<ast::FnDef>)
-            .visit(doc_comments::<ast::StructDef>)
-            .visit(doc_comments::<ast::EnumDef>)
-            .visit(doc_comments::<ast::TraitDef>)
-            .visit(doc_comments::<ast::Module>)
-            .visit(doc_comments::<ast::TypeAliasDef>)
-            .visit(doc_comments::<ast::ConstDef>)
-            .visit(doc_comments::<ast::StaticDef>)
-            .visit(doc_comments::<ast::NamedFieldDef>)
-            .visit(doc_comments::<ast::EnumVariant>)
-            .accept(&node)?
-    }
-
-    /// Get a description of this node.
-    ///
-    /// e.g. `struct Name`, `enum Name`, `fn Name`
-    fn description(&self, db: &RootDatabase) -> Option<String> {
-        // FIXME: After type inference is done, add type information to improve the output
-        let node = self.node(db)?;
-
-        fn visit_ascribed_node<T>(node: &T, prefix: &str) -> Option<String>
-        where
-            T: NameOwner + VisibilityOwner + TypeAscriptionOwner,
-        {
-            let mut string = visit_node(node, prefix)?;
-
-            if let Some(type_ref) = node.ascribed_type() {
-                string.push_str(": ");
-                type_ref.syntax().text().push_to(&mut string);
-            }
-
-            Some(string)
-        }
-
-        fn visit_node<T>(node: &T, label: &str) -> Option<String>
-        where
-            T: NameOwner + VisibilityOwner,
-        {
-            let mut string =
-                node.visibility().map(|v| format!("{} ", v.syntax().text())).unwrap_or_default();
-            string.push_str(label);
-            string.push_str(node.name()?.text().as_str());
-            Some(string)
-        }
-
-        visitor()
-            .visit(crate::completion::function_label)
-            .visit(|node: &ast::StructDef| visit_node(node, "struct "))
-            .visit(|node: &ast::EnumDef| visit_node(node, "enum "))
-            .visit(|node: &ast::TraitDef| visit_node(node, "trait "))
-            .visit(|node: &ast::Module| visit_node(node, "mod "))
-            .visit(|node: &ast::TypeAliasDef| visit_node(node, "type "))
-            .visit(|node: &ast::ConstDef| visit_ascribed_node(node, "const "))
-            .visit(|node: &ast::StaticDef| visit_ascribed_node(node, "static "))
-            .visit(|node: &ast::NamedFieldDef| visit_ascribed_node(node, ""))
-            .visit(|node: &ast::EnumVariant| Some(node.name()?.text().to_string()))
-            .accept(&node)?
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use ra_syntax::TextRange;
diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs
index 9063f78a9..d25795adc 100644
--- a/crates/ra_ide_api/src/lib.rs
+++ b/crates/ra_ide_api/src/lib.rs
@@ -13,7 +13,6 @@
 mod db;
 pub mod mock_analysis;
 mod symbol_index;
-mod navigation_target;
 mod change;
 
 mod status;
@@ -34,9 +33,9 @@ mod folding_ranges;
 mod line_index;
 mod line_index_utils;
 mod join_lines;
-mod structure;
 mod typing;
 mod matching_brace;
+mod display;
 
 #[cfg(test)]
 mod marks;
@@ -62,7 +61,6 @@ pub use crate::{
     change::{AnalysisChange, LibraryData},
     completion::{CompletionItem, CompletionItemKind, InsertTextFormat},
     runnables::{Runnable, RunnableKind},
-    navigation_target::NavigationTarget,
     references::ReferenceSearchResult,
     assists::{Assist, AssistId},
     hover::{HoverResult},
@@ -70,8 +68,8 @@ pub use crate::{
     line_index_utils::translate_offset_with_edit,
     folding_ranges::{Fold, FoldKind},
     syntax_highlighting::HighlightedRange,
-    structure::{StructureNode, file_structure},
     diagnostics::Severity,
+    display::{FunctionSignature, NavigationTarget, StructureNode, file_structure},
 };
 
 pub use ra_db::{
@@ -243,9 +241,7 @@ impl<T> RangeInfo<T> {
 
 #[derive(Debug)]
 pub struct CallInfo {
-    pub label: String,
-    pub doc: Option<Documentation>,
-    pub parameters: Vec<String>,
+    pub signature: FunctionSignature,
     pub active_parameter: Option<usize>,
 }
 
@@ -387,7 +383,7 @@ impl Analysis {
     /// file outline.
     pub fn file_structure(&self, file_id: FileId) -> Vec<StructureNode> {
         let file = self.db.parse(file_id);
-        structure::file_structure(&file)
+        file_structure(&file)
     }
 
     /// Returns the set of folding ranges.
diff --git a/crates/ra_ide_api/src/symbol_index.rs b/crates/ra_ide_api/src/symbol_index.rs
index 0eadc4e71..914d3fc71 100644
--- a/crates/ra_ide_api/src/symbol_index.rs
+++ b/crates/ra_ide_api/src/symbol_index.rs
@@ -275,7 +275,7 @@ fn to_file_symbol(node: &SyntaxNode, file_id: FileId) -> Option<FileSymbol> {
 mod tests {
     use ra_syntax::SmolStr;
     use crate::{
-        navigation_target::NavigationTarget,
+        display::NavigationTarget,
         mock_analysis::single_file,
         Query,
 };
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)