Repository: racer-rust/racer (language: rust)

Issue:
Find definition in use statement can jump into closure arg
Awesome bug (⦿＿⦿)
```rust
use reqwest;
fn f() {
    let a = Some(5).map(|reqwest| {});
}
```
When we use `definition` on `use req~`, it returns  a closure arg below.
It's terrible :fearful: 


Discussion / hints:
Hi! Can I pick this? My idea to solve this is to not consider matches from closures if the search string location is not in the closure's scope. From a little debugging I found that the `extern crate reqwest` also matches but only the first result is picked because this context is an exact match.
> to not consider matches from closures if the search string is not in the closure's scope

Agreed, we should do so.
But the problem is **racer is intended to do so**.
So there should be a bug around scope iterating.
Okay, I'll try to find the root cause. As I understand, _children_scopes (is that the correct word?)  shouldn't be even considered to search matches from. Is that correct?
> Is that correct?

Yes. 
In [resolve_name](https://github.com/racer-rust/racer/blob/3be80e8646f5880c57a17fc3fa9f6cd84676a796/src/racer/nameres.rs#L1414), first [search_local_scopes](https://github.com/racer-rust/racer/blob/3be80e8646f5880c57a17fc3fa9f6cd84676a796/src/racer/nameres.rs#L1258) is called, and then [search_crate_root](https://github.com/racer-rust/racer/blob/3be80e8646f5880c57a17fc3fa9f6cd84676a796/src/racer/nameres.rs#L850) is called.
And I think there's a bug in search_local_scopes.
@kngwyu Why is the whole scope searched if `pos == BytePos::ZERO` here?
https://github.com/racer-rust/racer/blob/master/src/racer/nameres.rs#L1277

@letmutx 
It's just a short-cut.
If `pos != 0` we search scopes recursively.
e.g.
```rust
mod mo {
    fn mod func() {}
    fn f() {
        // headers
        let a = 5;
        let b = 4;
        let c = ~ // completion point
        // names
        fn inner_func1() {}
        struct 
    }
}
```
If we throw a completion query at `let c = ~`,
1. search for functions/structs/... in `fn f()`
2. search local variable above `let c=`, in `fn f()` (by `search_scope_headers`)
3. search for functions/structs/... in `mod m`
4. search local variable above `let c=`, in `mod` (this is inefficient and we should fix this)

But if `pos == 0` we don't have to call `search_scope_headers`, so just call `search_scope`.
`pos == 0` is often used by finding names in external mod/crate.


Fix patch:
diff --git a/src/racer/nameres.rs b/src/racer/nameres.rs
index 0db2cd9f..6234b76d 100644
--- a/src/racer/nameres.rs
+++ b/src/racer/nameres.rs
@@ -13,7 +13,7 @@ use fileres::{get_crate_file, get_module_file, get_std_file};
 use matchers::{find_doc, ImportInfo, MatchCxt};
 use primitive;
 use util::{
-    self, calculate_str_hash, closure_valid_arg_scope, find_ident_end, get_rust_src_path,
+    self, calculate_str_hash, find_ident_end, get_rust_src_path,
     strip_words, symbol_matches, trim_visibility, txt_matches,
 };
 use {ast, core, matchers, scopes, typeinf};
@@ -647,7 +647,7 @@ fn search_scope_headers(
         }
         return out;
     } else if let Some(vec) =
-        search_closure_args(search_str, preblock, stmtstart, filepath, search_type)
+        search_closure_args(search_str, preblock, stmtstart, point - stmtstart, filepath, search_type)
     {
         return vec;
     }
@@ -1128,7 +1128,7 @@ pub fn search_scope(
         }
     }
 
