From c9909f42ba4adf55b1e73e7118b48f1b10c80ac6 Mon Sep 17 00:00:00 2001 From: "Jeremy A. Kolb" Date: Fri, 12 Oct 2018 07:54:57 -0400 Subject: [PATCH] A FnDescriptor shouldn't exist without a name --- crates/ra_analysis/src/descriptors.rs | 12 ++--- crates/ra_analysis/src/imp.rs | 68 +++++++++++++-------------- crates/ra_analysis/tests/tests.rs | 10 ++-- 3 files changed, 44 insertions(+), 46 deletions(-) diff --git a/crates/ra_analysis/src/descriptors.rs b/crates/ra_analysis/src/descriptors.rs index 4dcac1aa2f..faf945a41f 100644 --- a/crates/ra_analysis/src/descriptors.rs +++ b/crates/ra_analysis/src/descriptors.rs @@ -4,7 +4,7 @@ use std::{ use relative_path::RelativePathBuf; use ra_syntax::{ SmolStr, - ast::{self, NameOwner, AstNode, TypeParamsOwner}, + ast::{self, NameOwner, AstNode}, text_utils::is_subrange }; use { @@ -222,15 +222,15 @@ fn resolve_submodule( #[derive(Debug, Clone)] pub struct FnDescriptor { - pub name: Option, + pub name: String, pub label : String, pub ret_type: Option, pub params: Vec, } impl FnDescriptor { - pub fn new(node: ast::FnDef) -> Self { - let name = node.name().map(|name| name.text().to_string()); + pub fn new(node: ast::FnDef) -> Option { + let name = node.name()?.text().to_string(); // Strip the body out for the label. let label : String = if let Some(body) = node.body() { @@ -247,12 +247,12 @@ impl FnDescriptor { let params = FnDescriptor::param_list(node); let ret_type = node.ret_type().map(|r| r.syntax().text().to_string()); - FnDescriptor { + Some(FnDescriptor { name, ret_type, params, label - } + }) } fn param_list(node: ast::FnDef) -> Vec { diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs index 9e3ae2b037..aad54b9771 100644 --- a/crates/ra_analysis/src/imp.rs +++ b/crates/ra_analysis/src/imp.rs @@ -321,47 +321,45 @@ impl AnalysisImpl { for (_, fs) in file_symbols { if fs.kind == FN_DEF { if let Some(fn_def) = find_node_at_offset(syntax, fs.node_range.start()) { - let descriptor = FnDescriptor::new(fn_def); + 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; - // 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(); - 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; + 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(); - current_parameter = Some(commas); + 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)); + return Some((descriptor, current_parameter)); + } } } } diff --git a/crates/ra_analysis/tests/tests.rs b/crates/ra_analysis/tests/tests.rs index 9417ddc1d6..755640fb41 100644 --- a/crates/ra_analysis/tests/tests.rs +++ b/crates/ra_analysis/tests/tests.rs @@ -164,7 +164,7 @@ fn test_fn_signature_two_args_first() { r#"fn foo(x: u32, y: u32) -> u32 {x + y} fn bar() { foo(<|>3, ); }"#); - assert_eq!(desc.name, Some("foo".into())); + 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)); @@ -176,7 +176,7 @@ fn test_fn_signature_two_args_second() { r#"fn foo(x: u32, y: u32) -> u32 {x + y} fn bar() { foo(3, <|>); }"#); - assert_eq!(desc.name, Some("foo".into())); + 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)); @@ -188,7 +188,7 @@ fn test_fn_signature_for_impl() { r#"struct F; impl F { pub fn new() { F{}} } fn bar() {let _ : F = F::new(<|>);}"#); - assert_eq!(desc.name, Some("new".into())); + assert_eq!(desc.name, "new".to_string()); assert_eq!(desc.params, Vec::::new()); assert_eq!(desc.ret_type, None); assert_eq!(param, None); @@ -211,7 +211,7 @@ fn bar() { f.do_it(<|>); }"#); - assert_eq!(desc.name, Some("do_it".into())); + 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); @@ -234,7 +234,7 @@ fn bar() { f.do_it(<|>); }"#); - assert_eq!(desc.name, Some("do_it".into())); + 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));