Merge #912

912: Make goto definition/hover work for associated items r=matklad a=kjeremy

Just functions so far. Looking for comments.

Fixes #911
Towards #832

Co-authored-by: kjeremy <[email protected]>
Co-authored-by: Jeremy Kolb <[email protected]>

4 files changed, 116 insertions, 24 deletions

diff --git a/crates/ra_hir/src/ty/infer.rs b/crates/ra_hir/src/ty/infer.rs
index 5e4d49ffb..268d2c110 100644
--- a/crates/ra_hir/src/ty/infer.rs
+++ b/crates/ra_hir/src/ty/infer.rs
@@ -29,6 +29,7 @@ use crate::{
     Function, StructField, Path, Name,
     FnSignature, AdtDef,
     HirDatabase,
+    ImplItem,
     type_ref::{TypeRef, Mutability},
     expr::{Body, Expr, BindingAnnotation, Literal, ExprId, Pat, PatId, UnaryOp, BinaryOp, Statement, FieldPat, self},
     generics::GenericParams,
@@ -54,6 +55,14 @@ pub fn infer(db: &impl HirDatabase, func: Function) -> Arc<InferenceResult> {
     Arc::new(ctx.resolve_all())
 }
 
+#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
+enum ExprOrPatId {
+    ExprId(ExprId),
+    PatId(PatId),
+}
+
+impl_froms!(ExprOrPatId: ExprId, PatId);
+
 /// The result of type inference: A mapping from expressions and patterns to types.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct InferenceResult {
@@ -61,6 +70,8 @@ pub struct InferenceResult {
     method_resolutions: FxHashMap<ExprId, Function>,
     /// For each field access expr, records the field it resolves to.
     field_resolutions: FxHashMap<ExprId, StructField>,
+    /// For each associated item record what it resolves to
+    assoc_resolutions: FxHashMap<ExprOrPatId, ImplItem>,
     pub(super) type_of_expr: ArenaMap<ExprId, Ty>,
     pub(super) type_of_pat: ArenaMap<PatId, Ty>,
 }
@@ -72,6 +83,12 @@ impl InferenceResult {
     pub fn field_resolution(&self, expr: ExprId) -> Option<StructField> {
         self.field_resolutions.get(&expr).map(|it| *it)
     }
+    pub fn assoc_resolutions_for_expr(&self, id: ExprId) -> Option<ImplItem> {
+        self.assoc_resolutions.get(&id.into()).map(|it| *it)
+    }
+    pub fn assoc_resolutions_for_pat(&self, id: PatId) -> Option<ImplItem> {
+        self.assoc_resolutions.get(&id.into()).map(|it| *it)
+    }
 }
 
 impl Index<ExprId> for InferenceResult {
@@ -99,6 +116,7 @@ struct InferenceContext<'a, D: HirDatabase> {
     var_unification_table: InPlaceUnificationTable<TypeVarId>,
     method_resolutions: FxHashMap<ExprId, Function>,
     field_resolutions: FxHashMap<ExprId, StructField>,
+    assoc_resolutions: FxHashMap<ExprOrPatId, ImplItem>,
     type_of_expr: ArenaMap<ExprId, Ty>,
     type_of_pat: ArenaMap<PatId, Ty>,
     /// The return type of the function being inferred.
@@ -110,6 +128,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         InferenceContext {
             method_resolutions: FxHashMap::default(),
             field_resolutions: FxHashMap::default(),
+            assoc_resolutions: FxHashMap::default(),
             type_of_expr: ArenaMap::default(),
             type_of_pat: ArenaMap::default(),
             var_unification_table: InPlaceUnificationTable::new(),
@@ -135,6 +154,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         InferenceResult {
             method_resolutions: self.method_resolutions,
             field_resolutions: self.field_resolutions,
+            assoc_resolutions: self.assoc_resolutions,
             type_of_expr: expr_types,
             type_of_pat: pat_types,
         }
@@ -152,6 +172,10 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         self.field_resolutions.insert(expr, field);
     }
 
+    fn write_assoc_resolution(&mut self, id: ExprOrPatId, item: ImplItem) {
+        self.assoc_resolutions.insert(id, item);
+    }
+
     fn write_pat_ty(&mut self, pat: PatId, ty: Ty) {
         self.type_of_pat.insert(pat, ty);
     }
@@ -341,7 +365,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
         })
     }
 
