Repository: rust-analyzer/rust-analyzer
Pull request #1388 (2019-06-09 16:15:47)

Remove NavigationTarget::node and fill docs and description in during construction
Related dissused:

 https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/MBE.20discussion/near/167105559


Patch:
diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs
index 012f374ec8..9d9769859c 100644
--- a/crates/ra_hir/src/expr.rs
+++ b/crates/ra_hir/src/expr.rs
@@ -827,25 +827,25 @@ where
             ast::ExprKind::IndexExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
             ast::ExprKind::RangeExpr(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
             ast::ExprKind::MacroCall(e) => {
+                // very hacky.FIXME change to use the macro resolution
+                let path = e.path().and_then(Path::from_ast);
+
                 let ast_id = self
                     .db
                     .ast_id_map(self.current_file_id)
                     .ast_id(e)
                     .with_file_id(self.current_file_id);
 
-                if let Some(path) = e.path().and_then(Path::from_ast) {
-                    if let Some(def) = self.resolver.resolve_path_as_macro(self.db, &path) {
-                        let call_id = MacroCallLoc { def: def.id, ast_id }.id(self.db);
-                        let file_id = call_id.as_file(MacroFileKind::Expr);
-                        if let Some(node) = self.db.parse_or_expand(file_id) {
-                            if let Some(expr) = ast::Expr::cast(&*node) {
-                                log::debug!("macro expansion {}", expr.syntax().debug_dump());
-                                let old_file_id =
-                                    std::mem::replace(&mut self.current_file_id, file_id);
-                                let id = self.collect_expr(&expr);
-                                self.current_file_id = old_file_id;
-                                return id;
-                            }
+                if let Some(def) = self.resolver.resolve_macro_call(self.db, path) {
+                    let call_id = MacroCallLoc { def, ast_id }.id(self.db);
+                    let file_id = call_id.as_file(MacroFileKind::Expr);
+                    if let Some(node) = self.db.parse_or_expand(file_id) {
+                        if let Some(expr) = ast::Expr::cast(&*node) {
+                            log::debug!("macro expansion {}", expr.syntax().debug_dump());
+                            let old_file_id = std::mem::replace(&mut self.current_file_id, file_id);
+                            let id = self.collect_expr(&expr);
+                            self.current_file_id = old_file_id;
+                            return id;
                         }
                     }
                 }
diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs
index b5938fa032..dc0dd23c9e 100644
--- a/crates/ra_hir/src/nameres.rs
+++ b/crates/ra_hir/src/nameres.rs
@@ -92,6 +92,7 @@ pub struct CrateDefMap {
     extern_prelude: FxHashMap<Name, ModuleDef>,
     root: CrateModuleId,
     modules: Arena<CrateModuleId, ModuleData>,
+    public_macros: FxHashMap<Name, MacroDefId>,
 
     /// Some macros are not well-behavior, which leads to infinite loop
     /// e.g. macro_rules! foo { ($ty:ty) => { foo!($ty); } }
@@ -105,6 +106,7 @@ pub struct CrateDefMap {
     /// However, do we want to put it as a global variable?
     poison_macros: FxHashSet<MacroDefId>,
 
+    local_macros: FxHashMap<Name, MacroDefId>,
     diagnostics: Vec<DefDiagnostic>,
 }
 
@@ -247,7 +249,9 @@ impl CrateDefMap {
                 prelude: None,
                 root,
                 modules,
+                public_macros: FxHashMap::default(),
                 poison_macros: FxHashSet::default(),
+                local_macros: FxHashMap::default(),
                 diagnostics: Vec::new(),
             }
         };
@@ -309,7 +313,7 @@ impl CrateDefMap {
         (res.resolved_def.left().unwrap_or_else(PerNs::none), res.segment_index)
     }
 
-    pub(crate) fn resolve_path_with_macro(
+    fn resolve_path_with_macro(
         &self,
         db: &impl DefDatabase,
         original_module: CrateModuleId,
@@ -319,6 +323,27 @@ impl CrateDefMap {
         (res.resolved_def, res.segment_index)
     }
 
+    // FIXME: This seems to do the same work as `resolve_path_with_macro`, but
+    // using a completely different code path. Seems bad, huh?
+    pub(crate) fn find_macro(
+        &self,
+        db: &impl DefDatabase,
+        original_module: CrateModuleId,
+        path: &Path,
+    ) -> Option<MacroDefId> {
+        let name = path.expand_macro_expr()?;
+        // search local first
+        // FIXME: Remove public_macros check when we have a correct local_macors implementation
+        let local =
+            self.public_macros.get(&name).or_else(|| self.local_macros.get(&name)).map(|it| *it);
+        if local.is_some() {
+            return local;
+        }
+
+        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
+        res.resolved_def.right().map(|m| m.id)
+    }
+
     // Returns Yes if we are sure that additions to `ItemMap` wouldn't change
     // the result.
     fn resolve_path_fp_with_macro(
@@ -486,7 +511,7 @@ impl CrateDefMap {
         let from_scope = self[module]
             .scope
             .get_item_or_macro(name)
-            .unwrap_or_else(|| Either::Left(PerNs::none()));
+            .unwrap_or_else(|| Either::Left(PerNs::none()));;
         let from_extern_prelude =
             self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it));
         let from_prelude = self.resolve_in_prelude(db, name);
diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs
index 99110d58d2..3bfef799da 100644
--- a/crates/ra_hir/src/nameres/collector.rs
+++ b/crates/ra_hir/src/nameres/collector.rs
@@ -138,35 +138,15 @@ where
         }
     }
 
-    fn define_macro(
-        &mut self,
-        module_id: CrateModuleId,
-        name: Name,
-        macro_id: MacroDefId,
-        export: bool,
-    ) {
-        // macro-by-example in Rust have completely weird name resolution logic,
-        // unlike anything else in the language. We'd don't fully implement yet,
-        // just give a somewhat precise approximation.
-        //
-        // Specifically, we store a set of visible macros in each module, just
-        // like how we do with usual items. This is wrong, however, because
-        // macros can be shadowed and their scopes are mostly unrelated to
-        // modules. To paper over the second problem, we also maintain
-        // `global_macro_scope` which works when we construct `CrateDefMap`, but
-        // is completely ignored in expressions.
-        //
-        // What we should do is that, in CrateDefMap, we should maintain a
-        // separate tower of macro scopes, with ids. Then, for each item in the
-        // module, we need to store it's macro scope.
-        let def = Either::Right(MacroDef { id: macro_id });
-
-        // In Rust, `#[macro_export]` macros are unconditionally visible at the
-        // crate root, even if the parent modules is **not** visible.
+    fn define_macro(&mut self, name: Name, macro_id: MacroDefId, export: bool) {
         if export {
-            self.update(self.def_map.root, None, &[(name.clone(), def.clone())]);
+            self.def_map.public_macros.insert(name.clone(), macro_id);
+
+            let def = Either::Right(MacroDef { id: macro_id });
+            self.update(self.def_map.root, None, &[(name.clone(), def)]);
+        } else {
+            self.def_map.local_macros.insert(name.clone(), macro_id);
         }
-        self.update(module_id, None, &[(name.clone(), def)]);
         self.global_macro_scope.insert(name, macro_id);
     }
 
@@ -609,7 +589,7 @@ where
         if is_macro_rules(&mac.path) {
             if let Some(name) = &mac.name {
                 let macro_id = MacroDefId(mac.ast_id.with_file_id(self.file_id));
-                self.def_collector.define_macro(self.module_id, name.clone(), macro_id, mac.export)
+                self.def_collector.define_macro(name.clone(), macro_id, mac.export)
             }
             return;
         }
@@ -714,7 +694,9 @@ mod tests {
                 prelude: None,
                 root,
                 modules,
+                public_macros: FxHashMap::default(),
                 poison_macros: FxHashSet::default(),
+                local_macros: FxHashMap::default(),
                 diagnostics: Vec::new(),
             }
         };
diff --git a/crates/ra_hir/src/resolve.rs b/crates/ra_hir/src/resolve.rs
index 1b987c1b66..d6956f45e8 100644
--- a/crates/ra_hir/src/resolve.rs
+++ b/crates/ra_hir/src/resolve.rs
@@ -2,11 +2,11 @@
 use std::sync::Arc;
 
 use rustc_hash::{FxHashMap, FxHashSet};
-use either::Either;
 
 use crate::{
-    ModuleDef, Trait, MacroDef,
+    ModuleDef, Trait,
     code_model::Crate,
+    MacroDefId,
     db::HirDatabase,
     name::{Name, KnownName},
     nameres::{PerNs, CrateDefMap, CrateModuleId},
@@ -130,16 +130,13 @@ impl Resolver {
         resolution
     }
 
-    pub(crate) fn resolve_path_as_macro(
+    pub(crate) fn resolve_macro_call(
         &self,
         db: &impl HirDatabase,
-        path: &Path,
-    ) -> Option<MacroDef> {
-        let (item_map, module) = self.module()?;
-        match item_map.resolve_path_with_macro(db, module, path) {
-            (Either::Right(macro_def), None) => Some(macro_def),
-            _ => None,
-        }
+        path: Option<Path>,
+    ) -> Option<MacroDefId> {
+        let m = self.module()?;
+        m.0.find_macro(db, m.1, &path?)
     }
 
     /// Returns the resolved path segments
@@ -168,11 +165,7 @@ impl Resolver {
 
     /// Returns the fully resolved path if we were able to resolve it.
     /// otherwise returns `PerNs::none`
-    pub(crate) fn resolve_path_without_assoc_items(
-        &self,
-        db: &impl HirDatabase,
-        path: &Path,
-    ) -> PerNs<Resolution> {
+    pub(crate) fn resolve_path(&self, db: &impl HirDatabase, path: &Path) -> PerNs<Resolution> {
         // into_fully_resolved() returns the fully resolved path or PerNs::none() otherwise
         self.resolve_path_segments(db, path).into_fully_resolved()
     }
diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs
index 876ebe0e34..410064d45a 100644
--- a/crates/ra_hir/src/source_binder.rs
+++ b/crates/ra_hir/src/source_binder.rs
@@ -267,8 +267,9 @@ impl SourceAnalyzer {
         db: &impl HirDatabase,
         macro_call: &ast::MacroCall,
     ) -> Option<MacroDef> {
-        let path = macro_call.path().and_then(Path::from_ast)?;
-        self.resolver.resolve_path_as_macro(db, &path)
+        let id =
+            self.resolver.resolve_macro_call(db, macro_call.path().and_then(Path::from_ast))?;
+        Some(MacroDef { id })
     }
 
     pub fn resolve_hir_path(
@@ -276,7 +277,7 @@ impl SourceAnalyzer {
         db: &impl HirDatabase,
         path: &crate::Path,
     ) -> PerNs<crate::Resolution> {
-        self.resolver.resolve_path_without_assoc_items(db, path)
+        self.resolver.resolve_path(db, path)
     }
 
     pub fn resolve_path(&self, db: &impl HirDatabase, path: &ast::Path) -> Option<PathResolution> {
@@ -293,7 +294,7 @@ impl SourceAnalyzer {
             }
         }
         let hir_path = crate::Path::from_ast(path)?;
-        let res = self.resolver.resolve_path_without_assoc_items(db, &hir_path);
+        let res = self.resolver.resolve_path(db, &hir_path);
         let res = res.clone().take_types().or_else(|| res.take_values())?;
         let res = match res {
             crate::Resolution::Def(it) => PathResolution::Def(it),
diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs
index 6aa727ea10..6cc5dbc6f2 100644
--- a/crates/ra_hir/src/ty/infer.rs
+++ b/crates/ra_hir/src/ty/infer.rs
@@ -610,26 +610,23 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             None => return (Ty::Unknown, None),
         };
         let resolver = &self.resolver;
-        let typable: Option<TypableDef> =
-            // FIXME: this should resolve assoc items as well, see this example:
-            // https://play.rust-lang.org/?gist=087992e9e22495446c01c0d4e2d69521
-            match resolver.resolve_path_without_assoc_items(self.db, &path).take_types() {
-                Some(Resolution::Def(def)) => def.into(),
-                Some(Resolution::LocalBinding(..)) => {
-                    // this cannot happen
-                    log::error!("path resolved to local binding in type ns");
-                    return (Ty::Unknown, None);
-                }
-                Some(Resolution::GenericParam(..)) => {
-                    // generic params can't be used in struct literals
-                    return (Ty::Unknown, None);
-                }
-                Some(Resolution::SelfType(..)) => {
-                    // FIXME this is allowed in an impl for a struct, handle this
-                    return (Ty::Unknown, None);
-                }
-                None => return (Ty::Unknown, None),
-            };
+        let typable: Option<TypableDef> = match resolver.resolve_path(self.db, &path).take_types() {
+            Some(Resolution::Def(def)) => def.into(),
+            Some(Resolution::LocalBinding(..)) => {
+                // this cannot happen
+                log::error!("path resolved to local binding in type ns");
+                return (Ty::Unknown, None);
+            }
+            Some(Resolution::GenericParam(..)) => {
+                // generic params can't be used in struct literals
+                return (Ty::Unknown, None);
+            }
+            Some(Resolution::SelfType(..)) => {
+                // FIXME this is allowed in an impl for a struct, handle this
+                return (Ty::Unknown, None);
+            }
+            None => return (Ty::Unknown, None),
+        };
         let def = match typable {
             None => return (Ty::Unknown, None),
             Some(it) => it,
diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs
index 26c213a416..71cd722345 100644
--- a/crates/ra_hir/src/ty/lower.rs
+++ b/crates/ra_hir/src/ty/lower.rs
@@ -65,7 +65,7 @@ impl Ty {
 
     pub(crate) fn from_hir_path(db: &impl HirDatabase, resolver: &Resolver, path: &Path) -> Self {
         // Resolve the path (in type namespace)
-        let resolution = resolver.resolve_path_without_assoc_items(db, path).take_types();
+        let resolution = resolver.resolve_path(db, path).take_types();
 
         let def = match resolution {
             Some(Resolution::Def(def)) => def,
@@ -216,7 +216,7 @@ impl TraitRef {
         path: &Path,
         explicit_self_ty: Option<Ty>,
     ) -> Option<Self> {
-        let resolved = match resolver.resolve_path_without_assoc_items(db, &path).take_types()? {
+        let resolved = match resolver.resolve_path(db, &path).take_types()? {
             Resolution::Def(ModuleDef::Trait(tr)) => tr,
             _ => return None,
         };
diff --git a/crates/ra_ide_api/src/display.rs b/crates/ra_ide_api/src/display.rs
index 1b06abf94f..f11af0a0b6 100644
--- a/crates/ra_ide_api/src/display.rs
+++ b/crates/ra_ide_api/src/display.rs
@@ -4,14 +4,16 @@
 mod function_signature;
 mod navigation_target;
 mod structure;
+mod short_label;
 
-use crate::db::RootDatabase;
 use ra_syntax::{ast::{self, AstNode, TypeParamsOwner}, SyntaxKind::{ATTR, COMMENT}};
 
 pub use navigation_target::NavigationTarget;
 pub use structure::{StructureNode, file_structure};
 pub use function_signature::FunctionSignature;
 
+pub(crate) use short_label::ShortLabel;
+
 pub(crate) fn function_label(node: &ast::FnDef) -> String {
     FunctionSignature::from(node).to_string()
 }
@@ -73,10 +75,10 @@ where
 
 // FIXME: this should not really use navigation target. Rather, approximately
 // resolved symbol should return a `DefId`.
-pub(crate) fn doc_text_for(db: &RootDatabase, nav: NavigationTarget) -> Option<String> {
-    match (nav.description(db), nav.docs(db)) {
+pub(crate) fn doc_text_for(nav: NavigationTarget) -> Option<String> {
+    match (nav.description(), nav.docs()) {
         (Some(desc), docs) => Some(rust_code_markup_with_doc(desc, docs)),
-        (None, Some(docs)) => Some(docs),
+        (None, Some(docs)) => Some(docs.to_string()),
         _ => None,
     }
 }
diff --git a/crates/ra_ide_api/src/display/navigation_target.rs b/crates/ra_ide_api/src/display/navigation_target.rs
index 45002d098b..983ebe7883 100644
--- a/crates/ra_ide_api/src/display/navigation_target.rs
+++ b/crates/ra_ide_api/src/display/navigation_target.rs
@@ -1,13 +1,14 @@
 use ra_db::{FileId, SourceDatabase};
 use ra_syntax::{
-    SyntaxNode, AstNode, SmolStr, TextRange, TreeArc, AstPtr,
+    SyntaxNode, AstNode, SmolStr, TextRange, AstPtr,
     SyntaxKind::{self, NAME},
-    ast::{self, NameOwner, VisibilityOwner, TypeAscriptionOwner},
+    ast::{self, DocCommentsOwner},
     algo::visit::{visitor, Visitor},
 };
 use hir::{ModuleSource, FieldSource, ImplItem};
 
 use crate::{FileSymbol, db::RootDatabase};
+use super::short_label::ShortLabel;
 
 /// `NavigationTarget` represents and element in the editor's UI which you can
 /// click on to navigate to a particular piece of code.
@@ -22,6 +23,8 @@ pub struct NavigationTarget {
     full_range: TextRange,
     focus_range: Option<TextRange>,
     container_name: Option<SmolStr>,
+    description: Option<String>,
+    docs: Option<String>,
 }
 
 impl NavigationTarget {
@@ -51,6 +54,14 @@ impl NavigationTarget {
         self.full_range
     }
 
+    pub fn docs(&self) -> Option<&str> {
+        self.docs.as_ref().map(String::as_str)
+    }
+
+    pub fn description(&self) -> Option<&str> {
+        self.description.as_ref().map(String::as_str)
+    }
+
     /// A "most interesting" range withing the `full_range`.
     ///
     /// Typically, `full_range` is the whole syntax node,
@@ -60,10 +71,10 @@ impl NavigationTarget {
     }
 
     pub(crate) fn from_bind_pat(file_id: FileId, pat: &ast::BindPat) -> NavigationTarget {
-        NavigationTarget::from_named(file_id, pat)
+        NavigationTarget::from_named(file_id, pat, None, None)
     }
 
-    pub(crate) fn from_symbol(symbol: FileSymbol) -> NavigationTarget {
+    pub(crate) fn from_symbol(db: &RootDatabase, symbol: FileSymbol) -> NavigationTarget {
         NavigationTarget {
             file_id: symbol.file_id,
             name: symbol.name.clone(),
@@ -71,6 +82,8 @@ impl NavigationTarget {
             full_range: symbol.ptr.range(),
             focus_range: symbol.name_range,
             container_name: symbol.container_name.clone(),
+            description: description_from_symbol(db, &symbol),
+            docs: docs_from_symbol(db, &symbol),
         }
     }
 
@@ -84,6 +97,7 @@ impl NavigationTarget {
             ast::PatKind::BindPat(pat) => return NavigationTarget::from_bind_pat(file_id, &pat),
             _ => ("_".into(), pat.syntax_node_ptr().range()),
         };
+
         NavigationTarget {
             file_id,
             name,
@@ -91,6 +105,8 @@ impl NavigationTarget {
             focus_range: None,
             kind: NAME,
             container_name: None,
+            description: None, //< No documentation for Description
+            docs: None,        //< No documentation for Pattern
         }
     }
 
@@ -99,6 +115,7 @@ impl NavigationTarget {
         par: AstPtr<ast::SelfParam>,
     ) -> NavigationTarget {
         let (name, full_range) = ("self".into(), par.syntax_node_ptr().range());
+
         NavigationTarget {
             file_id,
             name,
@@ -106,6 +123,8 @@ impl NavigationTarget {
             focus_range: None,
             kind: NAME,
             container_name: None,
+            description: None, //< No document node for SelfParam
+            docs: None,        //< No document node for SelfParam
         }
     }
 
@@ -115,11 +134,16 @@ impl NavigationTarget {
         let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default();
         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())
+                NavigationTarget::from_syntax(file_id, name, None, node.syntax(), None, None)
             }
+            ModuleSource::Module(node) => NavigationTarget::from_syntax(
+                file_id,
+                name,
+                None,
+                node.syntax(),
+                node.doc_comment_text(),
+                node.short_label(),
+            ),
         }
     }
 
@@ -127,23 +151,37 @@ impl NavigationTarget {
         let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default();
         if let Some((file_id, source)) = module.declaration_source(db) {
             let file_id = file_id.as_original_file();
-            return NavigationTarget::from_syntax(file_id, name, None, source.syntax());
+            return NavigationTarget::from_syntax(
+                file_id,
+                name,
+                None,
+                source.syntax(),
+                source.doc_comment_text(),
+                source.short_label(),
+            );
         }
         NavigationTarget::from_module(db, module)
     }
 
     pub(crate) fn from_function(db: &RootDatabase, func: hir::Function) -> NavigationTarget {
         let (file_id, fn_def) = func.source(db);
-        NavigationTarget::from_named(file_id.original_file(db), &*fn_def)
+        NavigationTarget::from_named(
+            file_id.original_file(db),
+            &*fn_def,
+            fn_def.doc_comment_text(),
+            fn_def.short_label(),
+        )
     }
 
     pub(crate) fn from_field(db: &RootDatabase, field: hir::StructField) -> NavigationTarget {
         let (file_id, field) = field.source(db);
         let file_id = file_id.original_file(db);
         match field {
-            FieldSource::Named(it) => NavigationTarget::from_named(file_id, &*it),
+            FieldSource::Named(it) => {
+                NavigationTarget::from_named(file_id, &*it, it.doc_comment_text(), it.short_label())
+            }
             FieldSource::Pos(it) => {
-                NavigationTarget::from_syntax(file_id, "".into(), None, it.syntax())
+                NavigationTarget::from_syntax(file_id, "".into(), None, it.syntax(), None, None)
             }
         }
     }
@@ -152,15 +190,30 @@ impl NavigationTarget {
         match adt_def {
             hir::AdtDef::Struct(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::AdtDef::Union(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::AdtDef::Enum(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
         }
     }
@@ -174,35 +227,75 @@ impl NavigationTarget {
             hir::ModuleDef::Function(func) => NavigationTarget::from_function(db, func),
             hir::ModuleDef::Struct(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::Union(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::Const(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::Static(s) => {
                 let (file_id, node) = s.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::Enum(e) => {
                 let (file_id, node) = e.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::EnumVariant(var) => {
                 let (file_id, node) = var.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::Trait(e) => {
                 let (file_id, node) = e.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::TypeAlias(e) => {
                 let (file_id, node) = e.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             hir::ModuleDef::BuiltinType(..) => {
                 return None;
@@ -221,6 +314,8 @@ impl NavigationTarget {
             "impl".into(),
             None,
             node.syntax(),
+            None,
+            None,
         )
     }
 
@@ -229,11 +324,21 @@ impl NavigationTarget {
             ImplItem::Method(f) => NavigationTarget::from_function(db, f),
             ImplItem::Const(c) => {
                 let (file_id, node) = c.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
             ImplItem::TypeAlias(a) => {
                 let (file_id, node) = a.source(db);
-                NavigationTarget::from_named(file_id.original_file(db), &*node)
+                NavigationTarget::from_named(
+                    file_id.original_file(db),
+                    &*node,
+                    node.doc_comment_text(),
+                    node.short_label(),
+                )
             }
         }
     }
@@ -241,7 +346,12 @@ impl NavigationTarget {
     pub(crate) fn from_macro_def(db: &RootDatabase, macro_call: hir::MacroDef) -> NavigationTarget {
         let (file_id, node) = macro_call.source(db);
         log::debug!("nav target {}", node.syntax().debug_dump());
-        NavigationTarget::from_named(file_id.original_file(db), &*node)
+        NavigationTarget::from_named(
+            file_id.original_file(db),
+            &*node,
+            node.doc_comment_text(),
+            None,
+        )
     }
 
     #[cfg(test)]
@@ -269,11 +379,16 @@ impl NavigationTarget {
     }
 
     /// Allows `NavigationTarget` to be created from a `NameOwner`
-    pub(crate) fn from_named(file_id: FileId, node: &impl ast::NameOwner) -> NavigationTarget {
+    pub(crate) fn from_named(
+        file_id: FileId,
+        node: &impl ast::NameOwner,
+        docs: Option<String>,
+        description: Option<String>,
+    ) -> NavigationTarget {
         //FIXME: use `_` instead of empty string
         let name = node.name().map(|it| it.text().clone()).unwrap_or_default();
         let focus_range = node.name().map(|it| it.syntax().range());
-        NavigationTarget::from_syntax(file_id, name, focus_range, node.syntax())
+        NavigationTarget::from_syntax(file_id, name, focus_range, node.syntax(), docs, description)
     }
 
     fn from_syntax(
@@ -281,6 +396,8 @@ impl NavigationTarget {
         name: SmolStr,
         focus_range: Option<TextRange>,
         node: &SyntaxNode,
+        docs: Option<String>,
+        description: Option<String>,
     ) -> NavigationTarget {
         NavigationTarget {
             file_id,
@@ -290,83 +407,52 @@ impl NavigationTarget {
             focus_range,
             // ptr: Some(LocalSyntaxPtr::new(node)),
             container_name: None,
+            description,
+            docs,
         }
     }
+}
 
-    pub(crate) fn node(&self, db: &RootDatabase) -> Option<TreeArc<SyntaxNode>> {
-        let source_file = db.parse(self.file_id()).tree;
-        let source_file = source_file.syntax();
-        let node = source_file
-            .descendants()
-            .find(|node| node.kind() == self.kind() && node.range() == self.full_range())?
-            .to_owned();
-        Some(node)
-    }
+fn docs_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> Option<String> {
+    let file = db.parse(symbol.file_id).tree;
+    let node = symbol.ptr.to_node(file.syntax()).to_owned();
 
-    pub(crate) fn docs(&self, db: &RootDatabase) -> Option<String> {
-        let node = self.node(db)?;
-        fn doc_comments<N: ast::DocCommentsOwner>(node: &N) -> Option<String> {
-            node.doc_comment_text()
-        }
-
-        visitor()
-            .visit(doc_comments::<ast::FnDef>)
-            .visit(doc_comments::<ast::StructDef>)
-            .visit(doc_comments::<ast::EnumDef>)
-            .visit(doc_comments::<ast::TraitDef>)
-            .visit(doc_comments::<ast::Module>)
-            .visit(doc_comments::<ast::TypeAliasDef>)
-            .visit(doc_comments::<ast::ConstDef>)
-            .visit(doc_comments::<ast::StaticDef>)
-            .visit(doc_comments::<ast::NamedFieldDef>)
-            .visit(doc_comments::<ast::EnumVariant>)
-            .visit(doc_comments::<ast::MacroCall>)
-            .accept(&node)?
+    fn doc_comments<N: ast::DocCommentsOwner>(node: &N) -> Option<String> {
+        node.doc_comment_text()
     }
 
-    /// Get a description of this node.
-    ///
-    /// e.g. `struct Name`, `enum Name`, `fn Name`
-    pub(crate) fn description(&self, db: &RootDatabase) -> Option<String> {
-        // FIXME: After type inference is done, add type information to improve the output
-        let node = self.node(db)?;
-
-        fn visit_ascribed_node<T>(node: &T, prefix: &str) -> Option<String>
-        where
-            T: NameOwner + VisibilityOwner + TypeAscriptionOwner,
-        {
-            let mut string = visit_node(node, prefix)?;
-
-            if let Some(type_ref) = node.ascribed_type() {
-                string.push_str(": ");
-                type_ref.syntax().text().push_to(&mut string);
-            }
-
-            Some(string)
-        }
-
-        fn visit_node<T>(node: &T, label: &str) -> Option<String>
-        where
-            T: NameOwner + VisibilityOwner,
-        {
-            let mut string =
-                node.visibility().map(|v| format!("{} ", v.syntax().text())).unwrap_or_default();
-            string.push_str(label);
-            string.push_str(node.name()?.text().as_str());
-            Some(string)
-        }
+    visitor()
+        .visit(doc_comments::<ast::FnDef>)
+        .visit(doc_comments::<ast::StructDef>)
+        .visit(doc_comments::<ast::EnumDef>)
+        .visit(doc_comments::<ast::TraitDef>)
+        .visit(doc_comments::<ast::Module>)
+        .visit(doc_comments::<ast::TypeAliasDef>)
+        .visit(doc_comments::<ast::ConstDef>)
+        .visit(doc_comments::<ast::StaticDef>)
+        .visit(doc_comments::<ast::NamedFieldDef>)
+        .visit(doc_comments::<ast::EnumVariant>)
+        .visit(doc_comments::<ast::MacroCall>)
+        .accept(&node)?
+}
 
-        visitor()
-            .visit(|node: &ast::FnDef| Some(crate::display::function_label(node)))
-            .visit(|node: &ast::StructDef| visit_node(node, "struct "))
-            .visit(|node: &ast::EnumDef| visit_node(node, "enum "))
-            .visit(|node: &ast::TraitDef| visit_node(node, "trait "))
-            .visit(|node: &ast::Module| visit_node(node, "mod "))
-            .visit(|node: &ast::TypeAliasDef| visit_node(node, "type "))
-            .visit(|node: &ast::ConstDef| visit_ascribed_node(node, "const "))
-            .visit(|node: &ast::StaticDef| visit_ascribed_node(node, "static "))
-            .visit(|node: &ast::NamedFieldDef| visit_ascribed_node(node, ""))
-            .visit(|node: &ast::EnumVariant| Some(node.name()?.text().to_string()))
-            .accept(&node)?
-    }
+/// Get a description of a symbol.
+///
+/// e.g. `struct Name`, `enum Name`, `fn Name`
+fn description_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> Option<String> {
+    let file = db.parse(symbol.file_id).tree;
+    let node = symbol.ptr.to_node(file.syntax()).to_owned();
+
+    visitor()
+        .visit(|node: &ast::FnDef| node.short_label())
+        .visit(|node: &ast::StructDef| node.short_label())
+        .visit(|node: &ast::EnumDef| node.short_label())
+        .visit(|node: &ast::TraitDef| node.short_label())
+        .visit(|node: &ast::Module| node.short_label())
+        .visit(|node: &ast::TypeAliasDef| node.short_label())
+        .visit(|node: &ast::ConstDef| node.short_label())
+        .visit(|node: &ast::StaticDef| node.short_label())
+        .visit(|node: &ast::NamedFieldDef| node.short_label())
+        .visit(|node: &ast::EnumVariant| node.short_label())
+        .accept(&node)?
 }
diff --git a/crates/ra_ide_api/src/display/short_label.rs b/crates/ra_ide_api/src/display/short_label.rs
new file mode 100644
index 0000000000..dc8245c341
--- /dev/null
+++ b/crates/ra_ide_api/src/display/short_label.rs
@@ -0,0 +1,91 @@
+use ra_syntax::{
+    ast::{self, NameOwner, VisibilityOwner, TypeAscriptionOwner, AstNode},
+};
+
+pub(crate) trait ShortLabel {
+    fn short_label(&self) -> Option<String>;
+}
+
+impl ShortLabel for ast::FnDef {
+    fn short_label(&self) -> Option<String> {
+        Some(crate::display::function_label(self))
+    }
+}
+
+impl ShortLabel for ast::StructDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_node(self, "struct ")
+    }
+}
+
+impl ShortLabel for ast::EnumDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_node(self, "enum ")
+    }
+}
+
+impl ShortLabel for ast::TraitDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_node(self, "trait ")
+    }
+}
+
+impl ShortLabel for ast::Module {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_node(self, "mod ")
+    }
+}
+
+impl ShortLabel for ast::TypeAliasDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_node(self, "type ")
+    }
+}
+
+impl ShortLabel for ast::ConstDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_ascribed_node(self, "const ")
+    }
+}
+
+impl ShortLabel for ast::StaticDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_ascribed_node(self, "static ")
+    }
+}
+
+impl ShortLabel for ast::NamedFieldDef {
+    fn short_label(&self) -> Option<String> {
+        short_label_from_ascribed_node(self, "")
+    }
+}
+
+impl ShortLabel for ast::EnumVariant {
+    fn short_label(&self) -> Option<String> {
+        Some(self.name()?.text().to_string())
+    }
+}
+
+fn short_label_from_ascribed_node<T>(node: &T, prefix: &str) -> Option<String>
+where
+    T: NameOwner + VisibilityOwner + TypeAscriptionOwner,
+{
+    let mut buf = short_label_from_node(node, prefix)?;
+
+    if let Some(type_ref) = node.ascribed_type() {
+        buf.push_str(": ");
+        type_ref.syntax().text().push_to(&mut buf);
+    }
+
+    Some(buf)
+}
+
+fn short_label_from_node<T>(node: &T, label: &str) -> Option<String>
+where
+    T: NameOwner + VisibilityOwner,
+{
+    let mut buf = node.visibility().map(|v| format!("{} ", v.syntax().text())).unwrap_or_default();
+    buf.push_str(label);
+    buf.push_str(node.name()?.text().as_str());
+    Some(buf)
+}
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index e72b7a6e7a..325a5a4f3b 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -1,6 +1,6 @@
 use ra_db::{FileId, SourceDatabase};
 use ra_syntax::{
-    AstNode, ast,
+    AstNode, ast::{self, DocCommentsOwner},
     algo::{
         find_node_at_offset,
         visit::{visitor, Visitor},
@@ -13,6 +13,7 @@ use crate::{
     db::RootDatabase,
     RangeInfo,
     name_ref_kind::{NameRefKind::*, classify_name_ref},
+    display::ShortLabel,
 };
 
 pub(crate) fn goto_definition(
@@ -82,7 +83,7 @@ pub(crate) fn reference_definition(
     // Fallback index based approach:
     let navs = crate::symbol_index::index_resolve(db, name_ref)
         .into_iter()
-        .map(NavigationTarget::from_symbol)
+        .map(|s| NavigationTarget::from_symbol(db, s))
         .collect();
     Approximate(navs)
 }
@@ -114,17 +115,39 @@ pub(crate) fn name_definition(
 
 fn named_target(file_id: FileId, node: &SyntaxNode) -> Option<NavigationTarget> {
     visitor()
-        .visit(|node: &ast::StructDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::EnumDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::EnumVariant| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::FnDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::TypeAliasDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::ConstDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::StaticDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::TraitDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::NamedFieldDef| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::Module| NavigationTarget::from_named(file_id, node))
-        .visit(|node: &ast::MacroCall| NavigationTarget::from_named(file_id, node))
+        .visit(|node: &ast::StructDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::EnumDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::EnumVariant| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::FnDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::TypeAliasDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::ConstDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::StaticDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::TraitDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::NamedFieldDef| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::Module| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), node.short_label())
+        })
+        .visit(|node: &ast::MacroCall| {
+            NavigationTarget::from_named(file_id, node, node.doc_comment_text(), None)
+        })
         .accept(node)
 }
 
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index f56965ef5a..cb676eb12a 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -86,13 +86,13 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
         use crate::goto_definition::{ReferenceResult::*, reference_definition};
         let ref_result = reference_definition(db, position.file_id, name_ref);
         match ref_result {
-            Exact(nav) => res.extend(doc_text_for(db, nav)),
+            Exact(nav) => res.extend(doc_text_for(nav)),
             Approximate(navs) => {
                 // We are no longer exact
                 res.exact = false;
 
                 for nav in navs {
-                    res.extend(doc_text_for(db, nav))
+                    res.extend(doc_text_for(nav))
                 }
             }
         }
@@ -104,7 +104,7 @@ pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeIn
 
         if let Some(navs) = navs {
             for nav in navs {
-                res.extend(doc_text_for(db, nav))
+                res.extend(doc_text_for(nav))
             }
         }
 
diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs
index 2fe46cd133..dbebf50a62 100644
--- a/crates/ra_ide_api/src/lib.rs
+++ b/crates/ra_ide_api/src/lib.rs
@@ -393,7 +393,7 @@ impl Analysis {
         self.with_db(|db| {
             symbol_index::world_symbols(db, query)
                 .into_iter()
-                .map(NavigationTarget::from_symbol)
+                .map(|s| NavigationTarget::from_symbol(db, s))
                 .collect::<Vec<_>>()
         })
     }


Test patch:
diff --git a/crates/ra_hir/src/nameres/tests/macros.rs b/crates/ra_hir/src/nameres/tests/macros.rs
index 4e04740eb9..42241aeff7 100644
--- a/crates/ra_hir/src/nameres/tests/macros.rs
+++ b/crates/ra_hir/src/nameres/tests/macros.rs
@@ -21,7 +21,6 @@ fn macro_rules_are_globally_visible() {
    ⋮crate
    ⋮Foo: t v
    ⋮nested: t
-   ⋮structs: m
    ⋮
    ⋮crate::nested
    ⋮Bar: t v
@@ -47,7 +46,6 @@ fn macro_rules_can_define_modules() {
     );
     assert_snapshot_matches!(map, @r###"
    ⋮crate
-   ⋮m: m
    ⋮n1: t
    ⋮
    ⋮crate::n1
@@ -129,11 +127,8 @@ fn unexpanded_macro_should_expand_by_fixedpoint_loop() {
             "foo": ("/lib.rs", []),
         },
     );
-    assert_snapshot_matches!(map, @r###"
-   ⋮crate
-   ⋮Foo: t v
-   ⋮bar: m
-   ⋮baz: m
-   ⋮foo: m
-    "###);
+    assert_snapshot_matches!(map, @r###"crate
+Foo: t v
+bar: m
+foo: m"###);
 }
diff --git a/crates/ra_hir/src/ty/tests.rs b/crates/ra_hir/src/ty/tests.rs
index 54b2a8c162..c34e89af71 100644
--- a/crates/ra_hir/src/ty/tests.rs
+++ b/crates/ra_hir/src/ty/tests.rs
@@ -2481,10 +2481,8 @@ fn main() {
 }
 "#),
         @r###"
-   ⋮
-   ⋮[156; 182) '{     ...,2); }': ()
-   ⋮[166; 167) 'x': Foo
-    "###
+[156; 182) '{     ...,2); }': ()
+[166; 167) 'x': Foo"###
     );
 }