Merge #5137

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

bors r+
🤖

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

Author: bors[bot]

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,

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()
     }
 }
 

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 {

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(),
         )