-    fn infer_path_expr(&mut self, resolver: &Resolver, path: &Path) -> Option<Ty> {
+    fn infer_path_expr(&mut self, resolver: &Resolver, path: &Path, id: ExprOrPatId) -> Option<Ty> {
         let resolved = resolver.resolve_path_segments(self.db, &path);
 
         let (def, remaining_index) = resolved.into_inner();
@@ -393,26 +417,38 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             // Attempt to find an impl_item for the type which has a name matching
             // the current segment
             log::debug!("looking for path segment: {:?}", segment);
-            let item: crate::ModuleDef = ty.iterate_impl_items(self.db, |item| match item {
-                crate::ImplItem::Method(func) => {
-                    let sig = func.signature(self.db);
-                    if segment.name == *sig.name() {
-                        return Some(func.into());
+            let item: crate::ModuleDef = ty.iterate_impl_items(self.db, |item| {
+                let matching_def: Option<crate::ModuleDef> = match item {
+                    crate::ImplItem::Method(func) => {
+                        let sig = func.signature(self.db);
+                        if segment.name == *sig.name() {
+                            Some(func.into())
+                        } else {
+                            None
+                        }
                     }
-                    None
-                }
 
-                crate::ImplItem::Const(konst) => {
-                    let sig = konst.signature(self.db);
-                    if segment.name == *sig.name() {
-                        return Some(konst.into());
+                    crate::ImplItem::Const(konst) => {
+                        let sig = konst.signature(self.db);
+                        if segment.name == *sig.name() {
+                            Some(konst.into())
+                        } else {
+                            None
+                        }
                     }
-                    None
-                }
 
-                // TODO: Resolve associated types
-                crate::ImplItem::TypeAlias(_) => None,
+                    // TODO: Resolve associated types
+                    crate::ImplItem::TypeAlias(_) => None,
+                };
+                match matching_def {
+                    Some(_) => {
+                        self.write_assoc_resolution(id, item);
+                        return matching_def;
+                    }
+                    None => None,
+                }
             })?;
+
             resolved = Resolution::Def(item.into());
         }
 
@@ -420,7 +456,6 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             Resolution::Def(def) => {
                 let typable: Option<TypableDef> = def.into();
                 let typable = typable?;
-
                 let substs = Ty::substs_from_path(self.db, &self.resolver, path, typable);
                 let ty = self.db.type_for_def(typable, Namespace::Values).apply_substs(substs);
                 let ty = self.insert_type_vars(ty);
@@ -572,7 +607,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             Pat::Path(path) => {
                 // TODO use correct resolver for the surrounding expression
                 let resolver = self.resolver.clone();
-                self.infer_path_expr(&resolver, &path).unwrap_or(Ty::Unknown)
+                self.infer_path_expr(&resolver, &path, pat.into()).unwrap_or(Ty::Unknown)
             }
             Pat::Bind { mode, name: _name, subpat } => {
                 let inner_ty = if let Some(subpat) = subpat {
@@ -782,7 +817,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             Expr::Path(p) => {
                 // TODO this could be more efficient...
                 let resolver = expr::resolver_for_expr(self.body.clone(), self.db, tgt_expr);
-                self.infer_path_expr(&resolver, p).unwrap_or(Ty::Unknown)
+                self.infer_path_expr(&resolver, p, tgt_expr.into()).unwrap_or(Ty::Unknown)
             }
             Expr::Continue => Ty::Never,
             Expr::Break { expr } => {
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index 9ec179593..364263d9b 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -47,9 +47,10 @@ pub(crate) fn reference_definition(
     name_ref: &ast::NameRef,
 ) -> ReferenceResult {
     use self::ReferenceResult::*;
-    if let Some(function) =
-        hir::source_binder::function_from_child_node(db, file_id, name_ref.syntax())
-    {
+
+    let function = hir::source_binder::function_from_child_node(db, file_id, name_ref.syntax());
+
+    if let Some(function) = function {
         // Check if it is a method
         if let Some(method_call) = name_ref.syntax().parent().and_then(ast::MethodCallExpr::cast) {
             tested_by!(goto_definition_works_for_methods);
@@ -122,9 +123,29 @@ pub(crate) fn reference_definition(
             Some(Resolution::SelfType(_impl_block)) => {
                 // TODO: go to the implemented type
             }
-            None => {}
+            None => {
+                // If we failed to resolve then check associated items
+                if let Some(function) = function {
+                    // Should we do this above and then grab path from the PathExpr?
+                    if let Some(path_expr) =
+                        name_ref.syntax().ancestors().find_map(ast::PathExpr::cast)
+                    {
+                        let infer_result = function.infer(db);
+                        let source_map = function.body_source_map(db);
+                        let expr = ast::Expr::cast(path_expr.syntax()).unwrap();
+
+                        if let Some(res) = source_map
+                            .node_expr(expr)
+                            .and_then(|it| infer_result.assoc_resolutions_for_expr(it.into()))
+                        {
+                            return Exact(NavigationTarget::from_impl_item(db, res));
+                        }
+                    }
+                }
+            }
         }
     }
+
     // If that fails try the index based approach.
     let navs = crate::symbol_index::index_resolve(db, name_ref)
         .into_iter()
diff --git a/crates/ra_ide_api/src/hover.rs b/crates/ra_ide_api/src/hover.rs
index 8ec60090d..bcd052c8b 100644
--- a/crates/ra_ide_api/src/hover.rs
+++ b/crates/ra_ide_api/src/hover.rs
@@ -512,4 +512,26 @@ mod tests {
         let hover = analysis.hover(position).unwrap().unwrap();
         assert_eq!(trim_markup_opt(hover.info.first()), Some("Thing"));
     }
+
+    #[test]
+    fn test_hover_infer_associated_method_exact() {
+        let (analysis, position) = single_file_with_position(
+            "
+            struct Thing { x: u32 }
+
+            impl Thing {
+                fn new() -> Thing {
+                    Thing { x: 0 }
+                }
+            }
+
+            fn main() {
+                let foo_test = Thing::new<|>();
+            }
+            ",
+        );
+        let hover = analysis.hover(position).unwrap().unwrap();
+        assert_eq!(trim_markup_opt(hover.info.first()), Some("fn new() -> Thing"));
+        assert_eq!(hover.info.is_exact(), true);
+    }
 }
diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs
index 6538081ac..d806cb368 100644
--- a/crates/ra_ide_api/src/navigation_target.rs
+++ b/crates/ra_ide_api/src/navigation_target.rs
@@ -3,7 +3,7 @@ use ra_syntax::{
     SyntaxNode, SyntaxNodePtr, AstNode, SmolStr, TextRange, ast,
     SyntaxKind::{self, NAME},
 };
-use hir::{ModuleSource, FieldSource, Name};
+use hir::{ModuleSource, FieldSource, Name, ImplItem};
 
 use crate::{FileSymbol, db::RootDatabase};
 
@@ -174,6 +174,20 @@ impl NavigationTarget {
         )
     }
 
+    pub(crate) fn from_impl_item(db: &RootDatabase, impl_item: hir::ImplItem) -> NavigationTarget {
+        match impl_item {
+            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)
+            }
+            ImplItem::TypeAlias(a) => {
+                let (file_id, node) = a.source(db);
+                NavigationTarget::from_named(file_id.original_file(db), &*node)
+            }
+        }
+    }
+
     #[cfg(test)]
     pub(crate) fn assert_match(&self, expected: &str) {
         let actual = self.debug_render();