A FnDescriptor shouldn't exist without a name

3 files changed, 47 insertions, 49 deletions

diff --git a/crates/ra_analysis/src/descriptors.rs b/crates/ra_analysis/src/descriptors.rs
index 4dcac1aa2..faf945a41 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<String>,
+    pub name: String,
     pub label : String,
     pub ret_type: Option<String>,
     pub params: Vec<String>,
 }
 
 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<Self> {
+        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<String> {
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 9e3ae2b03..aad54b977 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 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;
+                    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);
                             }
-
-                            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 9417ddc1d..755640fb4 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::<String>::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));