Merge #1389

1389: Refactor hover to not depend on nav target r=matklad a=edwin0cheng



Co-authored-by: Edwin Cheng <[email protected]>

3 files changed, 165 insertions, 30 deletions

diff --git a/crates/ra_ide_api/src/display.rs b/crates/ra_ide_api/src/display.rs
index f11af0a0b..882518838 100644
--- a/crates/ra_ide_api/src/display.rs
+++ b/crates/ra_ide_api/src/display.rs
@@ -13,6 +13,7 @@ pub use structure::{StructureNode, file_structure};
 pub use function_signature::FunctionSignature;
 
 pub(crate) use short_label::ShortLabel;
+pub(crate) use navigation_target::{docs_from_symbol, description_from_symbol};
 
 pub(crate) fn function_label(node: &ast::FnDef) -> String {
     FunctionSignature::from(node).to_string()
@@ -72,13 +73,3 @@ where
         format!("```rust\n{}\n```", val.as_ref())
     }
 }
-
-// FIXME: this should not really use navigation target. Rather, approximately
-// resolved symbol should return a `DefId`.
-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.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 983ebe788..cfd3f5478 100644
--- a/crates/ra_ide_api/src/display/navigation_target.rs
+++ b/crates/ra_ide_api/src/display/navigation_target.rs
@@ -413,7 +413,7 @@ impl NavigationTarget {
     }
 }
 
-fn docs_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> Option<String> {
+pub(crate) 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();
 
@@ -439,7 +439,7 @@ fn docs_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> Option<String> {
 /// Get a description of a symbol.
 ///
 /// e.g. `struct Name`, `enum Name`, `fn Name`
-fn description_from_symbol(db: &RootDatabase, symbol: &FileSymbol) -> Option<String> {
+pub(crate) 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();
 
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index cb676eb12..fbabeb194 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -1,14 +1,15 @@
 use ra_db::SourceDatabase;
 use ra_syntax::{
-    AstNode, ast,
-    algo::{find_covering_element, find_node_at_offset, ancestors_at_offset},
+    AstNode, ast::{self, DocCommentsOwner},
+    algo::{find_covering_element, find_node_at_offset, ancestors_at_offset, visit::{visitor, Visitor}},
 };
 use hir::HirDisplay;
 
 use crate::{
     db::RootDatabase,
     RangeInfo, FilePosition, FileRange,
-    display::{rust_code_markup, doc_text_for},
+    display::{rust_code_markup, rust_code_markup_with_doc, ShortLabel, docs_from_symbol, description_from_symbol},
+    name_ref_kind::{NameRefKind::*, classify_name_ref},
 };
 
 /// Contains the results when hovering over an item
@@ -77,34 +78,177 @@ impl HoverResult {
     }
 }
 
+fn hover_text(docs: Option<String>, desc: Option<String>) -> Option<String> {
+    match (desc, docs) {
+        (Some(desc), docs) => Some(rust_code_markup_with_doc(desc, docs)),
+        (None, Some(docs)) => Some(docs.to_string()),
+        _ => None,
+    }
+}
+
 pub(crate) fn hover(db: &RootDatabase, position: FilePosition) -> Option<RangeInfo<HoverResult>> {
     let file = db.parse(position.file_id).tree;
     let mut res = HoverResult::new();
 
     let mut range = None;
     if let Some(name_ref) = find_node_at_offset::<ast::NameRef>(file.syntax(), position.offset) {
-        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(nav)),
-            Approximate(navs) => {
-                // We are no longer exact
-                res.exact = false;
-
-                for nav in navs {
-                    res.extend(doc_text_for(nav))
+        let analyzer = hir::SourceAnalyzer::new(db, position.file_id, name_ref.syntax(), None);
+
+        match classify_name_ref(db, &analyzer, name_ref) {
+            Some(Method(it)) => {
+                let it = it.source(db).1;
+                res.extend(hover_text(it.doc_comment_text(), it.short_label()));
+            }
+            Some(Macro(it)) => {
+                let it = it.source(db).1;
+                res.extend(hover_text(it.doc_comment_text(), None));
+            }
+            Some(FieldAccess(it)) => {
+                let it = it.source(db).1;
+                if let hir::FieldSource::Named(it) = it {
+                    res.extend(hover_text(it.doc_comment_text(), it.short_label()));
+                }
+            }
+            Some(AssocItem(it)) => match it {
+                hir::ImplItem::Method(it) => {
+                    let it = it.source(db).1;
+                    res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                }
+                hir::ImplItem::Const(it) => {
+                    let it = it.source(db).1;
+                    res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                }
+                hir::ImplItem::TypeAlias(it) => {
+                    let it = it.source(db).1;
+                    res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                }
+            },
+            Some(Def(it)) => {
+                match it {
+                    hir::ModuleDef::Module(it) => {
+                        let it = it.definition_source(db).1;
+                        if let hir::ModuleSource::Module(it) = it {
+                            res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                        }
+                    }
+                    hir::ModuleDef::Function(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::Struct(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::Union(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::Enum(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::EnumVariant(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::Const(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::Static(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::Trait(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::TypeAlias(it) => {
+                        let it = it.source(db).1;
+                        res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                    }
+                    hir::ModuleDef::BuiltinType(_) => {
+                        // FIXME: hover for builtin Type ?
+                    }
                 }
             }
+            Some(SelfType(ty)) => {
+                if let Some((adt_def, _)) = ty.as_adt() {
+                    match adt_def {
+                        hir::AdtDef::Struct(it) => {
+                            let it = it.source(db).1;
+                            res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                        }
+                        hir::AdtDef::Union(it) => {
+                            let it = it.source(db).1;
+                            res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                        }
+                        hir::AdtDef::Enum(it) => {
+                            let it = it.source(db).1;
+                            res.extend(hover_text(it.doc_comment_text(), it.short_label()))
+                        }
+                    }
+                }
+            }
+            Some(Pat(_)) => {
+                res.extend(None);
+            }
+            Some(SelfParam(_)) => {
+                res.extend(None);
+            }
+            Some(GenericParam(_)) => {
+                // FIXME: Hover for generic param
+            }
+            None => {}
         }
+
+        if res.is_empty() {
+            // Fallback index based approach:
+            let symbols = crate::symbol_index::index_resolve(db, name_ref);
+            for sym in symbols {
+                let docs = docs_from_symbol(db, &sym);
+                let desc = description_from_symbol(db, &sym);
+                res.extend(hover_text(docs, desc));
+            }
+        }
+
         if !res.is_empty() {
             range = Some(name_ref.syntax().range())
         }
     } else if let Some(name) = find_node_at_offset::<ast::Name>(file.syntax(), position.offset) {
-        let navs = crate::goto_definition::name_definition(db, position.file_id, name);
-
-        if let Some(navs) = navs {
-            for nav in navs {
-                res.extend(doc_text_for(nav))
+        if let Some(parent) = name.syntax().parent() {
+            let text = visitor()
+                .visit(|node: &ast::StructDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::EnumDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::EnumVariant| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::FnDef| hover_text(node.doc_comment_text(), node.short_label()))
+                .visit(|node: &ast::TypeAliasDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::ConstDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::StaticDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::TraitDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::NamedFieldDef| {
+                    hover_text(node.doc_comment_text(), node.short_label())
+                })
+                .visit(|node: &ast::Module| hover_text(node.doc_comment_text(), node.short_label()))
+                .visit(|node: &ast::MacroCall| hover_text(node.doc_comment_text(), None))
+                .accept(parent);
+
+            if let Some(text) = text {
+                res.extend(text);
             }
         }