Merge #693

693: Name resolution refactoring r=matklad a=flodiebold

This is still very WIP, but it's becoming quite big and I want to make sure this isn't going in a completely bad direction :sweat_smile:. I'm not really happy with how the path resolution looks, and I'm not sure `PerNs<Resolution>` is the best return type -- there are 'this cannot happen in the (types/values) namespace' cases everywhere. I also want to unify the `resolver` and `nameres` namespaces once I'm done switching everything to `Resolver`. Also, `Resolver` only has a lifetime because it needs to have a reference to the `ItemMap` during import resolution :confused: 

The differences in the completion snapshots are almost completely just ordering (except it completes `Self` as well now), so I changed it to sort the completions before snapshotting.

Co-authored-by: Florian Diebold <[email protected]>

9 files changed, 215 insertions, 104 deletions

diff --git a/crates/ra_ide_api/src/completion/complete_path.rs b/crates/ra_ide_api/src/completion/complete_path.rs
index b33ddcde5..0b9948d4b 100644
--- a/crates/ra_ide_api/src/completion/complete_path.rs
+++ b/crates/ra_ide_api/src/completion/complete_path.rs
@@ -1,21 +1,21 @@
 use join_to_string::join;
 
+use hir::{Docs, Resolution};
+
 use crate::{
     completion::{CompletionItem, CompletionItemKind, Completions, CompletionKind, CompletionContext},
 };
 
