Merge #110

110: Signature help r=matklad a=kjeremy

@matklad Once this is in shape I would like to add tests. I think a separate PR should be done for returning documentation information and markdown.

Fixes #102 

Co-authored-by: Jeremy A. Kolb <[email protected]>

10 files changed, 315 insertions, 12 deletions

diff --git a/crates/ra_analysis/src/descriptors.rs b/crates/ra_analysis/src/descriptors.rs
index 0731b5572..faf945a41 100644
--- a/crates/ra_analysis/src/descriptors.rs
+++ b/crates/ra_analysis/src/descriptors.rs
@@ -4,7 +4,8 @@ use std::{
 use relative_path::RelativePathBuf;
 use ra_syntax::{
     SmolStr,
-    ast::{self, NameOwner},
+    ast::{self, NameOwner, AstNode},
+    text_utils::is_subrange
 };
 use {
     FileId,
@@ -218,3 +219,56 @@ fn resolve_submodule(
     }
     (points_to, problem)
 }
+
+#[derive(Debug, Clone)]
+pub struct FnDescriptor {
+    pub name: String,
+    pub label : String,
+    pub ret_type: Option<String>,
+    pub params: Vec<String>,
+}
+
+impl FnDescriptor {
+    pub fn new(node: ast::FnDef) -> Option<Self> {
+        let name = node.name()?.text().to_string();
+
+        // Strip the body out for the label.
+        let label : String = if let Some(body) = node.body() {
+            let body_range = body.syntax().range();
+            let label : String = node.syntax().children()
+                .filter(|child| !is_subrange(body_range, child.range()))
+                .map(|node| node.text().to_string())
+                .collect();
+            label
+        } else {
+            node.syntax().text().to_string()
+        };
+
+        let params = FnDescriptor::param_list(node);
+        let ret_type = node.ret_type().map(|r| r.syntax().text().to_string());
+
+        Some(FnDescriptor {
+            name,
+            ret_type,
+            params,
+            label
+        })
+    }
+
+    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
+    }
+}
\ No newline at end of file
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 47bc0032b..aad54b977 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -12,19 +12,18 @@ use relative_path::RelativePath;
 use rustc_hash::FxHashSet;
 use ra_editor::{self, FileSymbol, LineIndex, find_node_at_offset, LocalEdit, resolve_local_name};
 use ra_syntax::{
-    TextUnit, TextRange, SmolStr, File, AstNode,
+    TextUnit, TextRange, SmolStr, File, AstNode, SyntaxNodeRef,
     SyntaxKind::*,
-    ast::{self, NameOwner},
+    ast::{self, NameOwner, ArgListOwner, Expr},
 };
 
 use {
     FileId, FileResolver, Query, Diagnostic, SourceChange, SourceFileEdit, Position, FileSystemEdit,
     JobToken, CrateGraph, CrateId,
     roots::{SourceRoot, ReadonlySourceRoot, WritableSourceRoot},
-    descriptors::{ModuleTreeDescriptor, Problem},
+    descriptors::{FnDescriptor, ModuleTreeDescriptor, Problem},
 };
 
-
 #[derive(Clone, Debug)]
 pub(crate) struct FileResolverImp {
     inner: Arc<FileResolver>
@@ -306,6 +305,68 @@ impl AnalysisImpl {
             .collect()
     }
 
+    pub fn resolve_callable(&self, file_id: FileId, offset: TextUnit, token: &JobToken)
+        -> Option<(FnDescriptor, Option<usize>)> {
+
+        let root = self.root(file_id);
+        let file = root.syntax(file_id);
+        let syntax = file.syntax();
+
+        // Find the calling expression and it's NameRef
+        let calling_node = FnCallNode::with_node(syntax, offset)?;
+        let name_ref = calling_node.name_ref()?;
+
+        // Resolve the function's NameRef (NOTE: this isn't entirely accurate).
+        let file_symbols = self.index_resolve(name_ref, token);
+        for (_, fs) in file_symbols {
+            if fs.kind == FN_DEF {
+                if let Some(fn_def) = find_node_at_offset(syntax, fs.node_range.start()) {
+                    if let Some(descriptor) = FnDescriptor::new(fn_def) {
+                        // 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(1);
+                            }
+                        } 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, 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 = commas + 1;
+                                }
+
+                                current_parameter = Some(commas);
+                            }
+                        }
+
+                        return Some((descriptor, current_parameter));
+                    }
+                }
+            }
+        }
+
+        None
+    }
+
     fn index_resolve(&self, name_ref: ast::NameRef, token: &JobToken) -> Vec<(FileId, FileSymbol)> {
         let name = name_ref.text();
         let mut query = Query::new(name.to_string());
@@ -355,3 +416,46 @@ impl CrateGraph {
         Some(crate_id)
     }
 }
