8 files changed, 108 insertions, 98 deletions

diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index 3fb29950b..8d2ff561a 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -85,31 +85,32 @@ fn name_definition(
 
 #[cfg(test)]
 mod tests {
-    use test_utils::assert_eq_dbg;
     use crate::mock_analysis::analysis_and_position;
 
+    fn check_goto(fixuture: &str, expected: &str) {
+        let (analysis, pos) = analysis_and_position(fixuture);
+
+        let mut navs = analysis.goto_definition(pos).unwrap().unwrap().info;
+        assert_eq!(navs.len(), 1);
+        let nav = navs.pop().unwrap();
+        nav.assert_match(expected);
+    }
+
     #[test]
     fn goto_definition_works_in_items() {
-        let (analysis, pos) = analysis_and_position(
+        check_goto(
             "
             //- /lib.rs
             struct Foo;
             enum E { X(Foo<|>) }
             ",
-        );
-
-        let symbols = analysis.goto_definition(pos).unwrap().unwrap();
-        assert_eq_dbg(
-            r#"[NavigationTarget { file_id: FileId(1), name: "Foo",
-                                   kind: STRUCT_DEF, range: [0; 11),
-                                   ptr: Some(LocalSyntaxPtr { range: [0; 11), kind: STRUCT_DEF }) }]"#,
-            &symbols,
+            "Foo STRUCT_DEF FileId(1) [0; 11) [7; 10)",
         );
     }
 
     #[test]
     fn goto_definition_resolves_correct_name() {
-        let (analysis, pos) = analysis_and_position(
+        check_goto(
             "
             //- /lib.rs
             use a::Foo;
@@ -121,47 +122,30 @@ mod tests {
             //- /b.rs
             struct Foo;
             ",
-        );
-
-        let symbols = analysis.goto_definition(pos).unwrap().unwrap();
-        assert_eq_dbg(
-            r#"[NavigationTarget { file_id: FileId(2), name: "Foo",
-                                   kind: STRUCT_DEF, range: [0; 11),
-                                   ptr: Some(LocalSyntaxPtr { range: [0; 11), kind: STRUCT_DEF }) }]"#,
-            &symbols,
+            "Foo STRUCT_DEF FileId(2) [0; 11) [7; 10)",
         );
     }
 
     #[test]
     fn goto_definition_works_for_module_declaration() {
-        let (analysis, pos) = analysis_and_position(
+        check_goto(
             "
             //- /lib.rs
             mod <|>foo;
             //- /foo.rs
             // empty
-        ",
-        );
-
-        let symbols = analysis.goto_definition(pos).unwrap().unwrap();
-        assert_eq_dbg(
-            r#"[NavigationTarget { file_id: FileId(2), name: "foo", kind: MODULE, range: [0; 0), ptr: None }]"#,
-            &symbols,
+            ",
+            "foo SOURCE_FILE FileId(2) [0; 10)",
         );
 
-        let (analysis, pos) = analysis_and_position(
+        check_goto(
             "
             //- /lib.rs
             mod <|>foo;
             //- /foo/mod.rs
             // empty
-        ",
-        );
-
-        let symbols = analysis.goto_definition(pos).unwrap().unwrap();
-        assert_eq_dbg(
-            r#"[NavigationTarget { file_id: FileId(2), name: "foo", kind: MODULE, range: [0; 0), ptr: None }]"#,
-            &symbols,
+            ",
+            "foo SOURCE_FILE FileId(2) [0; 10)",
         );
     }
 }
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index 9b06a0e58..f544ffa6d 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -92,7 +92,7 @@ impl NavigationTarget {
         let source_file = source_file.syntax();
         let node = source_file
             .descendants()
-            .find(|node| node.kind() == self.kind() && node.range() == self.range())?
+            .find(|node| node.kind() == self.kind() && node.range() == self.full_range())?
             .to_owned();
         Some(node)
     }
diff --git a/crates/ra_ide_api/src/imp.rs b/crates/ra_ide_api/src/imp.rs
index 12bfe1761..ba4aa0fd5 100644
--- a/crates/ra_ide_api/src/imp.rs
+++ b/crates/ra_ide_api/src/imp.rs
@@ -15,7 +15,7 @@ use ra_syntax::{
 
 use crate::{
     AnalysisChange,
-    Cancelable, NavigationTarget,
+    Cancelable,
     CrateId, db, Diagnostic, FileId, FilePosition, FileRange, FileSystemEdit,
     Query, RootChange, SourceChange, SourceFileEdit,
     symbol_index::{LibrarySymbolsQuery, FileSymbol},
@@ -98,19 +98,6 @@ impl db::RootDatabase {
 }
 
 impl db::RootDatabase {
-    /// This returns `Vec` because a module may be included from several places. We
-    /// don't handle this case yet though, so the Vec has length at most one.
-    pub(crate) fn parent_module(
-        &self,
-        position: FilePosition,
-    ) -> Cancelable<Vec<NavigationTarget>> {
-        let module = match source_binder::module_from_position(self, position)? {
-            None => return Ok(Vec::new()),
-            Some(it) => it,
-        };
-        let nav = NavigationTarget::from_module(self, module)?;
-        Ok(vec![nav])
-    }
     /// Returns `Vec` for the same reason as `parent_module`
     pub(crate) fn crate_for(&self, file_id: FileId) -> Cancelable<Vec<CrateId>> {
         let module = match source_binder::module_from_file_id(self, file_id)? {
diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs
index 2b02dab2a..6155d903a 100644
--- a/crates/ra_ide_api/src/lib.rs
+++ b/crates/ra_ide_api/src/lib.rs
@@ -31,6 +31,7 @@ mod extend_selection;
 mod hover;
 mod call_info;
 mod syntax_highlighting;
+mod parent_module;
 
 use std::{fmt, sync::Arc};
 
@@ -414,7 +415,7 @@ impl Analysis {
 
     /// Returns a `mod name;` declaration which created the current module.
     pub fn parent_module(&self, position: FilePosition) -> Cancelable<Vec<NavigationTarget>> {
-        self.with_db(|db| db.parent_module(position))?
+        self.with_db(|db| parent_module::parent_module(db, position))?
     }
 
     /// Returns crates this file belongs too.
diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs
index 943e62eb8..8b29c3a97 100644
--- a/crates/ra_ide_api/src/navigation_target.rs
+++ b/crates/ra_ide_api/src/navigation_target.rs
@@ -17,7 +17,7 @@ pub struct NavigationTarget {
     file_id: FileId,
     name: SmolStr,
     kind: SyntaxKind,
-    range: TextRange,
+    full_range: TextRange,
     focus_range: Option<TextRange>,
     // Should be DefId ideally
     ptr: Option<LocalSyntaxPtr>,
@@ -36,11 +36,11 @@ impl NavigationTarget {
         self.file_id
     }
 
-    pub fn range(&self) -> TextRange {
-        self.range
+    pub fn full_range(&self) -> TextRange {
+        self.full_range
     }
 
-    /// A "most interesting" range withing the `range`.
+    /// A "most interesting" range withing the `range_full`.
     ///
     /// Typically, `range` is the whole syntax node, including doc comments, and
     /// `focus_range` is the range of the identifier.
@@ -53,7 +53,7 @@ impl NavigationTarget {
             file_id: symbol.file_id,
             name: symbol.name.clone(),
             kind: symbol.ptr.kind(),
-            range: symbol.ptr.range(),
+            full_range: symbol.ptr.range(),
             focus_range: None,
             ptr: Some(symbol.ptr.clone()),
         }
@@ -66,7 +66,7 @@ impl NavigationTarget {
         NavigationTarget {
             file_id,
             name: entry.name().to_string().into(),
-            range: entry.ptr().range(),
+            full_range: entry.ptr().range(),
             focus_range: None,
             kind: NAME,
             ptr: None,
@@ -118,6 +118,27 @@ impl NavigationTarget {
         Ok(Some(res))
     }
 
+    #[cfg(test)]
+    pub(crate) fn assert_match(&self, expected: &str) {
+        let actual = self.debug_render();
+        test_utils::assert_eq_text!(expected.trim(), actual.trim(),);
+    }
+
+    #[cfg(test)]
+    pub(crate) fn debug_render(&self) -> String {
+        let mut buf = format!(
+            "{} {:?} {:?} {:?}",
+            self.name(),
+            self.kind(),
+            self.file_id(),
+            self.full_range()
+        );
+        if let Some(focus_range) = self.focus_range() {
+            buf.push_str(&format!(" {:?}", focus_range))
+        }
+        buf
+    }
+
     fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget {
         let name = node.name().map(|it| it.text().clone()).unwrap_or_default();
         let focus_range = node.name().map(|it| it.syntax().range());
@@ -134,7 +155,7 @@ impl NavigationTarget {
             file_id,
             name,
             kind: node.kind(),
-            range: node.range(),
+            full_range: node.range(),
             focus_range,
             ptr: Some(LocalSyntaxPtr::new(node)),
         }
diff --git a/crates/ra_ide_api/src/parent_module.rs b/crates/ra_ide_api/src/parent_module.rs
new file mode 100644
index 000000000..d345839a3
--- /dev/null
+++ b/crates/ra_ide_api/src/parent_module.rs
@@ -0,0 +1,52 @@
+use ra_db::{Cancelable, FilePosition};
+
+use crate::{NavigationTarget, db::RootDatabase};
+
+/// This returns `Vec` because a module may be included from several places. We
+/// don't handle this case yet though, so the Vec has length at most one.
+pub(crate) fn parent_module(
+    db: &RootDatabase,
+    position: FilePosition,
+) -> Cancelable<Vec<NavigationTarget>> {
+    let module = match hir::source_binder::module_from_position(db, position)? {
+        None => return Ok(Vec::new()),
+        Some(it) => it,
+    };
+    let nav = NavigationTarget::from_module(db, module)?;
+    Ok(vec![nav])
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::mock_analysis::analysis_and_position;
+
+    #[test]
+    fn test_resolve_parent_module() {
+        let (analysis, pos) = analysis_and_position(
+            "
+            //- /lib.rs
+            mod foo;
+            //- /foo.rs
+            <|>// empty
+            ",
+        );
+        let nav = analysis.parent_module(pos).unwrap().pop().unwrap();
+        nav.assert_match("foo SOURCE_FILE FileId(2) [0; 10)");
+    }
+
+    #[test]
+    fn test_resolve_parent_module_for_inline() {
+        let (analysis, pos) = analysis_and_position(
+            "
+            //- /lib.rs
+            mod foo {
+                mod bar {
+                    mod baz { <|> }
+                }
+            }
+            ",
+        );
+        let nav = analysis.parent_module(pos).unwrap().pop().unwrap();
+        nav.assert_match("baz MODULE FileId(1) [32; 44)");
+    }
+}
diff --git a/crates/ra_ide_api/tests/test/main.rs b/crates/ra_ide_api/tests/test/main.rs
index d1dc07e5b..7dc1dba73 100644
--- a/crates/ra_ide_api/tests/test/main.rs
+++ b/crates/ra_ide_api/tests/test/main.rs
@@ -4,7 +4,7 @@ use ra_syntax::TextRange;
 use test_utils::{assert_eq_dbg, assert_eq_text};
 
 use ra_ide_api::{
-    mock_analysis::{analysis_and_position, single_file, single_file_with_position, MockAnalysis},
+    mock_analysis::{single_file, single_file_with_position, MockAnalysis},
     AnalysisChange, CrateGraph, FileId, Query
 };
 
@@ -35,42 +35,6 @@ fn test_unresolved_module_diagnostic_no_diag_for_inline_mode() {
 }
 
 #[test]
-fn test_resolve_parent_module() {
-    let (analysis, pos) = analysis_and_position(
-        "
-        //- /lib.rs
-        mod foo;
-        //- /foo.rs
-        <|>// empty
-    ",
-    );
-    let symbols = analysis.parent_module(pos).unwrap();
-    assert_eq_dbg(
-        r#"[NavigationTarget { file_id: FileId(1), name: "foo", kind: MODULE, range: [4; 7), ptr: None }]"#,
-        &symbols,
-    );
-}
-
-#[test]
-fn test_resolve_parent_module_for_inline() {
-    let (analysis, pos) = analysis_and_position(
-        "
-        //- /lib.rs
-        mod foo {
-            mod bar {
-                mod baz { <|> }
-            }
-        }
-    ",
-    );
-    let symbols = analysis.parent_module(pos).unwrap();
-    assert_eq_dbg(
-        r#"[NavigationTarget { file_id: FileId(1), name: "baz", kind: MODULE, range: [36; 39), ptr: None }]"#,
-        &symbols,
-    );
-}
-
-#[test]
 fn test_resolve_crate_root() {
     let mock = MockAnalysis::with_files(
         "
@@ -245,5 +209,5 @@ pub trait HirDatabase: SyntaxDatabase {}
     let mut symbols = analysis.symbol_search(Query::new("Hir".into())).unwrap();
     let s = symbols.pop().unwrap();
     assert_eq!(s.name(), "HirDatabase");
-    assert_eq!(s.range(), TextRange::from_to(33.into(), 44.into()));
+    assert_eq!(s.full_range(), TextRange::from_to(33.into(), 44.into()));
 }
diff --git a/crates/ra_lsp_server/src/conv.rs b/crates/ra_lsp_server/src/conv.rs
index aad698da1..76fa98cbe 100644
--- a/crates/ra_lsp_server/src/conv.rs
+++ b/crates/ra_lsp_server/src/conv.rs
@@ -345,7 +345,8 @@ impl TryConvWith for &NavigationTarget {
     type Output = Location;
     fn try_conv_with(self, world: &ServerWorld) -> Result<Location> {
         let line_index = world.analysis().file_line_index(self.file_id());
-        to_location(self.file_id(), self.range(), &world, &line_index)
+        let range = self.focus_range().unwrap_or(self.full_range());
+        to_location(self.file_id(), range, &world, &line_index)
     }
 }
 
@@ -361,7 +362,7 @@ pub fn to_location_link(
     let res = LocationLink {
         origin_selection_range: Some(target.range.conv_with(line_index)),
         target_uri: url.to_string(),
-        target_range: target.info.range().conv_with(&tgt_line_index),
+        target_range: target.info.full_range().conv_with(&tgt_line_index),
         target_selection_range: target
             .info
             .focus_range()