-    if let Some(vec) = search_closure_args(search_str, &scopesrc[0..], start, filepath, search_type)
+    if let Some(vec) = search_closure_args(search_str, &scopesrc[0..], start, point - start, filepath, search_type)
     {
         for mat in vec {
             out.push(mat)
@@ -1147,6 +1147,7 @@ fn search_closure_args(
     search_str: &str,
     scope_src: &str,
     scope_src_pos: BytePos,
+    point: BytePos,
     filepath: &Path,
     search_type: SearchType,
 ) -> Option<Vec<Match>> {
@@ -1160,7 +1161,12 @@ fn search_closure_args(
         scope_src.len()
     );
 
-    if let Some((pipe_range, pipe_str)) = closure_valid_arg_scope(scope_src) {
+    if let Some((pipe_range, body_range)) = util::find_closure(scope_src) {
+        let pipe_str =  &scope_src[pipe_range.to_range()];
+        if point < pipe_range.start || point > body_range.end {
+            return None;
+        }
+
         debug!(
             "search_closure_args found valid closure arg scope: {}",
             pipe_str
diff --git a/src/racer/util.rs b/src/racer/util.rs
index 3752bda6..f61d83e4 100644
--- a/src/racer/util.rs
+++ b/src/racer/util.rs
@@ -79,6 +79,85 @@ pub fn symbol_matches(stype: SearchType, searchstr: &str, candidate: &str) -> bo
     }
 }
 
+pub fn find_closure(src: &str) -> Option<(ByteRange, ByteRange)> {
+    let (pipe_range, _) = closure_valid_arg_scope(src)?;
+    let mut chars = src.chars()
+                       .enumerate()
+                       .skip(pipe_range.end.0)
+                       .skip_while(|(_, c)| {
+                           c.is_whitespace()
+                       });
+    let (start, start_char) = chars.next().map(|(i, c)| (if c == '{' { i + 1 } else { i }, c))?;
+
+    let mut clevel = if start_char == '{' { 1 } else { 0 };
+    let mut plevel = 0;
+
+    let mut last = None;
+    for (i, current) in chars {
+        match current {
+            '{' => clevel += 1,
+            '(' => plevel += 1,
+            '}' => {
+                clevel -= 1;
+                if (clevel == 0 && start_char == '{') || (clevel == -1) {
+                    last = Some(i);
+                    break;
+                }
+            },
+            ';' => {
+                if start_char != '{' {
+                    last = Some(i);
+                    break;
+                }
+            },
+            ')' => {
+                plevel -= 1;
+                if plevel == 0 {
+                    last = Some(i + 1);
+                }
+                if plevel == -1 {
+                    last = Some(i + 1);
+                    break;
+                }
+            },
+            _ => {}
+        }
+    }
+    if let Some(last) = last {
+        Some((pipe_range, ByteRange::new(BytePos(start), BytePos(last))))
+    } else {
+        None
+    }
+}
+
+#[test]
+fn test_find_closure() {
+    let src = "|a, b, c| something()";
+    let src2 = "|a, b, c| { something() }";
+    let src3 = "let a = |a, b, c|something();";
+    let src4 = "let a = |a, b, c| something().second().third();";
+    let src5 = "| x: i32 | y.map(|z| z~)";
+    let src6 = "| x: i32 | Struct { x };";
+    let src7 = "y.map(| x: i32 | y.map(|z| z) )";
+    let src8 = "|z| z)";
+    let src9 = "let p = |z| something() + 5;";
+    let get_range = |a, b| ByteRange::new(BytePos(a as usize), BytePos(b as usize));
+    let find = |src: &str, a, off1: i32, b, off2: i32| {
+        get_range(src.find(a).unwrap() as i32 + off1, src.rfind(b).unwrap() as i32 + 1 + off2)
+    };
+    let get_pipe = |src| find(src, '|', 0, '|', 0);
+
+    assert_eq!(Some((get_pipe(src), find(src, 's', 0, ')', 0))), find_closure(src));
+    assert_eq!(Some((get_pipe(src2), find(src2, '{', 1, '}', -1))), find_closure(src2));
+    assert_eq!(Some((get_pipe(src3), find(src3, 's', 0, ')', 0))), find_closure(src3));
+    assert_eq!(Some((get_pipe(src4), find(src4, 's', 0, ')', 0))), find_closure(src4));
+    assert_eq!(Some((find(src5, '|', 0, 'y', -2), find(src5, 'y', 0, ')', 0))), find_closure(src5));
+    assert_eq!(Some((get_pipe(src6), find(src6, 'S', 0, ';', -1))), find_closure(src6));
+    assert_eq!(Some((find(src7, '|', 0, 'y', -2), find(src7, '2', 4, ')', 0))), find_closure(src7));
+    assert_eq!(Some((get_pipe(src8), find(src8, ' ', 1, ')', 0))), find_closure(src8));
+    assert_eq!(Some((get_pipe(src9), find(src9, 's', 0, '5', 0))), find_closure(src9));
+}
+
 /// Try to valid if the given scope contains a valid closure arg scope.
 pub fn closure_valid_arg_scope(scope_src: &str) -> Option<(ByteRange, &str)> {
     // Try to find the left and right pipe, if one or both are not present, this is not a valid


Test patch:
diff --git a/tests/system.rs b/tests/system.rs
index ab56bb72..88565c2f 100644
--- a/tests/system.rs
+++ b/tests/system.rs
@@ -4051,3 +4051,15 @@ fn completes_functions_from_trait_objects() {
     let got = get_all_completions(src, None);
     assert!(got.into_iter().any(|ma| ma.matchstr == "foo"));
 }
+
+#[test]
+fn import_stmt_doesnt_jump_to_closure_arg() {
+    let src = r#"
+    use req~west;
+    fn main() {
+        let y = Some(32i32).map(|reqwest| "something");
+    }
+    "#;
+
+    assert!(find_definition(src, None).is_none());
+}
diff --git a/testutils/src/lib.rs b/testutils/src/lib.rs
index 94bb98d7..842d39db 100644
--- a/testutils/src/lib.rs
+++ b/testutils/src/lib.rs
@@ -1,7 +1,7 @@
 //! system test utilities for racer
 extern crate racer;
 extern crate tempfile;
-use racer::{complete_from_file, find_definition, BytePos, Match};
+use racer::{complete_from_file, find_definition as racer_find_definition, BytePos, Match};
 use std::fmt;
 use std::fs;
 use std::io::{self, Write};
@@ -211,11 +211,19 @@ pub fn get_only_completion(src: &str, dir: Option<TmpDir>) -> Match {
 /// Return the definition for the given source.
 ///
 /// The point to find the definition at must be marked with '~'.
+///
+/// # Panics
+/// Panics if there are no matches
 pub fn get_definition(src: &str, dir: Option<TmpDir>) -> Match {
+    find_definition(src, dir).unwrap()
+}
+
+/// Optionally returns a defintion for the given source
+pub fn find_definition(src: &str, dir: Option<TmpDir>) -> Option<Match> {
     let dir = dir.unwrap_or_else(|| TmpDir::new());
     let (completion_point, clean_src) = get_pos_and_source(src);
     let path = dir.write_file("src.rs", &clean_src);
     let cache = racer::FileCache::default();
     let session = racer::Session::new(&cache);
-    find_definition(&path, completion_point, &session).unwrap()
+    racer_find_definition(&path, completion_point, &session)
 }