envapsulate navigation target better

crates/ra_hir/src/code_model_api.rs

@@ -274,6 +274,8 @@ pub struct Function {
     pub(crate) def_id: DefId,
 }
 
+pub use crate::code_model_impl::function::ScopeEntryWithSyntax;
+
 /// The declared signature of a function.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct FnSignature {

crates/ra_hir/src/code_model_impl/function.rs

@@ -15,7 +15,7 @@ use crate::{
     impl_block::ImplBlock,
 };
 
-pub use self::scope::{FnScopes, ScopesWithSyntaxMapping};
+pub use self::scope::{FnScopes, ScopesWithSyntaxMapping, ScopeEntryWithSyntax};
 
 impl Function {
     pub(crate) fn new(def_id: DefId) -> Function {

crates/ra_hir/src/lib.rs

@@ -59,5 +59,5 @@ pub use self::code_model_api::{
     Def,
     Module, ModuleSource, Problem,
     Struct, Enum, EnumVariant,
-    Function, FnSignature,
+    Function, FnSignature, ScopeEntryWithSyntax,
 };

crates/ra_ide_api/src/goto_definition.rs

@@ -1,6 +1,6 @@
 use ra_db::{FileId, Cancelable, SyntaxDatabase};
 use ra_syntax::{
-    TextRange, AstNode, ast, SyntaxKind::{NAME, MODULE},
+    AstNode, ast,
     algo::find_node_at_offset,
 };
 
@@ -32,13 +32,7 @@ pub(crate) fn reference_definition(
         let scope = fn_descr.scopes(db)?;
         // First try to resolve the symbol locally
         if let Some(entry) = scope.resolve_local_name(name_ref) {
-            let nav = NavigationTarget {
+            let nav = NavigationTarget::from_scope_entry(file_id, &entry);
-                file_id,
-                name: entry.name().to_string().into(),
-                range: entry.ptr().range(),
-                kind: NAME,
-                ptr: None,
-            };
             return Ok(vec![nav]);
         };
     }
@@ -79,18 +73,7 @@ fn name_definition(
             if let Some(child_module) =
                 hir::source_binder::module_from_declaration(db, file_id, module)?
             {
-                let (file_id, _) = child_module.definition_source(db)?;
+                let nav = NavigationTarget::from_module(db, child_module)?;
-                let name = match child_module.name(db)? {
-                    Some(name) => name.to_string().into(),
-                    None => "".into(),
-                };
-                let nav = NavigationTarget {
-                    file_id,
-                    name,
-                    range: TextRange::offset_len(0.into(), 0.into()),
-                    kind: MODULE,
-                    ptr: None,
-                };
                 return Ok(Some(vec![nav]));
             }
         }

crates/ra_ide_api/src/hover.rs

@@ -88,11 +88,11 @@ fn doc_text_for(db: &RootDatabase, nav: NavigationTarget) -> Cancelable<Option<S
 
 impl NavigationTarget {
     fn node(&self, db: &RootDatabase) -> Option<TreePtr<SyntaxNode>> {
-        let source_file = db.source_file(self.file_id);
+        let source_file = db.source_file(self.file_id());
         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.range())?
             .to_owned();
         Some(node)
     }

crates/ra_ide_api/src/imp.rs

@@ -11,7 +11,6 @@ use ra_syntax::{
     TextRange, AstNode, SourceFile,
     ast::{self, NameOwner},
     algo::find_node_at_offset,
-    SyntaxKind::*,
 };
 
 use crate::{
@@ -109,18 +108,8 @@ impl db::RootDatabase {
             None => return Ok(Vec::new()),
             Some(it) => it,
         };
-        let (file_id, ast_module) = match module.declaration_source(self)? {
+        let nav = NavigationTarget::from_module(self, module)?;
-            None => return Ok(Vec::new()),
+        Ok(vec![nav])
-            Some(it) => it,
-        };
-        let name = ast_module.name().unwrap();
-        Ok(vec![NavigationTarget {
-            file_id,
-            name: name.text().clone(),
-            range: name.syntax().range(),
-            kind: MODULE,
-            ptr: None,
-        }])
     }
     /// Returns `Vec` for the same reason as `parent_module`
     pub(crate) fn crate_for(&self, file_id: FileId) -> Cancelable<Vec<CrateId>> {

crates/ra_ide_api/src/lib.rs

@@ -34,9 +34,9 @@ mod syntax_highlighting;
 
 use std::{fmt, sync::Arc};
 
-use ra_syntax::{SmolStr, SourceFile, TreePtr, SyntaxKind, TextRange, TextUnit};
+use ra_syntax::{SourceFile, TreePtr, TextRange, TextUnit};
 use ra_text_edit::TextEdit;
-use ra_db::{SyntaxDatabase, FilesDatabase, LocalSyntaxPtr, BaseDatabase};
+use ra_db::{SyntaxDatabase, FilesDatabase, BaseDatabase};
 use rayon::prelude::*;
 use relative_path::RelativePathBuf;
 use rustc_hash::FxHashMap;
@@ -50,6 +50,7 @@ use crate::{
 pub use crate::{
     completion::{CompletionItem, CompletionItemKind, InsertText},
     runnables::{Runnable, RunnableKind},
+    navigation_target::NavigationTarget,
 };
 pub use ra_ide_api_light::{
     Fold, FoldKind, HighlightedRange, Severity, StructureNode,
@@ -243,39 +244,6 @@ impl Query {
     }
 }
 
-/// `NavigationTarget` represents and element in the editor's UI which you can
-/// click on to navigate to a particular piece of code.
-///
-/// Typically, a `NavigationTarget` corresponds to some element in the source
-/// code, like a function or a struct, but this is not strictly required.
-#[derive(Debug, Clone)]
-pub struct NavigationTarget {
-    file_id: FileId,
-    name: SmolStr,
-    kind: SyntaxKind,
-    range: TextRange,
-    // Should be DefId ideally
-    ptr: Option<LocalSyntaxPtr>,
-}
-
-impl NavigationTarget {
-    pub fn name(&self) -> &SmolStr {
-        &self.name
-    }
-
-    pub fn kind(&self) -> SyntaxKind {
-        self.kind
-    }
-
-    pub fn file_id(&self) -> FileId {
-        self.file_id
-    }
-
-    pub fn range(&self) -> TextRange {
-        self.range
-    }
-}
-
 #[derive(Debug)]
 pub struct RangeInfo<T> {
     pub range: TextRange,

crates/ra_ide_api/src/navigation_target.rs

@@ -1,27 +1,98 @@
 use ra_db::{FileId, LocalSyntaxPtr, Cancelable};
 use ra_syntax::{
-    SyntaxNode, AstNode, SmolStr,
+    SyntaxNode, AstNode, SmolStr, TextRange, ast,
-    ast
+    SyntaxKind::{self, NAME},
 };
 use hir::{Def, ModuleSource};
 
-use crate::{
+use crate::{FileSymbol, db::RootDatabase};
-    NavigationTarget,
+
-    FileSymbol,
+/// `NavigationTarget` represents and element in the editor's UI which you can
-    db::RootDatabase,
+/// click on to navigate to a particular piece of code.
-};
+///
+/// Typically, a `NavigationTarget` corresponds to some element in the source
+/// code, like a function or a struct, but this is not strictly required.
+#[derive(Debug, Clone)]
+pub struct NavigationTarget {
+    file_id: FileId,
+    name: SmolStr,
+    kind: SyntaxKind,
+    range: TextRange,
+    focus_range: Option<TextRange>,
+    // Should be DefId ideally
+    ptr: Option<LocalSyntaxPtr>,
+}
 
 impl NavigationTarget {
+    pub fn name(&self) -> &SmolStr {
+        &self.name
+    }
+
+    pub fn kind(&self) -> SyntaxKind {
+        self.kind
+    }
+
+    pub fn file_id(&self) -> FileId {
+        self.file_id
+    }
+
+    pub fn range(&self) -> TextRange {
+        self.range
+    }
+
+    /// A "most interesting" range withing the `range`.
+    ///
+    /// Typically, `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_symbol(symbol: FileSymbol) -> NavigationTarget {
         NavigationTarget {
             file_id: symbol.file_id,
             name: symbol.name.clone(),
             kind: symbol.ptr.kind(),
             range: symbol.ptr.range(),
+            focus_range: None,
             ptr: Some(symbol.ptr.clone()),
         }
     }
 
+    pub(crate) fn from_scope_entry(
+        file_id: FileId,
+        entry: &hir::ScopeEntryWithSyntax,
+    ) -> NavigationTarget {
+        NavigationTarget {
+            file_id,
+            name: entry.name().to_string().into(),
+            range: entry.ptr().range(),
+            focus_range: None,
+            kind: NAME,
+            ptr: None,
+        }
+    }
+
+    pub(crate) fn from_module(
+        db: &RootDatabase,
+        module: hir::Module,
+    ) -> Cancelable<NavigationTarget> {
+        let (file_id, source) = module.definition_source(db)?;
+        let name = module
+            .name(db)?
+            .map(|it| it.to_string().into())
+            .unwrap_or_default();
+        let res = match source {
+            ModuleSource::SourceFile(node) => {
+                NavigationTarget::from_syntax(file_id, name, None, node.syntax())
+            }
+            ModuleSource::Module(node) => {
+                NavigationTarget::from_syntax(file_id, name, None, node.syntax())
+            }
+        };
+        Ok(res)
+    }
+
     // TODO once Def::Item is gone, this should be able to always return a NavigationTarget
     pub(crate) fn from_def(db: &RootDatabase, def: Def) -> Cancelable<Option<NavigationTarget>> {
         let res = match def {
@@ -41,21 +112,7 @@ impl NavigationTarget {
                 let (file_id, node) = f.source(db)?;
                 NavigationTarget::from_named(file_id.original_file(db), &*node)
             }
-            Def::Module(m) => {
+            Def::Module(m) => NavigationTarget::from_module(db, m)?,
-                let (file_id, source) = m.definition_source(db)?;
-                let name = m
-                    .name(db)?
-                    .map(|it| it.to_string().into())
-                    .unwrap_or_default();
-                match source {
-                    ModuleSource::SourceFile(node) => {
-                        NavigationTarget::from_syntax(file_id, name, node.syntax())
-                    }
-                    ModuleSource::Module(node) => {
-                        NavigationTarget::from_syntax(file_id, name, node.syntax())
-                    }
-                }
-            }
             Def::Item => return Ok(None),
         };
         Ok(Some(res))
@@ -63,15 +120,22 @@ impl NavigationTarget {
 
     fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget {
         let name = node.name().map(|it| it.text().clone()).unwrap_or_default();
-        NavigationTarget::from_syntax(file_id, name, node.syntax())
+        let focus_range = node.name().map(|it| it.syntax().range());
+        NavigationTarget::from_syntax(file_id, name, focus_range, node.syntax())
     }
 
-    fn from_syntax(file_id: FileId, name: SmolStr, node: &SyntaxNode) -> NavigationTarget {
+    fn from_syntax(
+        file_id: FileId,
+        name: SmolStr,
+        focus_range: Option<TextRange>,
+        node: &SyntaxNode,
+    ) -> NavigationTarget {
         NavigationTarget {
             file_id,
             name,
             kind: node.kind(),
             range: node.range(),
+            focus_range,
             ptr: Some(LocalSyntaxPtr::new(node)),
         }
     }