Merge #5137

5137: Make gotodef tests more data-driven r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

9 files changed, 636 insertions, 737 deletions

diff --git a/crates/ra_db/src/lib.rs b/crates/ra_db/src/lib.rs
index 4a3ba57da..1ddacc1f6 100644
--- a/crates/ra_db/src/lib.rs
+++ b/crates/ra_db/src/lib.rs
@@ -80,7 +80,7 @@ pub struct FilePosition {
     pub offset: TextSize,
 }
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub struct FileRange {
     pub file_id: FileId,
     pub range: TextRange,
diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs
index 0481a7b12..fddf0604d 100644
--- a/crates/ra_hir_ty/src/test_db.rs
+++ b/crates/ra_hir_ty/src/test_db.rs
@@ -8,8 +8,10 @@ use std::{
 use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId, ModuleId};
 use hir_expand::{db::AstDatabase, diagnostics::DiagnosticSink};
 use ra_db::{salsa, CrateId, FileId, FileLoader, FileLoaderDelegate, SourceDatabase, Upcast};
-use rustc_hash::FxHashSet;
+use ra_syntax::TextRange;
+use rustc_hash::{FxHashMap, FxHashSet};
 use stdx::format_to;
+use test_utils::extract_annotations;
 
 use crate::{
     db::HirDatabase, diagnostics::Diagnostic, expr::ExprValidator,
@@ -155,17 +157,27 @@ impl TestDB {
         (buf, count)
     }
 
-    pub fn all_files(&self) -> Vec<FileId> {
-        let mut res = Vec::new();
+    pub fn extract_annotations(&self) -> FxHashMap<FileId, Vec<(TextRange, String)>> {
+        let mut files = Vec::new();
         let crate_graph = self.crate_graph();
         for krate in crate_graph.iter() {
             let crate_def_map = self.crate_def_map(krate);
             for (module_id, _) in crate_def_map.modules.iter() {
                 let file_id = crate_def_map[module_id].origin.file_id();
-                res.extend(file_id)
+                files.extend(file_id)
             }
         }
-        res
+        files
+            .into_iter()
+            .filter_map(|file_id| {
+                let text = self.file_text(file_id);
+                let annotations = extract_annotations(&text);
+                if annotations.is_empty() {
+                    return None;
+                }
+                Some((file_id, annotations))
+            })
+            .collect()
     }
 }
 
diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs
index 5424e6bb1..9084c3bed 100644
--- a/crates/ra_hir_ty/src/tests.rs
+++ b/crates/ra_hir_ty/src/tests.rs
@@ -28,7 +28,6 @@ use ra_syntax::{
     SyntaxNode,
 };
 use stdx::format_to;
-use test_utils::extract_annotations;
 
 use crate::{
     db::HirDatabase, display::HirDisplay, infer::TypeMismatch, test_db::TestDB, InferenceResult, Ty,
@@ -49,9 +48,7 @@ fn check_types_source_code(ra_fixture: &str) {
 fn check_types_impl(ra_fixture: &str, display_source: bool) {
     let db = TestDB::with_files(ra_fixture);
     let mut checked_one = false;
-    for file_id in db.all_files() {
-        let text = db.parse(file_id).syntax_node().to_string();
-        let annotations = extract_annotations(&text);
+    for (file_id, annotations) in db.extract_annotations() {
         for (range, expected) in annotations {
             let ty = type_at_range(&db, FileRange { file_id, range });
             let actual = if display_source {
diff --git a/crates/ra_ide/src/display/navigation_target.rs b/crates/ra_ide/src/display/navigation_target.rs
index 0b52b01ab..8bf2428ed 100644
--- a/crates/ra_ide/src/display/navigation_target.rs
+++ b/crates/ra_ide/src/display/navigation_target.rs
@@ -11,7 +11,7 @@ use ra_syntax::{
     TextRange,
 };
 
-use crate::{FileRange, FileSymbol};
+use crate::FileSymbol;
 
 use super::short_label::ShortLabel;
 
@@ -47,6 +47,19 @@ impl NavigationTarget {
     pub fn range(&self) -> TextRange {
         self.focus_range.unwrap_or(self.full_range)
     }
+    /// A "most interesting" range withing the `full_range`.
+    ///
+    /// Typically, `full_range` is the whole syntax node,
+    /// including doc comments, and `focus_range` is the range of the identifier.
+    pub fn focus_range(&self) -> Option<TextRange> {
+        self.focus_range
+    }
+    pub fn full_range(&self) -> TextRange {
+        self.full_range
+    }
+    pub fn file_id(&self) -> FileId {
+        self.file_id
+    }
 
     pub fn name(&self) -> &SmolStr {
         &self.name
@@ -60,18 +73,6 @@ impl NavigationTarget {
         self.kind
     }
 
-    pub fn file_id(&self) -> FileId {
-        self.file_id
-    }
-
-    pub fn file_range(&self) -> FileRange {
-        FileRange { file_id: self.file_id, range: self.full_range }
-    }
-
-    pub fn full_range(&self) -> TextRange {
-        self.full_range
-    }
-
     pub fn docs(&self) -> Option<&str> {
         self.docs.as_deref()
     }
@@ -80,14 +81,6 @@ impl NavigationTarget {
         self.description.as_deref()
     }
 
-    /// A "most interesting" range withing the `full_range`.
-    ///
-    /// Typically, `full_range` is the whole syntax node,
-    /// including doc comments, and `focus_range` is the range of the identifier.
-    pub fn focus_range(&self) -> Option<TextRange> {
-        self.focus_range
-    }
-
     pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget {
         let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default();
         if let Some(src) = module.declaration_source(db) {
@@ -278,16 +271,22 @@ impl ToNav for hir::Module {
 impl ToNav for hir::ImplDef {
     fn to_nav(&self, db: &RootDatabase) -> NavigationTarget {
         let src = self.source(db);
-        let frange = if let Some(item) = self.is_builtin_derive(db) {
+        let derive_attr = self.is_builtin_derive(db);
+        let frange = if let Some(item) = &derive_attr {
             original_range(db, item.syntax())
         } else {
             original_range(db, src.as_ref().map(|it| it.syntax()))
         };
+        let focus_range = if derive_attr.is_some() {
+            None
+        } else {
+            src.value.target_type().map(|ty| original_range(db, src.with_value(ty.syntax())).range)
+        };
 
         NavigationTarget::from_syntax(
             frange.file_id,
             "impl".into(),
-            None,
+            focus_range,
             frange.range,
             src.value.syntax().kind(),
         )
diff --git a/crates/ra_ide/src/goto_definition.rs b/crates/ra_ide/src/goto_definition.rs
index 969d5e0ff..4c78fa214 100644
--- a/crates/ra_ide/src/goto_definition.rs
+++ b/crates/ra_ide/src/goto_definition.rs
@@ -103,67 +103,32 @@ pub(crate) fn reference_definition(
 
 #[cfg(test)]
 mod tests {
-    use expect::{expect, Expect};
-    use test_utils::assert_eq_text;
-
-    use crate::mock_analysis::analysis_and_position;
-
-    fn check_goto(ra_fixture: &str, expected: &str, expected_range: &str) {
-        let (analysis, pos) = analysis_and_position(ra_fixture);
-
-        let mut navs = analysis.goto_definition(pos).unwrap().unwrap().info;
-        if navs.len() == 0 {
-            panic!("unresolved reference")
-        }
-        assert_eq!(navs.len(), 1);
-
-        let nav = navs.pop().unwrap();
-        let file_text = analysis.file_text(nav.file_id()).unwrap();
-
-        let mut actual = file_text[nav.full_range()].to_string();
-        if let Some(focus) = nav.focus_range() {
-            actual += "|";
-            actual += &file_text[focus];
-        }
-
-        if !expected_range.contains("...") {
-            test_utils::assert_eq_text!(&actual, expected_range);
-        } else {
-            let mut parts = expected_range.split("...");
-            let prefix = parts.next().unwrap();
-            let suffix = parts.next().unwrap();
-            assert!(
-                actual.starts_with(prefix) && actual.ends_with(suffix),
-                "\nExpected: {}\n Actual: {}\n",
-                expected_range,
-                actual
-            );
+    use ra_db::FileRange;
+    use ra_syntax::{TextRange, TextSize};
+
+    use crate::mock_analysis::MockAnalysis;
+
+    fn check(ra_fixture: &str) {
+        let (mock, position) = MockAnalysis::with_files_and_position(ra_fixture);
+        let (mut expected, data) = mock.annotation();
+        let analysis = mock.analysis();
+        match data.as_str() {
+            "" => (),
+            "file" => {
+                expected.range =
+                    TextRange::up_to(TextSize::of(&*analysis.file_text(expected.file_id).unwrap()))
+            }
+            data => panic!("bad data: {}", data),
         }
 
-        nav.assert_match(expected);
-    }
-
-    fn check(ra_fixture: &str, expect: Expect) {
-        let (analysis, pos) = analysis_and_position(ra_fixture);
-
-        let mut navs = analysis.goto_definition(pos).unwrap().unwrap().info;
+        let mut navs = analysis.goto_definition(position).unwrap().unwrap().info;
         if navs.len() == 0 {
             panic!("unresolved reference")
         }
         assert_eq!(navs.len(), 1);
 
         let nav = navs.pop().unwrap();
-        let file_text = analysis.file_text(nav.file_id()).unwrap();
-
-        let mut actual = nav.debug_render();
-        actual += "\n";
-        actual += &file_text[nav.full_range()].to_string();
-        if let Some(focus) = nav.focus_range() {
-            actual += "|";
-            actual += &file_text[focus];
-            actual += "\n";
-        }
-        expect.assert_eq(&actual);
+        assert_eq!(expected, FileRange { file_id: nav.file_id(), range: nav.range() });
     }
 
     #[test]
@@ -171,162 +136,116 @@ mod tests {
         check(
             r#"
 struct Foo;
+     //^^^
 enum E { X(Foo<|>) }
 "#,
-            expect![[r#"
-                Foo STRUCT_DEF FileId(1) 0..11 7..10
-                struct Foo;|Foo
-            "#]],
         );
     }
 
     #[test]
     fn goto_def_at_start_of_item() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            enum E { X(<|>Foo) }
-            ",
-            "Foo STRUCT_DEF FileId(1) 0..11 7..10",
-            "struct Foo;|Foo",
+        check(
+            r#"
+struct Foo;
+     //^^^
+enum E { X(<|>Foo) }
+"#,
         );
     }
 
     #[test]
     fn goto_definition_resolves_correct_name() {
-        check_goto(
-            "
-            //- /lib.rs
-            use a::Foo;
-            mod a;
-            mod b;
-            enum E { X(Foo<|>) }
-
-            //- /a.rs
-            struct Foo;
+        check(
+            r#"
+//- /lib.rs
+use a::Foo;
+mod a;
+mod b;
+enum E { X(Foo<|>) }
 
-            //- /b.rs
-            struct Foo;
-            ",
-            "Foo STRUCT_DEF FileId(2) 0..11 7..10",
-            "struct Foo;|Foo",
+//- /a.rs
+struct Foo;
+     //^^^
+//- /b.rs
+struct Foo;
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_module_declaration() {
-        check_goto(
+        check(
             r#"
 //- /lib.rs
 mod <|>foo;
 
 //- /foo.rs
 // empty
+//^ file
 "#,
-            "foo SOURCE_FILE FileId(2) 0..9",
-            "// empty\n",
         );
 
-        check_goto(
+        check(
             r#"
 //- /lib.rs
 mod <|>foo;
 
 //- /foo/mod.rs
 // empty
+//^ file
 "#,
-            "foo SOURCE_FILE FileId(2) 0..9",
-            "// empty\n",
         );
     }
 
     #[test]
     fn goto_def_for_macros() {
-        check_goto(
-            "
-            //- /lib.rs
-            macro_rules! foo { () => { () } }
-
-            fn bar() {
-                <|>foo!();
-            }
-            ",
-            "foo MACRO_CALL FileId(1) 0..33 13..16",
-            "macro_rules! foo { () => { () } }|foo",
+        check(
+            r#"
+macro_rules! foo { () => { () } }
+           //^^^
+fn bar() {
+    <|>foo!();
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_macros_from_other_crates() {
-        check_goto(
-            "
-            //- /lib.rs
-            use foo::foo;
-            fn bar() {
-                <|>foo!();
-            }
-
-            //- /foo/lib.rs
-            #[macro_export]
-            macro_rules! foo { () => { () } }
-            ",
-            "foo MACRO_CALL FileId(2) 0..49 29..32",
-            "#[macro_export]\nmacro_rules! foo { () => { () } }|foo",
-        );
-    }
-
-    #[test]
-    fn goto_def_for_use_alias() {
-        check_goto(
+        check(
             r#"
 //- /lib.rs
-use foo as bar<|>;
+use foo::foo;
+fn bar() {
+    <|>foo!();
+}
 
 //- /foo/lib.rs
 #[macro_export]
 macro_rules! foo { () => { () } }
+           //^^^
 "#,
-            "SOURCE_FILE FileId(2) 0..50",
-            "#[macro_export]\nmacro_rules! foo { () => { () } }\n",
-        );
-    }
-
-    #[test]
-    fn goto_def_for_use_alias_foo_macro() {
-        check_goto(
-            "
-            //- /lib.rs
-            use foo::foo as bar<|>;
-
-            //- /foo/lib.rs
-            #[macro_export]
-            macro_rules! foo { () => { () } }
-            ",
-            "foo MACRO_CALL FileId(2) 0..49 29..32",
-            "#[macro_export]\nmacro_rules! foo { () => { () } }|foo",
         );
     }
 
     #[test]
     fn goto_def_for_macros_in_use_tree() {
-        check_goto(
-            "
-            //- /lib.rs
-            use foo::foo<|>;
+        check(
+            r#"
+//- /lib.rs
+use foo::foo<|>;
 
-            //- /foo/lib.rs
-            #[macro_export]
-            macro_rules! foo { () => { () } }
-            ",
-            "foo MACRO_CALL FileId(2) 0..49 29..32",
-            "#[macro_export]\nmacro_rules! foo { () => { () } }|foo",
+//- /foo/lib.rs
+#[macro_export]
+macro_rules! foo { () => { () } }
+           //^^^
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_macro_defined_fn_with_arg() {
-        check_goto(
+        check(
             r#"
 //- /lib.rs
 macro_rules! define_fn {
@@ -334,522 +253,478 @@ macro_rules! define_fn {
 }
 
 define_fn!(foo);
+         //^^^
 
 fn bar() {
    <|>foo();
 }
 "#,
-            "foo FN_DEF FileId(1) 65..81 76..79",
-            "define_fn!(foo);|foo",
         );
     }
 
     #[test]
     fn goto_def_for_macro_defined_fn_no_arg() {
-        check_goto(
+        check(
             r#"
 //- /lib.rs
 macro_rules! define_fn {
     () => (fn foo() {})
 }
 
-define_fn!();
+  define_fn!();
+//^^^^^^^^^^^^^
 
 fn bar() {
    <|>foo();
 }
 "#,
-            "foo FN_DEF FileId(1) 52..65 52..65",
-            "define_fn!();|define_fn!();",
         );
     }
 
     #[test]
     fn goto_definition_works_for_macro_inside_pattern() {
-        check_goto(
-            "
-            //- /lib.rs
-            macro_rules! foo {() => {0}}
-
-            fn bar() {
-                match (0,1) {
-                    (<|>foo!(), _) => {}
-                }
-            }
-            ",
-            "foo MACRO_CALL FileId(1) 0..28 13..16",
-            "macro_rules! foo {() => {0}}|foo",
+        check(
+            r#"
+//- /lib.rs
+macro_rules! foo {() => {0}}
+           //^^^
+
+fn bar() {
+    match (0,1) {
+        (<|>foo!(), _) => {}
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_definition_works_for_macro_inside_match_arm_lhs() {
-        check_goto(
-            "
-            //- /lib.rs
-            macro_rules! foo {() => {0}}
-
-            fn bar() {
-                match 0 {
-                    <|>foo!() => {}
-                }
-            }
-            ",
-            "foo MACRO_CALL FileId(1) 0..28 13..16",
-            "macro_rules! foo {() => {0}}|foo",
+        check(
+            r#"
+//- /lib.rs
+macro_rules! foo {() => {0}}
+           //^^^
+fn bar() {
+    match 0 {
+        <|>foo!() => {}
+    }
+}
+"#,
+        );
+    }
+
+    #[test]
+    fn goto_def_for_use_alias() {
+        check(
+            r#"
+//- /lib.rs
+use foo as bar<|>;
+
+//- /foo/lib.rs
+// empty
+//^ file
+"#,
+        );
+    }
+
+    #[test]
+    fn goto_def_for_use_alias_foo_macro() {
+        check(
+            r#"
+//- /lib.rs
+use foo::foo as bar<|>;
+
+//- /foo/lib.rs
+#[macro_export]
+macro_rules! foo { () => { () } }
+           //^^^
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_methods() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            impl Foo {
-                fn frobnicate(&self) { }
-            }
+        check(
+            r#"
+//- /lib.rs
+struct Foo;
+impl Foo {
+    fn frobnicate(&self) { }
+     //^^^^^^^^^^
+}
 
-            fn bar(foo: &Foo) {
-                foo.frobnicate<|>();
-            }
-            ",
-            "frobnicate FN_DEF FileId(1) 27..51 30..40",
-            "fn frobnicate(&self) { }|frobnicate",
+fn bar(foo: &Foo) {
+    foo.frobnicate<|>();
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_fields() {
-        check_goto(
-            r"
-            //- /lib.rs
-            struct Foo {
-                spam: u32,
-            }
+        check(
+            r#"
+struct Foo {
+    spam: u32,
+} //^^^^
 
-            fn bar(foo: &Foo) {
-                foo.spam<|>;
-            }
-            ",
-            "spam RECORD_FIELD_DEF FileId(1) 17..26 17..21",
-            "spam: u32|spam",
+fn bar(foo: &Foo) {
+    foo.spam<|>;
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_record_fields() {
-        check_goto(
-            r"
-            //- /lib.rs
-            struct Foo {
-                spam: u32,
-            }
+        check(
+            r#"
+//- /lib.rs
+struct Foo {
+    spam: u32,
+} //^^^^
 
-            fn bar() -> Foo {
-                Foo {
-                    spam<|>: 0,
-                }
-            }
-            ",
-            "spam RECORD_FIELD_DEF FileId(1) 17..26 17..21",
-            "spam: u32|spam",
+fn bar() -> Foo {
+    Foo {
+        spam<|>: 0,
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_record_pat_fields() {
-        check_goto(
-            r"
-            //- /lib.rs
-            struct Foo {
-                spam: u32,
-            }
+        check(
+            r#"
+//- /lib.rs
+struct Foo {
+    spam: u32,
+} //^^^^
 
-            fn bar(foo: Foo) -> Foo {
-                let Foo { spam<|>: _, } = foo
-            }
-            ",
-            "spam RECORD_FIELD_DEF FileId(1) 17..26 17..21",
-            "spam: u32|spam",
+fn bar(foo: Foo) -> Foo {
+    let Foo { spam<|>: _, } = foo
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_record_fields_macros() {
-        check_goto(
+        check(
             r"
-            //- /lib.rs
-            macro_rules! m { () => { 92 };}
-            struct Foo { spam: u32 }
+macro_rules! m { () => { 92 };}
+struct Foo { spam: u32 }
+           //^^^^
 
-            fn bar() -> Foo {
-                Foo { spam<|>: m!() }
-            }
-            ",
-            "spam RECORD_FIELD_DEF FileId(1) 45..54 45..49",
-            "spam: u32|spam",
+fn bar() -> Foo {
+    Foo { spam<|>: m!() }
+}
+",
         );
     }
 
     #[test]
     fn goto_for_tuple_fields() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo(u32);
-
-            fn bar() {
-                let foo = Foo(0);
-                foo.<|>0;
-            }
-            ",
-            "TUPLE_FIELD_DEF FileId(1) 11..14",
-            "u32",
+        check(
+            r#"
+struct Foo(u32);
+         //^^^
+
+fn bar() {
+    let foo = Foo(0);
+    foo.<|>0;
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_ufcs_inherent_methods() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            impl Foo {
-                fn frobnicate() { }
-            }
+        check(
+            r#"
+struct Foo;
+impl Foo {
+    fn frobnicate() { }
+}    //^^^^^^^^^^
 
-            fn bar(foo: &Foo) {
-                Foo::frobnicate<|>();
-            }
-            ",
-            "frobnicate FN_DEF FileId(1) 27..46 30..40",
-            "fn frobnicate() { }|frobnicate",
+fn bar(foo: &Foo) {
+    Foo::frobnicate<|>();
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_ufcs_trait_methods_through_traits() {
-        check_goto(
-            "
-            //- /lib.rs
-            trait Foo {
-                fn frobnicate();
-            }
+        check(
+            r#"
+trait Foo {
+    fn frobnicate();
+}    //^^^^^^^^^^
 
-            fn bar() {
-                Foo::frobnicate<|>();
-            }
-            ",
-            "frobnicate FN_DEF FileId(1) 16..32 19..29",
-            "fn frobnicate();|frobnicate",
+fn bar() {
+    Foo::frobnicate<|>();
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_ufcs_trait_methods_through_self() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            trait Trait {
-                fn frobnicate();
-            }
-            impl Trait for Foo {}
+        check(
+            r#"
+struct Foo;
+trait Trait {
+    fn frobnicate();
+}    //^^^^^^^^^^
+impl Trait for Foo {}
 
-            fn bar() {
-                Foo::frobnicate<|>();
-            }
-            ",
-            "frobnicate FN_DEF FileId(1) 30..46 33..43",
-            "fn frobnicate();|frobnicate",
+fn bar() {
+    Foo::frobnicate<|>();
+}
+"#,
         );
     }
 
     #[test]
     fn goto_definition_on_self() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            impl Foo {
-                pub fn new() -> Self {
-                    Self<|> {}
-                }
-            }
-            ",
-            "impl IMPL_DEF FileId(1) 12..73",
-            "impl Foo {...}",
-        );
-
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            impl Foo {
-                pub fn new() -> Self<|> {
-                    Self {}
-                }
-            }
-            ",
-            "impl IMPL_DEF FileId(1) 12..73",
-            "impl Foo {...}",
-        );
-
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo { A }
-            impl Foo {
-                pub fn new() -> Self<|> {
-                    Foo::A
-                }
-            }
-            ",
-            "impl IMPL_DEF FileId(1) 15..75",
-            "impl Foo {...}",
-        );
-
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo { A }
-            impl Foo {
-                pub fn thing(a: &Self<|>) {
-                }
-            }
-            ",
-            "impl IMPL_DEF FileId(1) 15..62",
-            "impl Foo {...}",
+        check(
+            r#"
+struct Foo;
+impl Foo {
+   //^^^
+    pub fn new() -> Self {
+        Self<|> {}
+    }
+}
+"#,
+        );
+        check(
+            r#"
+struct Foo;
+impl Foo {
+   //^^^
+    pub fn new() -> Self<|> {
+        Self {}
+    }
+}
+"#,
+        );
+
+        check(
+            r#"
+enum Foo { A }
+impl Foo {
+   //^^^
+    pub fn new() -> Self<|> {
+        Foo::A
+    }
+}
+"#,
+        );
+
+        check(
+            r#"
+enum Foo { A }
+impl Foo {
+   //^^^
+    pub fn thing(a: &Self<|>) {
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_definition_on_self_in_trait_impl() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            trait Make {
-                fn new() -> Self;
-            }
-            impl Make for Foo {
-                fn new() -> Self {
-                    Self<|> {}
-                }
-            }
-            ",
-            "impl IMPL_DEF FileId(1) 49..115",
-            "impl Make for Foo {...}",
+        check(
+            r#"
+struct Foo;
+trait Make {
+    fn new() -> Self;
+}
+impl Make for Foo {
+            //^^^
+    fn new() -> Self {
+        Self<|> {}
+    }
+}
+"#,
         );
 
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo;
-            trait Make {
-                fn new() -> Self;
-            }
-            impl Make for Foo {
-                fn new() -> Self<|> {
-                    Self {}
-                }
-            }
-            ",
-            "impl IMPL_DEF FileId(1) 49..115",
-            "impl Make for Foo {...}",
+        check(
+            r#"
+struct Foo;
+trait Make {
+    fn new() -> Self;
+}
+impl Make for Foo {
+            //^^^
+    fn new() -> Self<|> {
+        Self {}
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_when_used_on_definition_name_itself() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo<|> { value: u32 }
-            ",
-            "Foo STRUCT_DEF FileId(1) 0..25 7..10",
-            "struct Foo { value: u32 }|Foo",
+        check(
+            r#"
+struct Foo<|> { value: u32 }
+     //^^^
+            "#,
         );
 
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            struct Foo {
-                field<|>: string,
-            }
-            "#,
-            "field RECORD_FIELD_DEF FileId(1) 17..30 17..22",
-            "field: string|field",
+struct Foo {
+    field<|>: string,
+} //^^^^^
+"#,
         );
 
-        check_goto(
-            "
-            //- /lib.rs
-            fn foo_test<|>() { }
-            ",
-            "foo_test FN_DEF FileId(1) 0..17 3..11",
-            "fn foo_test() { }|foo_test",
+        check(
+            r#"
+fn foo_test<|>() { }
+ //^^^^^^^^
+"#,
         );
 
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo<|> {
-                Variant,
-            }
-            ",
-            "Foo ENUM_DEF FileId(1) 0..25 5..8",
-            "enum Foo {...}|Foo",
-        );
-
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo {
-                Variant1,
-                Variant2<|>,
-                Variant3,
-            }
-            ",
-            "Variant2 ENUM_VARIANT FileId(1) 29..37 29..37",
-            "Variant2|Variant2",
+        check(
+            r#"
+enum Foo<|> { Variant }
+   //^^^
+"#,
         );
 
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            static INNER<|>: &str = "";
-            "#,
-            "INNER STATIC_DEF FileId(1) 0..24 7..12",
-            "static INNER: &str = \"\";|INNER",
+enum Foo {
+    Variant1,
+    Variant2<|>,
+  //^^^^^^^^
+    Variant3,
+}
+"#,
         );
 
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            const INNER<|>: &str = "";
-            "#,
-            "INNER CONST_DEF FileId(1) 0..23 6..11",
-            "const INNER: &str = \"\";|INNER",
+static INNER<|>: &str = "";
+     //^^^^^
+"#,
         );
 
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            type Thing<|> = Option<()>;
-            "#,
-            "Thing TYPE_ALIAS_DEF FileId(1) 0..24 5..10",
-            "type Thing = Option<()>;|Thing",
+const INNER<|>: &str = "";
+    //^^^^^
+"#,
         );
 
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            trait Foo<|> { }
-            "#,
-            "Foo TRAIT_DEF FileId(1) 0..13 6..9",
-            "trait Foo { }|Foo",
+type Thing<|> = Option<()>;
+   //^^^^^
+"#,
         );
 
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            mod bar<|> { }
-            "#,
-            "bar MODULE FileId(1) 0..11 4..7",
-            "mod bar { }|bar",
+trait Foo<|> { }
+    //^^^
+"#,
+        );
+
+        check(
+            r#"
+mod bar<|> { }
+  //^^^
+"#,
         );
     }
 
     #[test]
     fn goto_from_macro() {
-        check_goto(
-            "
-            //- /lib.rs
-            macro_rules! id {
-                ($($tt:tt)*) => { $($tt)* }
-            }
-            fn foo() {}
-            id! {
-                fn bar() {
-                    fo<|>o();
-                }
-            }
-            mod confuse_index { fn foo(); }
-            ",
-            "foo FN_DEF FileId(1) 52..63 55..58",
-            "fn foo() {}|foo",
+        check(
+            r#"
+macro_rules! id {
+    ($($tt:tt)*) => { $($tt)* }
+}
+fn foo() {}
+ //^^^
+id! {
+    fn bar() {
+        fo<|>o();
+    }
+}
+mod confuse_index { fn foo(); }
+"#,
         );
     }
 
     #[test]
     fn goto_through_format() {
-        check_goto(
-            "
-            //- /lib.rs
-            #[macro_export]
-            macro_rules! format {
-                ($($arg:tt)*) => ($crate::fmt::format($crate::__export::format_args!($($arg)*)))
-            }
-            #[rustc_builtin_macro]
-            #[macro_export]
-            macro_rules! format_args {
-                ($fmt:expr) => ({ /* compiler built-in */ });
-                ($fmt:expr, $($args:tt)*) => ({ /* compiler built-in */ })
-            }
-            pub mod __export {
-                pub use crate::format_args;
-                fn foo() {} // for index confusion
-            }
-            fn foo() -> i8 {}
-            fn test() {
-                format!(\"{}\", fo<|>o())
-            }
-            ",
-            "foo FN_DEF FileId(1) 398..415 401..404",
-            "fn foo() -> i8 {}|foo",
+        check(
+            r#"
+#[macro_export]
+macro_rules! format {
+    ($($arg:tt)*) => ($crate::fmt::format($crate::__export::format_args!($($arg)*)))
+}
+#[rustc_builtin_macro]
+#[macro_export]
+macro_rules! format_args {
+    ($fmt:expr) => ({ /* compiler built-in */ });
+    ($fmt:expr, $($args:tt)*) => ({ /* compiler built-in */ })
+}
+pub mod __export {
+    pub use crate::format_args;
+    fn foo() {} // for index confusion
+}
+fn foo() -> i8 {}
+ //^^^
+fn test() {
+    format!("{}", fo<|>o())
+}
+"#,
         );
     }
 
     #[test]
     fn goto_for_type_param() {
-        check_goto(
+        check(
             r#"
-            //- /lib.rs
-            struct Foo<T: Clone> {
-                t: <|>T,
-            }
-            "#,
-            "T TYPE_PARAM FileId(1) 11..19 11..12",
-            "T: Clone|T",
+struct Foo<T: Clone> { t: <|>T }
+         //^
+"#,
         );
     }
 
     #[test]
     fn goto_within_macro() {
-        check_goto(
+        check(
             r#"
-//- /lib.rs
 macro_rules! id {
     ($($tt:tt)*) => ($($tt)*)
 }
 
 fn foo() {
     let x = 1;
+      //^
     id!({
         let y = <|>x;
         let z = y;
     });
 }
 "#,
-            "x BIND_PAT FileId(1) 70..71",
-            "x",
         );
 
-        check_goto(
+        check(
             r#"
-//- /lib.rs
 macro_rules! id {
     ($($tt:tt)*) => ($($tt)*)
 }
@@ -858,159 +733,125 @@ fn foo() {
     let x = 1;
     id!({
         let y = x;
+          //^
         let z = <|>y;
     });
 }
 "#,
-            "y BIND_PAT FileId(1) 99..100",
-            "y",
         );
     }
 
     #[test]
     fn goto_def_in_local_fn() {
-        check_goto(
-            "
-            //- /lib.rs
-            fn main() {
-                fn foo() {
-                    let x = 92;
-                    <|>x;
-                }
-            }
-            ",
-            "x BIND_PAT FileId(1) 39..40",
-            "x",
+        check(
+            r#"
+fn main() {
+    fn foo() {
+        let x = 92;
+          //^
+        <|>x;
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_in_local_macro() {
-        check_goto(
-            r"
-            //- /lib.rs
-            fn bar() {
-                macro_rules! foo { () => { () } }
-                <|>foo!();
-            }
-            ",
-            "foo MACRO_CALL FileId(1) 15..48 28..31",
-            "macro_rules! foo { () => { () } }|foo",
+        check(
+            r#"
+fn bar() {
+    macro_rules! foo { () => { () } }
+               //^^^
+    <|>foo!();
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_field_init_shorthand() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo { x: i32 }
-            fn main() {
-                let x = 92;
-                Foo { x<|> };
-            }
-            ",
-            "x BIND_PAT FileId(1) 42..43",
-            "x",
+        check(
+            r#"
+struct Foo { x: i32 }
+fn main() {
+    let x = 92;
+      //^
+    Foo { x<|> };
+}
+"#,
         )
     }
 
     #[test]
     fn goto_def_for_enum_variant_field() {
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo {
-                Bar { x: i32 }
-            }
-            fn baz(foo: Foo) {
-                match foo {
-                    Foo::Bar { x<|> } => x
-                };
-            }
-            ",
-            "x RECORD_FIELD_DEF FileId(1) 21..27 21..22",
-            "x: i32|x",
+        check(
+            r#"
+enum Foo {
+    Bar { x: i32 }
+}       //^
+fn baz(foo: Foo) {
+    match foo {
+        Foo::Bar { x<|> } => x
+    };
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_enum_variant_self_pattern_const() {
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo {
-                Bar,
-            }
-            impl Foo {
-                fn baz(self) {
-                    match self {
-                        Self::Bar<|> => {}
-                    }
-                }
-            }
-            ",
-            "Bar ENUM_VARIANT FileId(1) 15..18 15..18",
-            "Bar|Bar",
+        check(
+            r#"
+enum Foo { Bar }
+         //^^^
+impl Foo {
+    fn baz(self) {
+        match self { Self::Bar<|> => {} }
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_enum_variant_self_pattern_record() {
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo {
-                Bar { val: i32 },
-            }
-            impl Foo {
-                fn baz(self) -> i32 {
-                    match self {
-                        Self::Bar<|> { val } => {}
-                    }
-                }
-            }
-            ",
-            "Bar ENUM_VARIANT FileId(1) 15..31 15..18",
-            "Bar { val: i32 }|Bar",
+        check(
+            r#"
+enum Foo { Bar { val: i32 } }
+         //^^^
+impl Foo {
+    fn baz(self) -> i32 {
+        match self { Self::Bar<|> { val } => {} }
+    }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_enum_variant_self_expr_const() {
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo {
-                Bar,
-            }
-            impl Foo {
-                fn baz(self) {
-                    Self::Bar<|>;
-                }
-            }
-            ",
-            "Bar ENUM_VARIANT FileId(1) 15..18 15..18",
-            "Bar|Bar",
+        check(
+            r#"
+enum Foo { Bar }
+         //^^^
+impl Foo {
+    fn baz(self) { Self::Bar<|>; }
+}
+"#,
         );
     }
 
     #[test]
     fn goto_def_for_enum_variant_self_expr_record() {
-        check_goto(
-            "
-            //- /lib.rs
-            enum Foo {
-                Bar { val: i32 },
-            }
-            impl Foo {
-                fn baz(self) {
-                    Self::Bar<|> {val: 4};
-                }
-            }
-            ",
-            "Bar ENUM_VARIANT FileId(1) 15..31 15..18",
-            "Bar { val: i32 }|Bar",
+        check(
+            r#"
+enum Foo { Bar { val: i32 } }
+         //^^^
+impl Foo {
+    fn baz(self) { Self::Bar<|> {val: 4}; }
+}
+"#,
         );
     }
 }
diff --git a/crates/ra_ide/src/goto_implementation.rs b/crates/ra_ide/src/goto_implementation.rs
index 0cec0657e..99a7022a4 100644
--- a/crates/ra_ide/src/goto_implementation.rs
+++ b/crates/ra_ide/src/goto_implementation.rs
@@ -74,135 +74,152 @@ fn impls_for_trait(
 
 #[cfg(test)]
 mod tests {
-    use crate::mock_analysis::analysis_and_position;
+    use ra_db::FileRange;
 
-    fn check_goto(fixture: &str, expected: &[&str]) {
-        let (analysis, pos) = analysis_and_position(fixture);
+    use crate::mock_analysis::MockAnalysis;
 
-        let mut navs = analysis.goto_implementation(pos).unwrap().unwrap().info;
-        assert_eq!(navs.len(), expected.len());
-        navs.sort_by_key(|nav| (nav.file_id(), nav.full_range().start()));
-        navs.into_iter().enumerate().for_each(|(i, nav)| nav.assert_match(expected[i]));
+    fn check(ra_fixture: &str) {
+        let (mock, position) = MockAnalysis::with_files_and_position(ra_fixture);
+        let annotations = mock.annotations();
+        let analysis = mock.analysis();
+
+        let navs = analysis.goto_implementation(position).unwrap().unwrap().info;
+
+        let key = |frange: &FileRange| (frange.file_id, frange.range.start());
+
+        let mut expected = annotations
+            .into_iter()
+            .map(|(range, data)| {
+                assert!(data.is_empty());
+                range
+            })
+            .collect::<Vec<_>>();
+        expected.sort_by_key(key);
+
+        let mut actual = navs
+            .into_iter()
+            .map(|nav| FileRange { file_id: nav.file_id(), range: nav.range() })
+            .collect::<Vec<_>>();
+        actual.sort_by_key(key);
+
+        assert_eq!(expected, actual);
     }
 
     #[test]
     fn goto_implementation_works() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo<|>;
-            impl Foo {}
-            ",
-            &["impl IMPL_DEF FileId(1) 12..23"],
+        check(
+            r#"
+struct Foo<|>;
+impl Foo {}
+   //^^^
+"#,
         );
     }
 
     #[test]
     fn goto_implementation_works_multiple_blocks() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo<|>;
-            impl Foo {}
-            impl Foo {}
-            ",
-            &["impl IMPL_DEF FileId(1) 12..23", "impl IMPL_DEF FileId(1) 24..35"],
+        check(
+            r#"
+struct Foo<|>;
+impl Foo {}
+   //^^^
+impl Foo {}
+   //^^^
+"#,
         );
     }
 
     #[test]
     fn goto_implementation_works_multiple_mods() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo<|>;
-            mod a {
-                impl super::Foo {}
-            }
-            mod b {
-                impl super::Foo {}
-            }
-            ",
-            &["impl IMPL_DEF FileId(1) 24..42", "impl IMPL_DEF FileId(1) 57..75"],
+        check(
+            r#"
+struct Foo<|>;
+mod a {
+    impl super::Foo {}
+       //^^^^^^^^^^
+}
+mod b {
+    impl super::Foo {}
+       //^^^^^^^^^^
+}
+"#,
         );
     }
 
     #[test]
     fn goto_implementation_works_multiple_files() {
-        check_goto(
-            "
-            //- /lib.rs
-            struct Foo<|>;
-            mod a;
-            mod b;
-            //- /a.rs
-            impl crate::Foo {}
-            //- /b.rs
-            impl crate::Foo {}
-            ",
-            &["impl IMPL_DEF FileId(2) 0..18", "impl IMPL_DEF FileId(3) 0..18"],
+        check(
+            r#"
+//- /lib.rs
+struct Foo<|>;
+mod a;
+mod b;
+//- /a.rs
+impl crate::Foo {}
+   //^^^^^^^^^^
+//- /b.rs
+impl crate::Foo {}
+   //^^^^^^^^^^
+"#,
         );
     }
 
     #[test]
     fn goto_implementation_for_trait() {
-        check_goto(
-            "
-            //- /lib.rs
-            trait T<|> {}
-            struct Foo;
-            impl T for Foo {}
-            ",
-            &["impl IMPL_DEF FileId(1) 23..40"],
+        check(
+            r#"
+trait T<|> {}
+struct Foo;
+impl T for Foo {}
+         //^^^
+"#,
         );
     }
 
     #[test]
     fn goto_implementation_for_trait_multiple_files() {
-        check_goto(
-            "
-            //- /lib.rs
-            trait T<|> {};
-            struct Foo;
-            mod a;
-            mod b;
-            //- /a.rs
-            impl crate::T for crate::Foo {}
-            //- /b.rs
-            impl crate::T for crate::Foo {}
-            ",
-            &["impl IMPL_DEF FileId(2) 0..31", "impl IMPL_DEF FileId(3) 0..31"],
+        check(
+            r#"
+//- /lib.rs
+trait T<|> {};
+struct Foo;
+mod a;
+mod b;
+//- /a.rs
+impl crate::T for crate::Foo {}
+                //^^^^^^^^^^
+//- /b.rs
+impl crate::T for crate::Foo {}
+                //^^^^^^^^^^
+            "#,
         );
     }
 
     #[test]
     fn goto_implementation_all_impls() {
-        check_goto(
-            "
-            //- /lib.rs
-            trait T {}
-            struct Foo<|>;
-            impl Foo {}
-            impl T for Foo {}
-            impl T for &Foo {}
-            ",
-            &[
-                "impl IMPL_DEF FileId(1) 23..34",
-                "impl IMPL_DEF FileId(1) 35..52",
-                "impl IMPL_DEF FileId(1) 53..71",
-            ],
+        check(
+            r#"
+//- /lib.rs
+trait T {}
+struct Foo<|>;
+impl Foo {}
+   //^^^
+impl T for Foo {}
+         //^^^
+impl T for &Foo {}
+         //^^^^
+"#,
         );
     }
 
     #[test]
     fn goto_implementation_to_builtin_derive() {
-        check_goto(
-            "
-            //- /lib.rs
-            #[derive(Copy)]
-            struct Foo<|>;
-            ",
-            &["impl IMPL_DEF FileId(1) 0..15"],
+        check(
+            r#"
+  #[derive(Copy)]
+//^^^^^^^^^^^^^^^
+struct Foo<|>;
+"#,
         );
     }
 }
diff --git a/crates/ra_ide/src/mock_analysis.rs b/crates/ra_ide/src/mock_analysis.rs
index 889b84c59..db6d50694 100644
--- a/crates/ra_ide/src/mock_analysis.rs
+++ b/crates/ra_ide/src/mock_analysis.rs
@@ -3,7 +3,9 @@ use std::sync::Arc;
 
 use ra_cfg::CfgOptions;
 use ra_db::{CrateName, Env, FileSet, SourceRoot, VfsPath};
-use test_utils::{extract_range_or_offset, Fixture, RangeOrOffset, CURSOR_MARKER};
+use test_utils::{
+    extract_annotations, extract_range_or_offset, Fixture, RangeOrOffset, CURSOR_MARKER,
+};
 
 use crate::{
     Analysis, AnalysisChange, AnalysisHost, CrateGraph, Edition, FileId, FilePosition, FileRange,
@@ -77,6 +79,24 @@ impl MockAnalysis {
             .expect("no file in this mock");
         FileId(idx as u32 + 1)
     }
+    pub fn annotations(&self) -> Vec<(FileRange, String)> {
+        self.files
+            .iter()
+            .enumerate()
+            .flat_map(|(idx, fixture)| {
+                let file_id = FileId(idx as u32 + 1);
+                let annotations = extract_annotations(&fixture.text);
+                annotations
+                    .into_iter()
+                    .map(move |(range, data)| (FileRange { file_id, range }, data))
+            })
+            .collect()
+    }
+    pub fn annotation(&self) -> (FileRange, String) {
+        let mut all = self.annotations();
+        assert_eq!(all.len(), 1);
+        all.pop().unwrap()
+    }
     pub fn analysis_host(self) -> AnalysisHost {
         let mut host = AnalysisHost::default();
         let mut change = AnalysisChange::new();
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 0940fcc28..25bcd80af 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -330,11 +330,12 @@ pub(crate) fn handle_workspace_symbol(
     fn exec_query(snap: &GlobalStateSnapshot, query: Query) -> Result<Vec<SymbolInformation>> {
         let mut res = Vec::new();
         for nav in snap.analysis.symbol_search(query)? {
+            let container_name = nav.container_name().map(|v| v.to_string());
             let info = SymbolInformation {
                 name: nav.name().to_string(),
                 kind: to_proto::symbol_kind(nav.kind()),
-                location: to_proto::location(snap, nav.file_range())?,
-                container_name: nav.container_name().map(|v| v.to_string()),
+                location: to_proto::location_from_nav(snap, nav)?,
+                container_name,
                 deprecated: None,
             };
             res.push(info);
@@ -1213,8 +1214,8 @@ fn show_impl_command_link(
             let position = to_proto::position(&line_index, position.offset);
             let locations: Vec<_> = nav_data
                 .info
-                .iter()
-                .filter_map(|it| to_proto::location(snap, it.file_range()).ok())
+                .into_iter()
+                .filter_map(|nav| to_proto::location_from_nav(snap, nav).ok())
                 .collect();
             let title = implementation_title(locations.len());
             let command = show_references_command(title, &uri, position, locations);
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index a03222ae9..a0a58f689 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -446,6 +446,18 @@ pub(crate) fn location(
     Ok(loc)
 }
 
+/// Perefer using `location_link`, if the client has the cap.
+pub(crate) fn location_from_nav(
+    snap: &GlobalStateSnapshot,
+    nav: NavigationTarget,
+) -> Result<lsp_types::Location> {
+    let url = url(snap, nav.file_id());
+    let line_index = snap.analysis.file_line_index(nav.file_id())?;
+    let range = range(&line_index, nav.full_range());
+    let loc = lsp_types::Location::new(url, range);
+    Ok(loc)
+}
+
 pub(crate) fn location_link(
     snap: &GlobalStateSnapshot,
     src: Option<FileRange>,