Repository: rust-analyzer/rust-analyzer
Pull request #462 (2019-01-08 15:45:07)

Remove UI-ish FnSignatureInfo from hir


Patch:
diff --git a/crates/ra_analysis/src/call_info.rs b/crates/ra_analysis/src/call_info.rs
new file mode 100644
index 0000000000..1dac955843
--- /dev/null
+++ b/crates/ra_analysis/src/call_info.rs
@@ -0,0 +1,451 @@
+use std::cmp::{max, min};
+
+use ra_db::{SyntaxDatabase, Cancelable};
+use ra_syntax::{
+    AstNode, SyntaxNode, TextUnit, TextRange,
+    SyntaxKind::FN_DEF,
+    ast::{self, ArgListOwner, DocCommentsOwner},
+};
+use ra_editor::find_node_at_offset;
+
+use crate::{FilePosition, CallInfo, db::RootDatabase};
+
+/// Computes parameter information for the given call expression.
+pub(crate) fn call_info(db: &RootDatabase, position: FilePosition) -> Cancelable<Option<CallInfo>> {
+    let file = db.source_file(position.file_id);
+    let syntax = file.syntax();
+
+    // Find the calling expression and it's NameRef
+    let calling_node = ctry!(FnCallNode::with_node(syntax, position.offset));
+    let name_ref = ctry!(calling_node.name_ref());
+
+    // Resolve the function's NameRef (NOTE: this isn't entirely accurate).
+    let file_symbols = db.index_resolve(name_ref)?;
+    let symbol = ctry!(file_symbols.into_iter().find(|it| it.ptr.kind() == FN_DEF));
+    let fn_file = db.source_file(symbol.file_id);
+    let fn_def = symbol.ptr.resolve(&fn_file);
+    let fn_def = ast::FnDef::cast(&fn_def).unwrap();
+    let mut call_info = ctry!(CallInfo::new(fn_def));
+    // If we have a calling expression let's find which argument we are on
+    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 {
+        if !has_self {
+            call_info.active_parameter = Some(0);
+        }
+    } else if num_params > 1 {
+        // Count how many parameters into the call we are.
+        // TODO: This is best effort for now and should be fixed at some point.
+        // It may be better to see where we are in the arg_list and then check
+        // where offset is in that list (or beyond).
+        // Revisit this after we get documentation comments in.
+        if let Some(ref arg_list) = calling_node.arg_list() {
+            let start = arg_list.syntax().range().start();
+
+            let range_search = TextRange::from_to(start, position.offset);
+            let mut commas: usize = arg_list
+                .syntax()
+                .text()
+                .slice(range_search)
+                .to_string()
+                .matches(',')
+                .count();
+
+            // If we have a method call eat the first param since it's just self.
+            if has_self {
+                commas += 1;
+            }
+
+            call_info.active_parameter = Some(commas);
+        }
+    }
+
+    Ok(Some(call_info))
+}
+
+enum FnCallNode<'a> {
+    CallExpr(&'a ast::CallExpr),
+    MethodCallExpr(&'a ast::MethodCallExpr),
+}
+
+impl<'a> FnCallNode<'a> {
+    pub fn with_node(syntax: &'a SyntaxNode, offset: TextUnit) -> Option<FnCallNode<'a>> {
+        if let Some(expr) = find_node_at_offset::<ast::CallExpr>(syntax, offset) {
+            return Some(FnCallNode::CallExpr(expr));
+        }
+        if let Some(expr) = find_node_at_offset::<ast::MethodCallExpr>(syntax, offset) {
+            return Some(FnCallNode::MethodCallExpr(expr));
+        }
+        None
+    }
+
+    pub fn name_ref(&self) -> Option<&'a ast::NameRef> {
+        match *self {
+            FnCallNode::CallExpr(call_expr) => Some(match call_expr.expr()?.kind() {
+                ast::ExprKind::PathExpr(path_expr) => path_expr.path()?.segment()?.name_ref()?,
+                _ => return None,
+            }),
+
+            FnCallNode::MethodCallExpr(call_expr) => call_expr
+                .syntax()
+                .children()
+                .filter_map(ast::NameRef::cast)
+                .nth(0),
+        }
+    }
+
+    pub fn arg_list(&self) -> Option<&'a ast::ArgList> {
+        match *self {
+            FnCallNode::CallExpr(expr) => expr.arg_list(),
+            FnCallNode::MethodCallExpr(expr) => expr.arg_list(),
+        }
+    }
+}
+
+impl CallInfo {
+    fn new(node: &ast::FnDef) -> Option<Self> {
+        let mut doc = None;
+
+        // Strip the body out for the label.
+        let mut label: String = if let Some(body) = node.body() {
+            let body_range = body.syntax().range();
+            let label: String = node
+                .syntax()
+                .children()
+                .filter(|child| !child.range().is_subrange(&body_range))
+                .map(|node| node.text().to_string())
+                .collect();
+            label
+        } else {
+            node.syntax().text().to_string()
+        };
+
+        if let Some((comment_range, docs)) = 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"));
+            }
+        }
+
+        Some(CallInfo {
+            parameters: param_list(node),
+            label: label.trim().to_owned(),
+            doc,
+            active_parameter: None,
+        })
+    }
+}
+
+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() {
+        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()),
+        );
+    }
+    res
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::mock_analysis::single_file_with_position;
+
+    fn call_info(text: &str) -> CallInfo {
+        let (analysis, position) = single_file_with_position(text);
+        analysis.call_info(position).unwrap().unwrap()
+    }
+
+    #[test]
+    fn test_fn_signature_two_args_first() {
+        let info = call_info(
+            r#"fn foo(x: u32, y: u32) -> u32 {x + y}
+fn bar() { foo(<|>3, ); }"#,
+        );
+
+        assert_eq!(info.parameters, vec!("x".to_string(), "y".to_string()));
+        assert_eq!(info.active_parameter, Some(0));
+    }
+
+    #[test]
+    fn test_fn_signature_two_args_second() {
+        let info = call_info(
+            r#"fn foo(x: u32, y: u32) -> u32 {x + y}
+fn bar() { foo(3, <|>); }"#,
+        );
+
+        assert_eq!(info.parameters, vec!("x".to_string(), "y".to_string()));
+        assert_eq!(info.active_parameter, Some(1));
+    }
+
+    #[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_eq!(info.active_parameter, None);
+    }
+
+    #[test]
+    fn test_fn_signature_for_method_self() {
+        let info = call_info(
+            r#"struct F;
+impl F {
+    pub fn new() -> F{
+        F{}
+    }
+
+    pub fn do_it(&self) {}
+}
+
+fn bar() {
+    let f : F = F::new();
+    f.do_it(<|>);
+}"#,
+        );
+
+        assert_eq!(info.parameters, vec!["&self".to_string()]);
+        assert_eq!(info.active_parameter, None);
+    }
+
+    #[test]
+    fn test_fn_signature_for_method_with_arg() {
+        let info = call_info(
+            r#"struct F;
+impl F {
+    pub fn new() -> F{
+        F{}
+    }
+
+    pub fn do_it(&self, x: i32) {}
+}
+
+fn bar() {
+    let f : F = F::new();
+    f.do_it(<|>);
+}"#,
+        );
+
+        assert_eq!(info.parameters, vec!["&self".to_string(), "x".to_string()]);
+        assert_eq!(info.active_parameter, Some(1));
+    }
+
+    #[test]
+    fn test_fn_signature_with_docs_simple() {
+        let info = call_info(
+            r#"
+/// test
+// non-doc-comment
+fn foo(j: u32) -> u32 {
+    j
+}
+
+fn bar() {
+    let _ = foo(<|>);
+}
+"#,
+        );
+
+        assert_eq!(info.parameters, vec!["j".to_string()]);
+        assert_eq!(info.active_parameter, Some(0));
+        assert_eq!(info.label, "fn foo(j: u32) -> u32".to_string());
+        assert_eq!(info.doc, Some("test".into()));
+    }
+
+    #[test]
+    fn test_fn_signature_with_docs() {
+        let info = call_info(
+            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!(info.parameters, vec!["x".to_string()]);
+        assert_eq!(info.active_parameter, Some(0));
+        assert_eq!(info.label, "pub fn add_one(x: i32) -> i32".to_string());
+        assert_eq!(
+            info.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 info = call_info(
+            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!(info.parameters, vec!["x".to_string()]);
+        assert_eq!(info.active_parameter, Some(0));
+        assert_eq!(info.label, "pub fn add_one(x: i32) -> i32".to_string());
+        assert_eq!(
+            info.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 info = call_info(
+            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!(
+            info.parameters,
+            vec!["&mut self".to_string(), "ctx".to_string()]
+        );
+        assert_eq!(info.active_parameter, Some(1));
+        assert_eq!(
+            info.doc,
+            Some(
+                r#"Method is called when writer finishes.
+
+By default this method stops actor's `Context`."#
+                    .into()
+            )
+        );
+    }
+
+}
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 98554dd4cd..b3f75fdbeb 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -3,13 +3,13 @@ use std::sync::Arc;
 use salsa::Database;
 
 use hir::{
-    self, FnSignatureInfo, Problem, source_binder,
+    self, Problem, source_binder,
 };
 use ra_db::{FilesDatabase, SourceRoot, SourceRootId, SyntaxDatabase};
 use ra_editor::{self, find_node_at_offset, assists, LocalEdit, Severity};
 use ra_syntax::{
-    SyntaxNode, TextRange, TextUnit, AstNode, SourceFile,
-    ast::{self, ArgListOwner, NameOwner},
+    TextRange, AstNode, SourceFile,
+    ast::{self, NameOwner},
     SyntaxKind::*,
 };
 
@@ -262,75 +262,6 @@ impl db::RootDatabase {
             .collect()
     }
 
-    pub(crate) fn resolve_callable(
-        &self,
-        position: FilePosition,
-    ) -> Cancelable<Option<(FnSignatureInfo, Option<usize>)>> {
-        let file = self.source_file(position.file_id);
-        let syntax = file.syntax();
-
-        // Find the calling expression and it's NameRef
-        let calling_node = ctry!(FnCallNode::with_node(syntax, position.offset));
-        let name_ref = ctry!(calling_node.name_ref());
-
-        // Resolve the function's NameRef (NOTE: this isn't entirely accurate).
-        let file_symbols = self.index_resolve(name_ref)?;
-        for symbol in file_symbols {
-            if symbol.ptr.kind() == FN_DEF {
-                let fn_file = self.source_file(symbol.file_id);
-                let fn_def = symbol.ptr.resolve(&fn_file);
-                let fn_def = ast::FnDef::cast(&fn_def).unwrap();
-                let descr = ctry!(source_binder::function_from_source(
-                    self,
-                    symbol.file_id,
-                    fn_def
-                )?);
-                if let Some(descriptor) = descr.signature_info(self) {
-                    // If we have a calling expression let's find which argument we are on
-                    let mut current_parameter = None;
-
-                    let num_params = descriptor.params.len();
-                    let has_self = fn_def.param_list().and_then(|l| l.self_param()).is_some();
-
-                    if num_params == 1 {
-                        if !has_self {
-                            current_parameter = Some(0);
-                        }
-                    } else if num_params > 1 {
-                        // Count how many parameters into the call we are.
-                        // TODO: This is best effort for now and should be fixed at some point.
-                        // It may be better to see where we are in the arg_list and then check
-                        // where offset is in that list (or beyond).
-                        // Revisit this after we get documentation comments in.
-                        if let Some(ref arg_list) = calling_node.arg_list() {
-                            let start = arg_list.syntax().range().start();
-
-                            let range_search = TextRange::from_to(start, position.offset);
-                            let mut commas: usize = arg_list
-                                .syntax()
-                                .text()
-                                .slice(range_search)
-                                .to_string()
-                                .matches(',')
-                                .count();
-
-                            // If we have a method call eat the first param since it's just self.
-                            if has_self {
-                                commas += 1;
-                            }
-
-                            current_parameter = Some(commas);
-                        }
-                    }
-
-                    return Ok(Some((descriptor, current_parameter)));
-                }
-            }
-        }
-
-        Ok(None)
-    }
-
     pub(crate) fn rename(
         &self,
         position: FilePosition,
@@ -375,42 +306,3 @@ impl SourceChange {
         }
     }
 }
-
-enum FnCallNode<'a> {
-    CallExpr(&'a ast::CallExpr),
-    MethodCallExpr(&'a ast::MethodCallExpr),
-}
-
-impl<'a> FnCallNode<'a> {
-    pub fn with_node(syntax: &'a SyntaxNode, offset: TextUnit) -> Option<FnCallNode<'a>> {
-        if let Some(expr) = find_node_at_offset::<ast::CallExpr>(syntax, offset) {
-            return Some(FnCallNode::CallExpr(expr));
-        }
-        if let Some(expr) = find_node_at_offset::<ast::MethodCallExpr>(syntax, offset) {
-            return Some(FnCallNode::MethodCallExpr(expr));
-        }
-        None
-    }
-
-    pub fn name_ref(&self) -> Option<&'a ast::NameRef> {
-        match *self {
-            FnCallNode::CallExpr(call_expr) => Some(match call_expr.expr()?.kind() {
-                ast::ExprKind::PathExpr(path_expr) => path_expr.path()?.segment()?.name_ref()?,
-                _ => return None,
-            }),
-
-            FnCallNode::MethodCallExpr(call_expr) => call_expr
-                .syntax()
-                .children()
-                .filter_map(ast::NameRef::cast)
-                .nth(0),
-        }
-    }
-
-    pub fn arg_list(&self) -> Option<&'a ast::ArgList> {
-        match *self {
-            FnCallNode::CallExpr(expr) => expr.arg_list(),
-            FnCallNode::MethodCallExpr(expr) => expr.arg_list(),
-        }
-    }
-}
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index ec400ffe23..771a349c8c 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -22,6 +22,7 @@ mod symbol_index;
 
 mod extend_selection;
 mod hover;
+mod call_info;
 mod syntax_highlighting;
 
 use std::{fmt, sync::Arc};
@@ -39,7 +40,6 @@ pub use crate::{
     completion::{CompletionItem, CompletionItemKind, InsertText},
     runnables::{Runnable, RunnableKind},
 };
-pub use hir::FnSignatureInfo;
 pub use ra_editor::{Fold, FoldKind, HighlightedRange, LineIndex, Severity, StructureNode};
 
 pub use ra_db::{
@@ -272,6 +272,14 @@ impl<T> RangeInfo<T> {
     }
 }
 
+#[derive(Debug)]
+pub struct CallInfo {
+    pub label: String,
+    pub doc: Option<String>,
+    pub parameters: Vec<String>,
+    pub active_parameter: Option<usize>,
+}
+
 /// `AnalysisHost` stores the current state of the world.
 #[derive(Debug, Default)]
 pub struct AnalysisHost {
@@ -391,6 +399,10 @@ impl Analysis {
     pub fn hover(&self, position: FilePosition) -> Cancelable<Option<RangeInfo<String>>> {
         hover::hover(&*self.db, position)
     }
+    /// Computes parameter information for the given call expression.
+    pub fn call_info(&self, position: FilePosition) -> Cancelable<Option<CallInfo>> {
+        call_info::call_info(&*self.db, position)
+    }
     /// Returns a `mod name;` declaration which created the current module.
     pub fn parent_module(&self, position: FilePosition) -> Cancelable<Vec<NavigationTarget>> {
         self.db.parent_module(position)
@@ -425,13 +437,6 @@ impl Analysis {
     pub fn diagnostics(&self, file_id: FileId) -> Cancelable<Vec<Diagnostic>> {
         self.db.diagnostics(file_id)
     }
-    /// Computes parameter information for the given call expression.
-    pub fn resolve_callable(
-        &self,
-        position: FilePosition,
-    ) -> Cancelable<Option<(FnSignatureInfo, Option<usize>)>> {
-        self.db.resolve_callable(position)
-    }
     /// Computes the type of the expression at the given position.
     pub fn type_of(&self, frange: FileRange) -> Cancelable<Option<String>> {
         hover::type_of(&*self.db, frange)
diff --git a/crates/ra_hir/src/function.rs b/crates/ra_hir/src/function.rs
index 81b790c5f9..2cfc4caa42 100644
--- a/crates/ra_hir/src/function.rs
+++ b/crates/ra_hir/src/function.rs
@@ -1,14 +1,11 @@
 mod scope;
 
-use std::{
-    cmp::{max, min},
-    sync::Arc,
-};
+use std::sync::Arc;
 
 use ra_db::Cancelable;
 use ra_syntax::{
-    TextRange, TextUnit, TreePtr,
-    ast::{self, AstNode, DocCommentsOwner, NameOwner},
+    TreePtr,
+    ast::{self, AstNode},
 };
 
 use crate::{DefId, DefKind, HirDatabase, ty::InferenceResult, Module, Crate, impl_block::ImplBlock, expr::{Body, BodySyntaxMapping}, type_ref::{TypeRef, Mutability}, Name};
@@ -57,11 +54,6 @@ impl Function {
         db.fn_signature(self.def_id)
     }
 
-    pub fn signature_info(&self, db: &impl HirDatabase) -> Option<FnSignatureInfo> {
-        let syntax = self.syntax(db);
-        FnSignatureInfo::new(&syntax)
-    }
-
     pub fn infer(&self, db: &impl HirDatabase) -> Cancelable<Arc<InferenceResult>> {
         db.infer(self.def_id)
     }
@@ -132,116 +124,3 @@ pub(crate) fn fn_signature(db: &impl HirDatabase, def_id: DefId) -> Arc<FnSignat
     let sig = FnSignature { args, ret_type };
     Arc::new(sig)
 }
-
-#[derive(Debug, Clone)]
-pub struct FnSignatureInfo {
-    pub name: String,
-    pub label: String,
-    pub ret_type: Option<String>,
-    pub params: Vec<String>,
-    pub doc: Option<String>,
-}
-
-impl FnSignatureInfo {
-    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 mut label: String = if let Some(body) = node.body() {
-            let body_range = body.syntax().range();
-            let label: String = node
-                .syntax()
-                .children()
-                .filter(|child| !child.range().is_subrange(&body_range))
-                .map(|node| node.text().to_string())
-                .collect();
-            label
-        } else {
-            node.syntax().text().to_string()
-        };
-
-        if let Some((comment_range, docs)) = FnSignatureInfo::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 = FnSignatureInfo::param_list(node);
-        let ret_type = node.ret_type().map(|r| r.syntax().text().to_string());
-
-        Some(FnSignatureInfo {
-            name,
-            ret_type,
-            params,
-            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() {
-            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()),
-            );
-        }
-        res
-    }
-}
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index f8ac28cf70..197d8c4fd6 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -53,8 +53,6 @@ pub use self::{
     impl_block::{ImplBlock, ImplItem},
 };
 
-pub use self::function::FnSignatureInfo;
-
 pub use self::code_model_api::{
     Crate, CrateDependency,
     Module, ModuleSource, Problem,
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 99f15354f8..b9b42f1b39 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -475,36 +475,30 @@ pub fn handle_signature_help(
     params: req::TextDocumentPositionParams,
 ) -> Result<Option<req::SignatureHelp>> {
     let position = params.try_conv_with(&world)?;
-
-    if let Some((descriptor, active_param)) = world.analysis().resolve_callable(position)? {
-        let parameters: Vec<ParameterInformation> = descriptor
-            .params
-            .iter()
+    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 documentation = if let Some(doc) = descriptor.doc {
-            Some(Documentation::MarkupContent(MarkupContent {
+        let documentation = call_info.doc.map(|value| {
+            Documentation::MarkupContent(MarkupContent {
                 kind: MarkupKind::Markdown,
-                value: doc,
-            }))
-        } else {
-            None
-        };
-
+                value,
+            })
+        });
         let sig_info = SignatureInformation {
-            label: descriptor.label,
+            label: call_info.label,
             documentation,
             parameters: Some(parameters),
         };
-
         Ok(Some(req::SignatureHelp {
             signatures: vec![sig_info],
             active_signature: Some(0),
-            active_parameter: active_param.map(|a| a as u64),
+            active_parameter: call_info.active_parameter.map(|it| it as u64),
         }))
     } else {
         Ok(None)


Test patch:
diff --git a/crates/ra_analysis/tests/test/main.rs b/crates/ra_analysis/tests/test/main.rs
index 1f70af12aa..2c0735cb50 100644
--- a/crates/ra_analysis/tests/test/main.rs
+++ b/crates/ra_analysis/tests/test/main.rs
@@ -5,14 +5,9 @@ use test_utils::{assert_eq_dbg, assert_eq_text};
 
 use ra_analysis::{
     mock_analysis::{analysis_and_position, single_file, single_file_with_position, MockAnalysis},
-    AnalysisChange, CrateGraph, FileId, FnSignatureInfo, Query
+    AnalysisChange, CrateGraph, FileId, Query
 };
 
-fn get_signature(text: &str) -> (FnSignatureInfo, Option<usize>) {
-    let (analysis, position) = single_file_with_position(text);
-    analysis.resolve_callable(position).unwrap().unwrap()
-}
-
 #[test]
 fn test_unresolved_module_diagnostic() {
     let (analysis, file_id) = single_file("mod foo;");
@@ -99,260 +94,6 @@ fn test_resolve_crate_root() {
     assert_eq!(host.analysis().crate_for(mod_file).unwrap(), vec![crate_id]);
 }
 
-#[test]
-fn test_fn_signature_two_args_first() {
-    let (desc, param) = get_signature(
-        r#"fn foo(x: u32, y: u32) -> u32 {x + y}
-fn bar() { foo(<|>3, ); }"#,
-    );
-
-    assert_eq!(desc.name, "foo".to_string());
-    assert_eq!(desc.params, vec!("x".to_string(), "y".to_string()));
-    assert_eq!(desc.ret_type, Some("-> u32".into()));
-    assert_eq!(param, Some(0));
-}
-
-#[test]
-fn test_fn_signature_two_args_second() {
-    let (desc, param) = get_signature(
-        r#"fn foo(x: u32, y: u32) -> u32 {x + y}
-fn bar() { foo(3, <|>); }"#,
-    );
-
-    assert_eq!(desc.name, "foo".to_string());
-    assert_eq!(desc.params, vec!("x".to_string(), "y".to_string()));
-    assert_eq!(desc.ret_type, Some("-> u32".into()));
-    assert_eq!(param, Some(1));
-}
-
-#[test]
-fn test_fn_signature_for_impl() {
-    let (desc, param) = get_signature(
-        r#"struct F; impl F { pub fn new() { F{}} }
-fn bar() {let _ : F = F::new(<|>);}"#,
-    );
-
-    assert_eq!(desc.name, "new".to_string());
-    assert_eq!(desc.params, Vec::<String>::new());
-    assert_eq!(desc.ret_type, None);
-    assert_eq!(param, None);
-}
-
-#[test]
-fn test_fn_signature_for_method_self() {
-    let (desc, param) = get_signature(
-        r#"struct F;
-impl F {
-    pub fn new() -> F{
-        F{}
-    }
-
-    pub fn do_it(&self) {}
-}
-
-fn bar() {
-    let f : F = F::new();
-    f.do_it(<|>);
-}"#,
-    );
-
-    assert_eq!(desc.name, "do_it".to_string());
-    assert_eq!(desc.params, vec!["&self".to_string()]);
-    assert_eq!(desc.ret_type, None);
-    assert_eq!(param, None);
-}
-
-#[test]
-fn test_fn_signature_for_method_with_arg() {
-    let (desc, param) = get_signature(
-        r#"struct F;
-impl F {
-    pub fn new() -> F{
-        F{}
-    }
-
-    pub fn do_it(&self, x: i32) {}
-}
-
-fn bar() {
-    let f : F = F::new();
-    f.do_it(<|>);
-}"#,
-    );
-
-    assert_eq!(desc.name, "do_it".to_string());
-    assert_eq!(desc.params, vec!["&self".to_string(), "x".to_string()]);
-    assert_eq!(desc.ret_type, None);
-    assert_eq!(param, Some(1));
-}
-
-#[test]
-fn test_fn_signature_with_docs_simple() {
-    let (desc, param) = get_signature(
-        r#"
-/// test
-// non-doc-comment
-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).unwrap()