-use hir::Docs;
-
 pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) {
-    let (path, module) = match (&ctx.path_prefix, &ctx.module) {
-        (Some(path), Some(module)) => (path.clone(), module),
+    let path = match &ctx.path_prefix {
+        Some(path) => path.clone(),
         _ => return,
     };
-    let def_id = match module.resolve_path(ctx.db, &path).take_types() {
-        Some(it) => it,
-        None => return,
+    let def = match ctx.resolver.resolve_path(ctx.db, &path).take_types() {
+        Some(Resolution::Def(def)) => def,
+        _ => return,
     };
-    match def_id {
+    match def {
         hir::ModuleDef::Module(module) => {
             let module_scope = module.scope(ctx.db);
             for (name, res) in module_scope.entries() {
@@ -24,7 +24,7 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) {
                     ctx.source_range(),
                     name.to_string(),
                 )
-                .from_resolution(ctx, res)
+                .from_resolution(ctx, &res.def.map(hir::Resolution::Def))
                 .add_to(acc);
             }
         }
@@ -66,6 +66,17 @@ mod tests {
     }
 
     #[test]
+    #[ignore] // should not complete foo, which currently doesn't work
+    fn dont_complete_current_use() {
+        check_reference_completion(
+            "dont_complete_current_use",
+            r"
+            use self::foo<|>;
+            ",
+        );
+    }
+
+    #[test]
     fn completes_mod_with_docs() {
         check_reference_completion(
             "mod_with_docs",
diff --git a/crates/ra_ide_api/src/completion/complete_scope.rs b/crates/ra_ide_api/src/completion/complete_scope.rs
index f837bb1db..44514ab2b 100644
--- a/crates/ra_ide_api/src/completion/complete_scope.rs
+++ b/crates/ra_ide_api/src/completion/complete_scope.rs
@@ -1,63 +1,20 @@
-use rustc_hash::FxHashSet;
-use ra_syntax::ast::AstNode;
-use crate::completion::{CompletionItem, CompletionItemKind, Completions, CompletionKind, CompletionContext};
+use crate::completion::{CompletionItem, Completions, CompletionKind, CompletionContext};
 
 pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) {
     if !ctx.is_trivial_path {
         return;
     }
-    let module = match &ctx.module {
-        Some(it) => it,
-        None => return,
-    };
-    if let Some(function) = &ctx.function {
-        let scopes = function.scopes(ctx.db);
-        complete_fn(acc, &scopes, ctx);
-    }
-
-    let module_scope = module.scope(ctx.db);
-    module_scope
-        .entries()
-        .filter(|(_name, res)| {
-            // For cases like `use self::foo<|>` don't suggest foo itself.
-            match res.import {
-                None => true,
-                Some(import) => {
-                    let source = module.import_source(ctx.db, import);
-                    !source.syntax().range().is_subrange(&ctx.leaf.range())
-                }
-            }
-        })
-        .for_each(|(name, res)| {
-            CompletionItem::new(
-                CompletionKind::Reference,
-                ctx.source_range(),
-                name.to_string(),
-            )
-            .from_resolution(ctx, res)
-            .add_to(acc)
-        });
-}
+    let names = ctx.resolver.all_names();
 
-fn complete_fn(
-    acc: &mut Completions,
-    scopes: &hir::ScopesWithSyntaxMapping,
-    ctx: &CompletionContext,
-) {
-    let mut shadowed = FxHashSet::default();
-    scopes
-        .scope_chain_for_offset(ctx.offset)
-        .flat_map(|scope| scopes.scopes.entries(scope).iter())
-        .filter(|entry| shadowed.insert(entry.name()))
-        .for_each(|entry| {
-            CompletionItem::new(
-                CompletionKind::Reference,
-                ctx.source_range(),
-                entry.name().to_string(),
-            )
-            .kind(CompletionItemKind::Binding)
-            .add_to(acc)
-        });
+    names.into_iter().for_each(|(name, res)| {
+        CompletionItem::new(
+            CompletionKind::Reference,
+            ctx.source_range(),
+            name.to_string(),
+        )
+        .from_resolution(ctx, &res)
+        .add_to(acc)
+    });
 }
 
 #[cfg(test)]
@@ -116,6 +73,30 @@ mod tests {
     }
 
     #[test]
+    fn completes_generic_params() {
+        check_reference_completion(
+            "generic_params",
+            r"
+            fn quux<T>() {
+                <|>
+            }
+            ",
+        );
+    }
+
+    #[test]
+    fn completes_generic_params_in_struct() {
+        check_reference_completion(
+            "generic_params_in_struct",
+            r"
+            struct X<T> {
+                x: <|>
+            }
+            ",
+        );
+    }
+
+    #[test]
     fn completes_module_items() {
         check_reference_completion(
             "module_items",
@@ -174,5 +155,4 @@ mod tests {
     fn completes_self_in_methods() {
         check_reference_completion("self_in_methods", r"impl S { fn foo(&self) { <|> } }")
     }
-
 }
diff --git a/crates/ra_ide_api/src/completion/completion_context.rs b/crates/ra_ide_api/src/completion/completion_context.rs
index 578af6e5b..5d1851da6 100644
--- a/crates/ra_ide_api/src/completion/completion_context.rs
+++ b/crates/ra_ide_api/src/completion/completion_context.rs
@@ -5,7 +5,7 @@ use ra_syntax::{
     algo::{find_leaf_at_offset, find_covering_node, find_node_at_offset},
     SyntaxKind::*,
 };
-use hir::source_binder;
+use hir::{source_binder, Resolver};
 
 use crate::{db, FilePosition};
 
@@ -16,6 +16,7 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) db: &'a db::RootDatabase,
     pub(super) offset: TextUnit,
     pub(super) leaf: &'a SyntaxNode,
+    pub(super) resolver: Resolver,
     pub(super) module: Option<hir::Module>,
     pub(super) function: Option<hir::Function>,
     pub(super) function_syntax: Option<&'a ast::FnDef>,
@@ -42,12 +43,14 @@ impl<'a> CompletionContext<'a> {
         original_file: &'a SourceFile,
         position: FilePosition,
     ) -> Option<CompletionContext<'a>> {
+        let resolver = source_binder::resolver_for_position(db, position);
         let module = source_binder::module_from_position(db, position);
         let leaf = find_leaf_at_offset(original_file.syntax(), position.offset).left_biased()?;
         let mut ctx = CompletionContext {
             db,
             leaf,
             offset: position.offset,
+            resolver,
             module,
             function: None,
             function_syntax: None,
diff --git a/crates/ra_ide_api/src/completion/completion_item.rs b/crates/ra_ide_api/src/completion/completion_item.rs
index d3bc14894..bada6a33b 100644
--- a/crates/ra_ide_api/src/completion/completion_item.rs
+++ b/crates/ra_ide_api/src/completion/completion_item.rs
@@ -1,5 +1,7 @@
-use hir::{Docs, Documentation};
-use ra_syntax::TextRange;
+use hir::{Docs, Documentation, PerNs, Resolution};
+use ra_syntax::{
+    TextRange,
+};
 use ra_text_edit::TextEdit;
 use test_utils::tested_by;
 
@@ -48,6 +50,7 @@ pub enum CompletionItemKind {
     Trait,
     TypeAlias,
     Method,
+    TypeParam,
 }
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
@@ -207,23 +210,34 @@ impl Builder {
     pub(super) fn from_resolution(
         mut self,
         ctx: &CompletionContext,
-        resolution: &hir::Resolution,
+        resolution: &PerNs<Resolution>,
     ) -> Builder {
-        let def = resolution.def.take_types().or(resolution.def.take_values());
+        use hir::ModuleDef::*;
+
+        let def = resolution
+            .as_ref()
+            .take_types()
+            .or(resolution.as_ref().take_values());
         let def = match def {
             None => return self,
             Some(it) => it,
         };
         let (kind, docs) = match def {
-            hir::ModuleDef::Module(it) => (CompletionItemKind::Module, it.docs(ctx.db)),
-            hir::ModuleDef::Function(func) => return self.from_function(ctx, func),
-            hir::ModuleDef::Struct(it) => (CompletionItemKind::Struct, it.docs(ctx.db)),
-            hir::ModuleDef::Enum(it) => (CompletionItemKind::Enum, it.docs(ctx.db)),
-            hir::ModuleDef::EnumVariant(it) => (CompletionItemKind::EnumVariant, it.docs(ctx.db)),
-            hir::ModuleDef::Const(it) => (CompletionItemKind::Const, it.docs(ctx.db)),
-            hir::ModuleDef::Static(it) => (CompletionItemKind::Static, it.docs(ctx.db)),
-            hir::ModuleDef::Trait(it) => (CompletionItemKind::Trait, it.docs(ctx.db)),
-            hir::ModuleDef::Type(it) => (CompletionItemKind::TypeAlias, it.docs(ctx.db)),
+            Resolution::Def(Module(it)) => (CompletionItemKind::Module, it.docs(ctx.db)),
+            Resolution::Def(Function(func)) => return self.from_function(ctx, *func),
+            Resolution::Def(Struct(it)) => (CompletionItemKind::Struct, it.docs(ctx.db)),
+            Resolution::Def(Enum(it)) => (CompletionItemKind::Enum, it.docs(ctx.db)),
+            Resolution::Def(EnumVariant(it)) => (CompletionItemKind::EnumVariant, it.docs(ctx.db)),
+            Resolution::Def(Const(it)) => (CompletionItemKind::Const, it.docs(ctx.db)),
+            Resolution::Def(Static(it)) => (CompletionItemKind::Static, it.docs(ctx.db)),
+            Resolution::Def(Trait(it)) => (CompletionItemKind::Trait, it.docs(ctx.db)),
+            Resolution::Def(Type(it)) => (CompletionItemKind::TypeAlias, it.docs(ctx.db)),
+            Resolution::GenericParam(..) => (CompletionItemKind::TypeParam, None),
+            Resolution::LocalBinding(..) => (CompletionItemKind::Binding, None),
+            Resolution::SelfType(..) => (
+                CompletionItemKind::TypeParam, // (does this need its own kind?)
+                None,
+            ),
         };
         self.kind = Some(kind);
         self.documentation = docs;
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__generic_params.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__generic_params.snap
new file mode 100644
index 000000000..71cb55a5b
--- /dev/null
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__generic_params.snap
@@ -0,0 +1,40 @@
+---
+created: "2019-02-01T22:20:40.580128393+00:00"
+creator: [email protected]
+expression: kind_completions
+source: crates/ra_ide_api/src/completion/completion_item.rs
+---
+[
+    CompletionItem {
+        completion_kind: Reference,
+        label: "T",
+        kind: Some(
+            TypeParam
+        ),
+        detail: None,
+        documentation: None,
+        lookup: None,
+        insert_text: None,
+        insert_text_format: PlainText,
+        source_range: [44; 44),
+        text_edit: None
+    },
+    CompletionItem {
+        completion_kind: Reference,
+        label: "quux",
+        kind: Some(
+            Function
+        ),
+        detail: Some(
+            "fn quux<T>()"
+        ),
+        documentation: None,
+        lookup: None,
+        insert_text: Some(
+            "quux()$0"
+        ),
+        insert_text_format: Snippet,
+        source_range: [44; 44),
+        text_edit: None
+    }
+]
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__generic_params_in_struct.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__generic_params_in_struct.snap
new file mode 100644
index 000000000..a35c0cd13
--- /dev/null
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__generic_params_in_struct.snap
@@ -0,0 +1,36 @@
+---
+created: "2019-02-01T22:23:21.508620224+00:00"
+creator: [email protected]
+expression: kind_completions
+source: crates/ra_ide_api/src/completion/completion_item.rs
+---
+[
+    CompletionItem {
+        completion_kind: Reference,
+        label: "T",
+        kind: Some(
+            TypeParam
+        ),
+        detail: None,
+        documentation: None,
+        lookup: None,
+        insert_text: None,
+        insert_text_format: PlainText,
+        source_range: [46; 46),
+        text_edit: None
+    },
+    CompletionItem {
+        completion_kind: Reference,
+        label: "X",
+        kind: Some(
+            Struct
+        ),
+        detail: None,
+        documentation: None,
+        lookup: None,
+        insert_text: None,
+        insert_text_format: PlainText,
+        source_range: [46; 46),
+        text_edit: None
+    }
+]
diff --git a/crates/ra_ide_api/src/completion/snapshots/completion_item__self_in_methods.snap b/crates/ra_ide_api/src/completion/snapshots/completion_item__self_in_methods.snap
index 6a49e325c..ba1d4abbd 100644
--- a/crates/ra_ide_api/src/completion/snapshots/completion_item__self_in_methods.snap
+++ b/crates/ra_ide_api/src/completion/snapshots/completion_item__self_in_methods.snap
@@ -1,12 +1,26 @@
 ---
-created: "2019-01-23T05:27:32.422259+00:00"
-creator: insta@0.4.0
+created: "2019-01-27T20:17:10.051725945+00:00"
+creator: insta@0.5.2
 expression: kind_completions
 source: crates/ra_ide_api/src/completion/completion_item.rs
 ---
 [
     CompletionItem {
         completion_kind: Reference,
+        label: "Self",
+        kind: Some(
+            TypeParam
+        ),
+        detail: None,
+        documentation: None,
+        lookup: None,
+        insert_text: None,
+        insert_text_format: PlainText,
+        source_range: [25; 25),
+        text_edit: None
+    },
+    CompletionItem {
+        completion_kind: Reference,
         label: "self",
         kind: Some(
             Binding
diff --git a/crates/ra_ide_api/src/goto_definition.rs b/crates/ra_ide_api/src/goto_definition.rs
index 2a20c20ee..88efcea2a 100644
--- a/crates/ra_ide_api/src/goto_definition.rs
+++ b/crates/ra_ide_api/src/goto_definition.rs
@@ -4,6 +4,7 @@ use ra_syntax::{
     algo::find_node_at_offset,
 };
 use test_utils::tested_by;
+use hir::Resolution;
 
 use crate::{FilePosition, NavigationTarget, db::RootDatabase, RangeInfo};
 
@@ -48,14 +49,7 @@ pub(crate) fn reference_definition(
     if let Some(function) =
         hir::source_binder::function_from_child_node(db, file_id, name_ref.syntax())
     {
-        let scope = function.scopes(db);
-        // First try to resolve the symbol locally
-        if let Some(entry) = scope.resolve_local_name(name_ref) {
-            let nav = NavigationTarget::from_scope_entry(file_id, &entry);
-            return Exact(nav);
-        };
-
-        // Next check if it is a method
+        // Check if it is a method
         if let Some(method_call) = name_ref
             .syntax()
             .parent()
@@ -86,19 +80,37 @@ pub(crate) fn reference_definition(
             };
         }
     }
-    // Then try module name resolution
-    if let Some(module) = hir::source_binder::module_from_child_node(db, file_id, name_ref.syntax())
+    // Try name resolution
+    let resolver = hir::source_binder::resolver_for_node(db, file_id, name_ref.syntax());
+    if let Some(path) = name_ref
+        .syntax()
+        .ancestors()
+        .find_map(ast::Path::cast)
+        .and_then(hir::Path::from_ast)
     {
-        if let Some(path) = name_ref
-            .syntax()
-            .ancestors()
-            .find_map(ast::Path::cast)
-            .and_then(hir::Path::from_ast)
-        {
-            let resolved = module.resolve_path(db, &path);
-            if let Some(def_id) = resolved.take_types().or(resolved.take_values()) {
-                return Exact(NavigationTarget::from_def(db, def_id));
+        let resolved = resolver.resolve_path(db, &path);
+        match resolved.clone().take_types().or(resolved.take_values()) {
+            Some(Resolution::Def(def)) => return Exact(NavigationTarget::from_def(db, def)),
+            Some(Resolution::LocalBinding(pat)) => {
+                let body = resolver.body().expect("no body for local binding");
+                let syntax_mapping = body.syntax_mapping(db);
+                let ptr = syntax_mapping
+                    .pat_syntax(pat)
+                    .expect("pattern not found in syntax mapping");
+                let name = path
+                    .as_ident()
+                    .cloned()
+                    .expect("local binding from a multi-segment path");
+                let nav = NavigationTarget::from_scope_entry(file_id, name, ptr);
+                return Exact(nav);
+            }
+            Some(Resolution::GenericParam(..)) => {
+                // TODO go to the generic param def
+            }
+            Some(Resolution::SelfType(_impl_block)) => {
+                // TODO go to the implemented type
             }
+            None => {}
         }
     }
     // If that fails try the index based approach.
diff --git a/crates/ra_ide_api/src/navigation_target.rs b/crates/ra_ide_api/src/navigation_target.rs
index 5ccb5cc2e..617908aed 100644
--- a/crates/ra_ide_api/src/navigation_target.rs
+++ b/crates/ra_ide_api/src/navigation_target.rs
@@ -1,9 +1,9 @@
 use ra_db::FileId;
 use ra_syntax::{
-    SyntaxNode, AstNode, SmolStr, TextRange, ast,
+    SyntaxNode, SyntaxNodePtr, AstNode, SmolStr, TextRange, ast,
     SyntaxKind::{self, NAME},
 };
-use hir::{ModuleSource, FieldSource};
+use hir::{ModuleSource, FieldSource, Name};
 
 use crate::{FileSymbol, db::RootDatabase};
 
@@ -58,12 +58,13 @@ impl NavigationTarget {
 
     pub(crate) fn from_scope_entry(
         file_id: FileId,
-        entry: &hir::ScopeEntryWithSyntax,
+        name: Name,
+        ptr: SyntaxNodePtr,
     ) -> NavigationTarget {
         NavigationTarget {
             file_id,
-            name: entry.name().to_string().into(),
-            full_range: entry.ptr().range(),
+            name: name.to_string().into(),
+            full_range: ptr.range(),
             focus_range: None,
             kind: NAME,
         }