+
+enum FnCallNode<'a> {
+    CallExpr(ast::CallExpr<'a>),
+    MethodCallExpr(ast::MethodCallExpr<'a>)
+}
+
+impl<'a> FnCallNode<'a> {
+    pub fn with_node(syntax: SyntaxNodeRef, offset: TextUnit) -> Option<FnCallNode> {
+        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<ast::NameRef> {
+        match *self {
+            FnCallNode::CallExpr(call_expr) => {
+                Some(match call_expr.expr()? {
+                    Expr::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<ast::ArgList> {
+        match *self {
+            FnCallNode::CallExpr(expr) => expr.arg_list(),
+            FnCallNode::MethodCallExpr(expr) => expr.arg_list()
+        }
+    }
+}
\ No newline at end of file
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index 849fd93e4..1aca72ae0 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -38,6 +38,7 @@ pub use ra_editor::{
     Fold, FoldKind
 };
 pub use job::{JobToken, JobHandle};
+pub use descriptors::FnDescriptor;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct FileId(pub u32);
@@ -236,6 +237,11 @@ impl Analysis {
         let file = self.imp.file_syntax(file_id);
         ra_editor::folding_ranges(&file)
     }
+
+    pub fn resolve_callable(&self, file_id: FileId, offset: TextUnit, token: &JobToken)
+        -> Option<(FnDescriptor, Option<usize>)> {
+        self.imp.resolve_callable(file_id, offset, token)
+    }
 }
 
 #[derive(Debug)]
diff --git a/crates/ra_analysis/tests/tests.rs b/crates/ra_analysis/tests/tests.rs
index a886cd0ff..755640fb4 100644
--- a/crates/ra_analysis/tests/tests.rs
+++ b/crates/ra_analysis/tests/tests.rs
@@ -1,6 +1,8 @@
 extern crate relative_path;
 extern crate ra_analysis;
 extern crate rustc_hash;
+extern crate ra_editor;
+extern crate ra_syntax;
 extern crate test_utils;
 
 use std::{
@@ -9,8 +11,8 @@ use std::{
 
 use rustc_hash::FxHashMap;
 use relative_path::{RelativePath, RelativePathBuf};
-use ra_analysis::{Analysis, AnalysisHost, FileId, FileResolver, JobHandle, CrateGraph, CrateId};
-use test_utils::assert_eq_dbg;
+use ra_analysis::{Analysis, AnalysisHost, FileId, FileResolver, JobHandle, CrateGraph, CrateId, FnDescriptor};
+use test_utils::{assert_eq_dbg, extract_offset};
 
 #[derive(Debug)]
 struct FileMap(Vec<(FileId, RelativePathBuf)>);
@@ -39,7 +41,7 @@ impl FileResolver for FileMap {
     }
 }
 
-fn analysis_host(files: &'static [(&'static str, &'static str)]) -> AnalysisHost {
+fn analysis_host(files: &[(&str, &str)]) -> AnalysisHost {
     let mut host = AnalysisHost::new();
     let mut file_map = Vec::new();
     for (id, &(path, contents)) in files.iter().enumerate() {
@@ -53,10 +55,20 @@ fn analysis_host(files: &'static [(&'static str, &'static str)]) -> AnalysisHost
     host
 }
 
-fn analysis(files: &'static [(&'static str, &'static str)]) -> Analysis {
+fn analysis(files: &[(&str, &str)]) -> Analysis {
     analysis_host(files).analysis()
 }
 
+fn get_signature(text: &str) -> (FnDescriptor, Option<usize>) {
+    let (offset, code) = extract_offset(text);
+    let code = code.as_str();
+
+    let (_handle, token) = JobHandle::new();
+    let snap = analysis(&[("/lib.rs", code)]);
+
+    snap.resolve_callable(FileId(1), offset, &token).unwrap()
+}
+
 #[test]
 fn test_resolve_module() {
     let snap = analysis(&[
@@ -145,3 +157,85 @@ fn test_resolve_crate_root() {
         vec![CrateId(1)],
     );
 }
+
+#[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));
+}
\ No newline at end of file
diff --git a/crates/ra_lsp_server/src/caps.rs b/crates/ra_lsp_server/src/caps.rs
index 3c628f29c..5598ec75f 100644
--- a/crates/ra_lsp_server/src/caps.rs
+++ b/crates/ra_lsp_server/src/caps.rs
@@ -7,6 +7,7 @@ use languageserver_types::{
     TextDocumentSyncKind,
     ExecuteCommandOptions,
     CompletionOptions,
+    SignatureHelpOptions,
     DocumentOnTypeFormattingOptions,
 };
 
@@ -26,7 +27,9 @@ pub fn server_capabilities() -> ServerCapabilities {
             resolve_provider: None,
             trigger_characters: None,
         }),
-        signature_help_provider: None,
+        signature_help_provider: Some(SignatureHelpOptions {
+            trigger_characters: Some(vec!["(".to_string(), ",".to_string()])
+        }),
         definition_provider: Some(true),
         type_definition_provider: None,
         implementation_provider: None,
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index ab8be15e9..f65e2a889 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -411,6 +411,42 @@ pub fn handle_folding_range(
     Ok(res)
 }
 
+pub fn handle_signature_help(
+    world: ServerWorld,
+    params: req::TextDocumentPositionParams,
+    token: JobToken,
+) -> Result<Option<req::SignatureHelp>> {
+    use languageserver_types::{ParameterInformation, SignatureInformation};
+
+    let file_id = params.text_document.try_conv_with(&world)?;
+    let line_index = world.analysis().file_line_index(file_id);
+    let offset = params.position.conv_with(&line_index);
+
+    if let Some((descriptor, active_param)) = world.analysis().resolve_callable(file_id, offset, &token) {
+        let parameters : Vec<ParameterInformation> =
+            descriptor.params.iter().map(|param|
+                ParameterInformation {
+                    label: param.clone(),
+                    documentation: None
+                }
+            ).collect();
+
+        let sig_info = SignatureInformation {
+            label: descriptor.label,
+            documentation: None,
+            parameters: Some(parameters)
+        };
+
+        Ok(Some(req::SignatureHelp {
+            signatures: vec![sig_info],
+            active_signature: Some(0),
+            active_parameter: active_param.map(|a| a as u64)
+        }))
+    } else {
+        Ok(None)
+    }
+}
+
 pub fn handle_code_action(
     world: ServerWorld,
     params: req::CodeActionParams,
diff --git a/crates/ra_lsp_server/src/main_loop/mod.rs b/crates/ra_lsp_server/src/main_loop/mod.rs
index 402615e42..f4e7cfc33 100644
--- a/crates/ra_lsp_server/src/main_loop/mod.rs
+++ b/crates/ra_lsp_server/src/main_loop/mod.rs
@@ -255,6 +255,7 @@ fn on_request(
         .on::<req::Completion>(handlers::handle_completion)?
         .on::<req::CodeActionRequest>(handlers::handle_code_action)?
         .on::<req::FoldingRangeRequest>(handlers::handle_folding_range)?
+        .on::<req::SignatureHelpRequest>(handlers::handle_signature_help)?
         .finish();
     match req {
         Ok((id, handle)) => {
diff --git a/crates/ra_lsp_server/src/req.rs b/crates/ra_lsp_server/src/req.rs
index f80957589..1630edf7f 100644
--- a/crates/ra_lsp_server/src/req.rs
+++ b/crates/ra_lsp_server/src/req.rs
@@ -14,6 +14,7 @@ pub use languageserver_types::{
     CompletionParams, CompletionResponse,
     DocumentOnTypeFormattingParams,
     TextDocumentEdit,
+    SignatureHelp, Hover
 };
 
 pub enum SyntaxTree {}
diff --git a/crates/ra_syntax/src/ast/generated.rs b/crates/ra_syntax/src/ast/generated.rs
index ef7b5b1a1..48c9038dc 100644
--- a/crates/ra_syntax/src/ast/generated.rs
+++ b/crates/ra_syntax/src/ast/generated.rs
@@ -1387,7 +1387,11 @@ impl<'a> AstNode<'a> for PathExpr<'a> {
     fn syntax(self) -> SyntaxNodeRef<'a> { self.syntax }
 }
 
-impl<'a> PathExpr<'a> {}
+impl<'a> PathExpr<'a> {
+    pub fn path(self) -> Option<Path<'a>> {
+        super::child_opt(self)
+    }
+}
 
 // PathPat
 #[derive(Debug, Clone, Copy)]
diff --git a/crates/ra_syntax/src/grammar.ron b/crates/ra_syntax/src/grammar.ron
index 9da0c2c13..a904f7505 100644
--- a/crates/ra_syntax/src/grammar.ron
+++ b/crates/ra_syntax/src/grammar.ron
@@ -342,7 +342,7 @@ Grammar(
         "TupleExpr": (),
         "ArrayExpr": (),
         "ParenExpr": (),
-        "PathExpr": (),
+        "PathExpr": (options: ["Path"]),
         "LambdaExpr": (
             options: [
                 "ParamList",