Repository: rust-analyzer/rust-analyzer
Pull request #1277 (2019-05-15 04:07:01)

Improve macro item resolution
~This PR add a new namespace `Macros` in `per_ns.rs` to allow following use case:~
This PR improve macro item resolution to allow following use case:

```rust
//- /main.rs     
use foo::bar;
bar!();

//- /lib.rs  (crate foo)
#[macro_export]
macro_rules! bar{
() => {
    struct Foo { field: u32 } 
}
```


matklad: I am not sure if macros should go into `PerNs` long term: 

* I wish we could install a strict separation between pre-expansion and post-expansion phases
* `macro_rules` for local crate doesn't really work like an item: you can have two macros with a same in a module, and the second macro would shadow the first one.

However, I don't know how the long-term solution should look like, and I won't be able to dig into that this week (there's a ton of work related to wrapping up my Rust course), so I am totally fine with landing this approach!

Although let me jot down a possible alternative!

The difference between type/value ns and macro ns is that macro ns is distinguished purely syntactically. That is, we know that we should look at the macro if the path ends with `!`. This is in contrast to types/values, where the same syntactic form can refer to either ns. 

So, what if instead of introduce a third variant to per-ns, we extend resolver interface to roughtly this:

```
fn resolve(&self, path: hir::Path) -> Option<ModuleDef>;
fn resolve_with_macros(&self, path: hir::Path) -> Option<ModuleDefOrMacro>
```

So, only callers that explicitly opt-into caring about macros would get macro resolution?
flodiebold: I've actually thought that having one method per namespace would be a better interface for `Resolver`... I think in most cases, we have one namespace in which we're actually looking for results.
DJMcNab: Don't macro re-exports just use the normal syntax (I.e with no `!`)?

I may be misunderstanding the discussion of course. 
edwin0cheng: @DJMcNab I think they means if that macro is resolved, we know it is macro which must be following a `!`, If it is not resolved, we don't need to know whether it is types or values or macro.
edwin0cheng: @matklad I removed the Macro namespace and add a new enum `ModuleDefOrMacroNs` to replace the `PerNs<ModuleDef>` :

```rust
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum ModuleDefOrMacroNs {
    ModuleDefNs(PerNs<ModuleDef>),
    MacroNs(MacroDef),
}

#[derive(Debug, Clone, PartialEq, Eq, Default)]
pub struct Resolution {
    /// None for unresolved
    pub def: ModuleDefOrMacroNs,
    /// ident by which this is imported into local scope.
    pub import: Option<ImportId>,
}
```

And it allow as handle the import-resolution for macro and without adding a new namespace. 
And note that this PR do not try to change the name-resolution part. It only affect import-resolution.



edwin0cheng: It is already to review now 👍 

Patch:
diff --git a/Cargo.lock b/Cargo.lock
index a61aec4e12..79e6329076 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1117,6 +1117,7 @@ dependencies = [
  "chalk-ir 0.1.0 (git+https://github.com/flodiebold/chalk.git?branch=fuel)",
  "chalk-rust-ir 0.1.0 (git+https://github.com/flodiebold/chalk.git?branch=fuel)",
  "chalk-solve 0.1.0 (git+https://github.com/flodiebold/chalk.git?branch=fuel)",
+ "either 1.5.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ena 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "flexi_logger 0.11.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "insta 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/crates/ra_hir/Cargo.toml b/crates/ra_hir/Cargo.toml
index 294d047d89..18ecb79575 100644
--- a/crates/ra_hir/Cargo.toml
+++ b/crates/ra_hir/Cargo.toml
@@ -12,6 +12,7 @@ rustc-hash = "1.0"
 parking_lot = "0.7.0"
 ena = "0.11"
 join_to_string = "0.1.3"
+either = "1.5.2"
 
 ra_syntax = { path = "../ra_syntax" }
 ra_arena = { path = "../ra_arena" }
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index 49030ce676..a9ffc87822 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -13,7 +13,7 @@ use crate::{
     adt::{EnumVariantId, StructFieldId, VariantDef},
     generics::HasGenericParams,
     docs::{Documentation, Docs, docs_from_ast},
-    ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeAliasId},
+    ids::{FunctionId, StructId, EnumId, AstItemDef, ConstId, StaticId, TraitId, TypeAliasId, MacroDefId},
     impl_block::ImplBlock,
     resolve::Resolver,
     diagnostics::{DiagnosticSink},
@@ -937,6 +937,10 @@ impl Docs for TypeAlias {
         docs_from_ast(&*self.source(db).1)
     }
 }
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct MacroDef {
+    pub(crate) id: MacroDefId,
+}
 
 pub enum Container {
     Trait(Trait),
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index fe2d4adeee..cb09c60f8a 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -80,5 +80,5 @@ pub use self::code_model::{
     Function, FnSignature,
     StructField, FieldSource,
     Static, Const, ConstSignature,
-    Trait, TypeAlias, Container
+    Trait, TypeAlias, MacroDef, Container
 };
diff --git a/crates/ra_hir/src/nameres.rs b/crates/ra_hir/src/nameres.rs
index 0290b3474f..9b9212bfcb 100644
--- a/crates/ra_hir/src/nameres.rs
+++ b/crates/ra_hir/src/nameres.rs
@@ -56,6 +56,7 @@ mod tests;
 use std::sync::Arc;
 
 use rustc_hash::{FxHashMap, FxHashSet};
+use either::Either;
 use ra_arena::{Arena, RawId, impl_arena_id};
 use ra_db::{FileId, Edition};
 use test_utils::tested_by;
@@ -63,7 +64,7 @@ use ra_syntax::ast;
 use ra_prof::profile;
 
 use crate::{
-    ModuleDef, Name, Crate, Module,
+    ModuleDef, Name, Crate, Module, MacroDef,
     DefDatabase, Path, PathKind, HirFileId, Trait,
     ids::MacroDefId,
     diagnostics::DiagnosticSink,
@@ -136,6 +137,7 @@ pub(crate) struct ModuleData {
 #[derive(Debug, Default, PartialEq, Eq, Clone)]
 pub struct ModuleScope {
     items: FxHashMap<Name, Resolution>,
+    macros: FxHashMap<Name, MacroDef>,
 }
 
 impl ModuleScope {
@@ -151,8 +153,17 @@ impl ModuleScope {
             _ => None,
         })
     }
+    fn get_item_or_macro(&self, name: &Name) -> Option<ItemOrMacro> {
+        match (self.items.get(name), self.macros.get(name)) {
+            (Some(item), _) if !item.def.is_none() => Some(Either::Left(item.def)),
+            (_, Some(macro_)) => Some(Either::Right(*macro_)),
+            _ => None,
+        }
+    }
 }
 
+type ItemOrMacro = Either<PerNs<ModuleDef>, MacroDef>;
+
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
 pub struct Resolution {
     /// None for unresolved
@@ -163,18 +174,18 @@ pub struct Resolution {
 
 #[derive(Debug, Clone)]
 struct ResolvePathResult {
-    resolved_def: PerNs<ModuleDef>,
+    resolved_def: ItemOrMacro,
     segment_index: Option<usize>,
     reached_fixedpoint: ReachedFixedPoint,
 }
 
 impl ResolvePathResult {
     fn empty(reached_fixedpoint: ReachedFixedPoint) -> ResolvePathResult {
-        ResolvePathResult::with(PerNs::none(), reached_fixedpoint, None)
+        ResolvePathResult::with(Either::Left(PerNs::none()), reached_fixedpoint, None)
     }
 
     fn with(
-        resolved_def: PerNs<ModuleDef>,
+        resolved_def: ItemOrMacro,
         reached_fixedpoint: ReachedFixedPoint,
         segment_index: Option<usize>,
     ) -> ResolvePathResult {
@@ -194,6 +205,21 @@ enum ReachedFixedPoint {
     No,
 }
 
+/// helper function for select item or macro to use
+fn or(left: ItemOrMacro, right: ItemOrMacro) -> ItemOrMacro {
+    match (left, right) {
+        (Either::Left(s), Either::Left(o)) => Either::Left(s.or(o)),
+        (Either::Right(s), _) => Either::Right(s),
+        (Either::Left(s), Either::Right(o)) => {
+            if !s.is_none() {
+                Either::Left(s)
+            } else {
+                Either::Right(o)
+            }
+        }
+    }
+}
+
 impl CrateDefMap {
     pub(crate) fn crate_def_map_query(db: &impl DefDatabase, krate: Crate) -> Arc<CrateDefMap> {
         let _p = profile("crate_def_map_query");
@@ -268,7 +294,17 @@ impl CrateDefMap {
         original_module: CrateModuleId,
         path: &Path,
     ) -> (PerNs<ModuleDef>, Option<usize>) {
-        let res = self.resolve_path_fp(db, ResolveMode::Other, original_module, path);
+        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
+        (res.resolved_def.left().unwrap_or_else(PerNs::none), res.segment_index)
+    }
+
+    fn resolve_path_with_macro(
+        &self,
+        db: &impl DefDatabase,
+        original_module: CrateModuleId,
+        path: &Path,
+    ) -> (ItemOrMacro, Option<usize>) {
+        let res = self.resolve_path_fp_with_macro(db, ResolveMode::Other, original_module, path);
         (res.resolved_def, res.segment_index)
     }
 
@@ -278,7 +314,7 @@ impl CrateDefMap {
 
     // Returns Yes if we are sure that additions to `ItemMap` wouldn't change
     // the result.
-    fn resolve_path_fp(
+    fn resolve_path_fp_with_macro(
         &self,
         db: &impl DefDatabase,
         mode: ResolveMode,
@@ -286,13 +322,13 @@ impl CrateDefMap {
         path: &Path,
     ) -> ResolvePathResult {
         let mut segments = path.segments.iter().enumerate();
-        let mut curr_per_ns: PerNs<ModuleDef> = match path.kind {
-            PathKind::Crate => {
-                PerNs::types(Module { krate: self.krate, module_id: self.root }.into())
-            }
-            PathKind::Self_ => {
-                PerNs::types(Module { krate: self.krate, module_id: original_module }.into())
-            }
+        let mut curr_per_ns: ItemOrMacro = match path.kind {
+            PathKind::Crate => Either::Left(PerNs::types(
+                Module { krate: self.krate, module_id: self.root }.into(),
+            )),
+            PathKind::Self_ => Either::Left(PerNs::types(
+                Module { krate: self.krate, module_id: original_module }.into(),
+            )),
             // plain import or absolute path in 2015: crate-relative with
             // fallback to extern prelude (with the simplification in
             // rust-lang/rust#57745)
@@ -314,11 +350,11 @@ impl CrateDefMap {
                     None => return ResolvePathResult::empty(ReachedFixedPoint::Yes),
                 };
                 log::debug!("resolving {:?} in module", segment);
-                self.resolve_name_in_module(db, original_module, &segment.name)
+                self.resolve_name_in_module_with_macro(db, original_module, &segment.name)
             }
             PathKind::Super => {
                 if let Some(p) = self.modules[original_module].parent {
-                    PerNs::types(Module { krate: self.krate, module_id: p }.into())
+                    Either::Left(PerNs::types(Module { krate: self.krate, module_id: p }.into()))
                 } else {
                     log::debug!("super path in root module");
                     return ResolvePathResult::empty(ReachedFixedPoint::Yes);
@@ -332,7 +368,7 @@ impl CrateDefMap {
                 };
                 if let Some(def) = self.extern_prelude.get(&segment.name) {
                     log::debug!("absolute path {:?} resolved to crate {:?}", path, def);
-                    PerNs::types(*def)
+                    Either::Left(PerNs::types(*def))
                 } else {
                     return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude
                 }
@@ -340,7 +376,7 @@ impl CrateDefMap {
         };
 
         for (i, segment) in segments {
-            let curr = match curr_per_ns.as_ref().take_types() {
+            let curr = match curr_per_ns.as_ref().left().map_or(None, |m| m.as_ref().take_types()) {
                 Some(r) => r,
                 None => {
                     // we still have path segments left, but the path so far
@@ -362,7 +398,8 @@ impl CrateDefMap {
                         };
                         log::debug!("resolving {:?} in other crate", path);
                         let defp_map = db.crate_def_map(module.krate);
-                        let (def, s) = defp_map.resolve_path(db, module.module_id, &path);
+                        let (def, s) =
+                            defp_map.resolve_path_with_macro(db, module.module_id, &path);
                         return ResolvePathResult::with(
                             def,
                             ReachedFixedPoint::Yes,
@@ -370,8 +407,8 @@ impl CrateDefMap {
                         );
                     }
 
-                    match self[module.module_id].scope.items.get(&segment.name) {
-                        Some(res) if !res.def.is_none() => res.def,
+                    match self[module.module_id].scope.get_item_or_macro(&segment.name) {
+                        Some(res) => res,
                         _ => {
                             log::debug!("path segment {:?} not found", segment.name);
                             return ResolvePathResult::empty(ReachedFixedPoint::No);
@@ -382,10 +419,10 @@ impl CrateDefMap {
                     // enum variant
                     tested_by!(can_import_enum_variant);
                     match e.variant(db, &segment.name) {
-                        Some(variant) => PerNs::both(variant.into(), variant.into()),
+                        Some(variant) => Either::Left(PerNs::both(variant.into(), variant.into())),
                         None => {
                             return ResolvePathResult::with(
-                                PerNs::types((*e).into()),
+                                Either::Left(PerNs::types((*e).into())),
                                 ReachedFixedPoint::Yes,
                                 Some(i),
                             );
@@ -402,7 +439,7 @@ impl CrateDefMap {
                     );
 
                     return ResolvePathResult::with(
-                        PerNs::types((*s).into()),
+                        Either::Left(PerNs::types((*s).into())),
                         ReachedFixedPoint::Yes,
                         Some(i),
                     );
@@ -412,12 +449,12 @@ impl CrateDefMap {
         ResolvePathResult::with(curr_per_ns, ReachedFixedPoint::Yes, None)
     }
 
-    fn resolve_name_in_crate_root_or_extern_prelude(&self, name: &Name) -> PerNs<ModuleDef> {
+    fn resolve_name_in_crate_root_or_extern_prelude(&self, name: &Name) -> ItemOrMacro {
         let from_crate_root =
-            self[self.root].scope.items.get(name).map_or(PerNs::none(), |it| it.def);
+            self[self.root].scope.get_item_or_macro(name).unwrap_or(Either::Left(PerNs::none()));
         let from_extern_prelude = self.resolve_name_in_extern_prelude(name);
 
-        from_crate_root.or(from_extern_prelude)
+        or(from_crate_root, Either::Left(from_extern_prelude))
     }
 
     pub(crate) fn resolve_name_in_module(
@@ -426,32 +463,42 @@ impl CrateDefMap {
         module: CrateModuleId,
         name: &Name,
     ) -> PerNs<ModuleDef> {
+        self.resolve_name_in_module_with_macro(db, module, name).left().unwrap_or_else(PerNs::none)
+    }
+
+    fn resolve_name_in_module_with_macro(
+        &self,
+        db: &impl DefDatabase,
+        module: CrateModuleId,
+        name: &Name,
+    ) -> ItemOrMacro {
         // Resolve in:
         //  - current module / scope
         //  - extern prelude
         //  - std prelude
-        let from_scope = self[module].scope.items.get(name).map_or(PerNs::none(), |it| it.def);
+        let from_scope =
+            self[module].scope.get_item_or_macro(name).unwrap_or(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);
 
-        from_scope.or(from_extern_prelude).or(from_prelude)
+        or(from_scope, or(Either::Left(from_extern_prelude), from_prelude))
     }
 
     fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs<ModuleDef> {
         self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it))
     }
 
-    fn resolve_in_prelude(&self, db: &impl DefDatabase, name: &Name) -> PerNs<ModuleDef> {
+    fn resolve_in_prelude(&self, db: &impl DefDatabase, name: &Name) -> ItemOrMacro {
         if let Some(prelude) = self.prelude {
             let resolution = if prelude.krate == self.krate {
-                self[prelude.module_id].scope.items.get(name).cloned()
+                self[prelude.module_id].scope.get_item_or_macro(name)
             } else {
-                db.crate_def_map(prelude.krate)[prelude.module_id].scope.items.get(name).cloned()
+                db.crate_def_map(prelude.krate)[prelude.module_id].scope.get_item_or_macro(name)
             };
-            resolution.map(|r| r.def).unwrap_or_else(PerNs::none)
+            resolution.unwrap_or(Either::Left(PerNs::none()))
         } else {
-            PerNs::none()
+            Either::Left(PerNs::none())
         }
     }
 }
@@ -463,7 +510,7 @@ mod diagnostics {
     use crate::{
         AstId, DefDatabase,
         nameres::CrateModuleId,
-        diagnostics::{DiagnosticSink, UnresolvedModule},
+        diagnostics::{DiagnosticSink, UnresolvedModule}
 };
 
     #[derive(Debug, PartialEq, Eq)]
diff --git a/crates/ra_hir/src/nameres/collector.rs b/crates/ra_hir/src/nameres/collector.rs
index 621236551e..ba7ea00174 100644
--- a/crates/ra_hir/src/nameres/collector.rs
+++ b/crates/ra_hir/src/nameres/collector.rs
@@ -1,17 +1,18 @@
 use arrayvec::ArrayVec;
 use rustc_hash::FxHashMap;
+use either::Either;
 use relative_path::RelativePathBuf;
 use test_utils::tested_by;
 use ra_db::FileId;
 use ra_syntax::ast;
 
 use crate::{
-    Function, Module, Struct, Union, Enum, Const, Static, Trait, TypeAlias,
+    Function, Module, Struct, Union, Enum, Const, Static, Trait, TypeAlias, MacroDef,
     DefDatabase, HirFileId, Name, Path,
     KnownName,
     nameres::{
         Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode,
-        CrateDefMap, CrateModuleId, ModuleData,
+        CrateDefMap, CrateModuleId, ModuleData, ItemOrMacro,
         diagnostics::DefDiagnostic,
         raw,
     },
@@ -124,13 +125,21 @@ where
         let unresolved_imports = std::mem::replace(&mut self.unresolved_imports, Vec::new());
         // show unresolved imports in completion, etc
         for (module_id, import, import_data) in unresolved_imports {
-            self.record_resolved_import(module_id, PerNs::none(), import, &import_data)
+            self.record_resolved_import(
+                module_id,
+                Either::Left(PerNs::none()),
+                import,
+                &import_data,
+            )
         }
     }
 
     fn define_macro(&mut self, name: Name, macro_id: MacroDefId, export: bool) {
         if export {
             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);
         }
@@ -161,7 +170,7 @@ where
         &self,
         module_id: CrateModuleId,
         import: &raw::ImportData,
-    ) -> (PerNs<ModuleDef>, ReachedFixedPoint) {
+    ) -> (ItemOrMacro, ReachedFixedPoint) {
         log::debug!("resolving import: {:?} ({:?})", import, self.def_map.edition);
         if import.is_extern_crate {
             let res = self.def_map.resolve_name_in_extern_prelude(
@@ -170,10 +179,14 @@ where
                     .as_ident()
                     .expect("extern crate should have been desugared to one-element path"),
             );
-            (res, ReachedFixedPoint::Yes)
+            (Either::Left(res), ReachedFixedPoint::Yes)
         } else {
-            let res =
-                self.def_map.resolve_path_fp(self.db, ResolveMode::Import, module_id, &import.path);
+            let res = self.def_map.resolve_path_fp_with_macro(
+                self.db,
+                ResolveMode::Import,
+                module_id,
+                &import.path,
+            );
 
             (res.resolved_def, res.reached_fixedpoint)
         }
@@ -182,13 +195,13 @@ where
     fn record_resolved_import(
         &mut self,
         module_id: CrateModuleId,
-        def: PerNs<ModuleDef>,
+        def: ItemOrMacro,
         import_id: raw::ImportId,
         import: &raw::ImportData,
     ) {
         if import.is_glob {
             log::debug!("glob import: {:?}", import);
-            match def.take_types() {
+            match def.left().and_then(|item| item.take_types()) {
                 Some(ModuleDef::Module(m)) => {
                     if import.is_prelude {
                         tested_by!(std_prelude);
@@ -201,9 +214,14 @@ where
                         let items = scope
                             .items
                             .iter()
-                            .map(|(name, res)| (name.clone(), res.clone()))
-                            .collect::<Vec<_>>();
-                        self.update(module_id, Some(import_id), &items);
+                            .map(|(name, res)| (name.clone(), Either::Left(res.clone())));
+                        let macros = scope
+                            .macros
+                            .iter()
+                            .map(|(name, res)| (name.clone(), Either::Right(res.clone())));
+
+                        let all = items.chain(macros).collect::<Vec<_>>();
+                        self.update(module_id, Some(import_id), &all);
                     } else {
                         // glob import from same crate => we do an initial
                         // import, and then need to propagate any further
@@ -212,9 +230,15 @@ where
                         let items = scope
                             .items
                             .iter()
-                            .map(|(name, res)| (name.clone(), res.clone()))
-                            .collect::<Vec<_>>();
-                        self.update(module_id, Some(import_id), &items);
+                            .map(|(name, res)| (name.clone(), Either::Left(res.clone())));
+                        let macros = scope
+                            .macros
+                            .iter()
+                            .map(|(name, res)| (name.clone(), Either::Right(res.clone())));
+
+                        let all = items.chain(macros).collect::<Vec<_>>();
+
+                        self.update(module_id, Some(import_id), &all);
                         // record the glob import in case we add further items
                         self.glob_imports
                             .entry(m.module_id)
@@ -234,7 +258,7 @@ where
                                 import: Some(import_id),
                             };
                             let name = variant.name(self.db)?;
-                            Some((name, res))
+                            Some((name, Either::Left(res)))
                         })
                         .collect::<Vec<_>>();
                     self.update(module_id, Some(import_id), &resolutions);
@@ -254,11 +278,18 @@ where
 
                     // extern crates in the crate root are special-cased to insert entries into the extern prelude: rust-lang/rust#54658
                     if import.is_extern_crate && module_id == self.def_map.root {
-                        if let Some(def) = def.take_types() {
+                        if let Some(def) = def.left().and_then(|item| item.take_types()) {
                             self.def_map.extern_prelude.insert(name.clone(), def);
                         }
                     }
-                    let resolution = Resolution { def, import: Some(import_id) };
+
+                    let resolution = match def {
+                        Either::Left(item) => {
+                            Either::Left(Resolution { def: item, import: Some(import_id) })
+                        }
+                        Either::Right(macro_) => Either::Right(macro_),
+                    };
+
                     self.update(module_id, Some(import_id), &[(name, resolution)]);
                 }
                 None => tested_by!(bogus_paths),
@@ -270,7 +301,7 @@ where
         &mut self,
         module_id: CrateModuleId,
         import: Option<raw::ImportId>,
-        resolutions: &[(Name, Resolution)],
+        resolutions: &[(Name, Either<Resolution, MacroDef>)],
     ) {
         self.update_recursive(module_id, import, resolutions, 0)
     }
@@ -279,7 +310,7 @@ where
         &mut self,
         module_id: CrateModuleId,
         import: Option<raw::ImportId>,
-        resolutions: &[(Name, Resolution)],
+        resolutions: &[(Name, Either<Resolution, MacroDef>)],
         depth: usize,
     ) {
         if depth > 100 {
@@ -289,25 +320,38 @@ where
         let module_items = &mut self.def_map.modules[module_id].scope;
         let mut changed = false;
         for (name, res) in resolutions {
-            let existing = module_items.items.entry(name.clone()).or_default();
-            if existing.def.types.is_none() && res.def.types.is_some() {
-                existing.def.types = res.def.types;
-                existing.import = import.or(res.import);
-                changed = true;
-            }
-            if existing.def.values.is_none() && res.def.values.is_some() {
-                existing.def.values = res.def.values;
-                existing.import = import.or(res.import);
-                changed = true;
-            }
-            if existing.def.is_none()
-                && res.def.is_none()
-                && existing.import.is_none()
-                && res.import.is_some()
-            {
-                existing.import = res.import;
+            match res {
+                // item
+                Either::Left(res) => {
+                    let existing = module_items.items.entry(name.clone()).or_default();
+
+                    if existing.def.types.is_none() && res.def.types.is_some() {
+                        existing.def.types = res.def.types;
+                        existing.import = import.or(res.import);
+                        changed = true;
+                    }
+                    if existing.def.values.is_none() && res.def.values.is_some() {
+                        existing.def.values = res.def.values;
+                        existing.import = import.or(res.import);
+                        changed = true;
+                    }
+
+                    if existing.def.is_none()
+                        && res.def.is_none()
+                        && existing.import.is_none()
+                        && res.import.is_some()
+                    {
+                        existing.import = res.import;
+                    }
+                }
+                // macro
+                Either::Right(res) => {
+                    // Always shadowing
+                    module_items.macros.insert(name.clone(), *res);
+                }
             }
         }
+
         if !changed {
             return;
         }
@@ -324,37 +368,64 @@ where
         }
     }
 
-    // XXX: this is just a pile of hacks now, because `PerNs` does not handle
-    // macro namespace.
     fn resolve_macros(&mut self) -> ReachedFixedPoint {
         let mut macros = std::mem::replace(&mut self.unexpanded_macros, Vec::new());
         let mut resolved = Vec::new();
         let mut res = ReachedFixedPoint::Yes;
         macros.retain(|(module_id, ast_id, path)| {
-            if path.segments.len() != 2 {
-                return true;
+            let resolved_res = self.def_map.resolve_path_fp_with_macro(
+                self.db,
+                ResolveMode::Other,
+                *module_id,
+                path,
+            );
+
+            if let Some(def) = resolved_res.resolved_def.right() {
+                let call_id = MacroCallLoc { def: def.id, ast_id: *ast_id }.id(self.db);
+                resolved.push((*module_id, call_id, def.id));
+                res = ReachedFixedPoint::No;
+                return false;
             }
-            let crate_name = &path.segments[0].name;
-            let krate = match self.def_map.resolve_name_in_extern_prelude(crate_name).take_types() {
-                Some(ModuleDef::Module(m)) => m.krate(self.db),
-                _ => return true,
-            };
-            let krate = match krate {
-                Some(it) => it,
-                _ => return true,
-            };
-            res = ReachedFixedPoint::No;
-            let def_map = self.db.crate_def_map(krate);
-            if let Some(macro_id) = def_map.public_macros.get(&path.segments[1].name).cloned() {
-                let call_id = MacroCallLoc { def: macro_id, ast_id: *ast_id }.id(self.db);
-                resolved.push((*module_id, call_id, macro_id));
+
+            if resolved_res.reached_fixedpoint != ReachedFixedPoint::Yes {
+                let crate_name = &path.segments[0].name;
+
+                // FIXME:
+                // $crate are not handled in resolver right now
+                if crate_name.to_string() == "$crate" {
+                    return true;
+                }
+
+                // FIXME:
+                // Currently `#[cfg(test)]` are ignored and cargo-metadata do not insert
+                // dev-dependencies of dependencies. For example,
+                // if we depend on parking lot, and parking lot has a dev-dependency on lazy_static.
+                // Then `lazy_static` wil not included in `CrateGraph`
+                // We can fix that by proper handling `cfg(test)`.
+                //
+                // So right now we set the fixpoint to No only if its crate is in CrateGraph
+                // See issue #1282 for details
+                let krate =
+                    match self.def_map.resolve_name_in_extern_prelude(crate_name).take_types() {
+                        Some(ModuleDef::Module(m)) => m.krate(self.db),
+                        _ => return true,
+                    };
+                if krate.is_none() {
+                    return true;
+                }
+
+                res = resolved_res.reached_fixedpoint;
             }
-            false
+
+            true
         });
 
+        self.unexpanded_macros = macros;
+
         for (module_id, macro_call_id, macro_def_id) in resolved {
             self.collect_macro_expansion(module_id, macro_call_id, macro_def_id);
         }
+
         res
     }
 
@@ -475,7 +546,7 @@ where
             ),
             import: None,
         };
-        self.def_collector.update(self.module_id, None, &[(name, resolution)]);
+        self.def_collector.update(self.module_id, None, &[(name, Either::Left(resolution))]);
         res
     }
 
@@ -506,7 +577,7 @@ where
             raw::DefKind::TypeAlias(ast_id) => PerNs::types(def!(TypeAlias, ast_id)),
         };
         let resolution = Resolution { def, import: None };
-        self.def_collector.update(self.module_id, None, &[(name, resolution)])
+        self.def_collector.update(self.module_id, None, &[(name, Either::Left(resolution))])
     }
 
     fn collect_macro(&mut self, mac: &raw::MacroData) {
diff --git a/docs/user/README.md b/docs/user/README.md
index 47fc840f77..affb969390 100644
--- a/docs/user/README.md
+++ b/docs/user/README.md
@@ -89,25 +89,6 @@ to load path and require it in `init.el`
 * (Optionally) bind commands like `rust-analyzer-join-lines` or `rust-analyzer-extend-selection` to keys
 
 
-## Vim and NeoVim
-
-* Install coc.nvim by following the instructions at [coc.nvim]
-* Add rust analyzer using: [coc.nvim wiki][coc-wiki]
-
-```jsonc
- "languageserver": {
-  "rust": {
-    "command": "ra_lsp_server",
-    "filetypes": ["rust"],
-    "rootPatterns": ["Cargo.toml"]
-  }
-}
-```
-
-[coc.nvim]: https://github.com/neoclide/coc.nvim
-[coc-wiki]: https://github.com/neoclide/coc.nvim/wiki/Language-servers#rust
-
-
 ## Sublime Text 3
 
 Prequisites:


Test patch:
diff --git a/crates/ra_hir/src/nameres/tests.rs b/crates/ra_hir/src/nameres/tests.rs
index 14c8ee50b6..ffb627c02c 100644
--- a/crates/ra_hir/src/nameres/tests.rs
+++ b/crates/ra_hir/src/nameres/tests.rs
@@ -7,6 +7,7 @@ use std::sync::Arc;
 use ra_db::SourceDatabase;
 use test_utils::covers;
 use insta::assert_snapshot_matches;
+use either::Either;
 
 use crate::{
     Crate,
@@ -35,10 +36,22 @@ fn render_crate_def_map(map: &CrateDefMap) -> String {
         *buf += path;
         *buf += "\n";
 
-        let mut entries = map.modules[module].scope.items.iter().collect::<Vec<_>>();
+        let items =
+            map.modules[module].scope.items.iter().map(|(name, it)| (name, Either::Left(it)));
+        let macros =
+            map.modules[module].scope.macros.iter().map(|(name, m)| (name, Either::Right(m)));
+        let mut entries = items.chain(macros).collect::<Vec<_>>();
+
         entries.sort_by_key(|(name, _)| *name);
         for (name, res) in entries {
-            *buf += &format!("{}: {}\n", name, dump_resolution(res))
+            match res {
+                Either::Left(it) => {
+                    *buf += &format!("{}: {}\n", name, dump_resolution(it));
+                }
+                Either::Right(_) => {
+                    *buf += &format!("{}: m\n", name);
+                }
+            }
         }
         for (name, child) in map.modules[module].children.iter() {
             let path = path.to_string() + &format!("::{}", name);
diff --git a/crates/ra_hir/src/nameres/tests/macros.rs b/crates/ra_hir/src/nameres/tests/macros.rs
index f7ca380ad1..42241aeff7 100644
--- a/crates/ra_hir/src/nameres/tests/macros.rs
+++ b/crates/ra_hir/src/nameres/tests/macros.rs
@@ -92,3 +92,43 @@ fn macro_rules_from_other_crates_are_visible() {
    ⋮bar: t
     "###);
 }
+
+#[test]
+fn unexpanded_macro_should_expand_by_fixedpoint_loop() {
+    let map = def_map_with_crate_graph(
+        "
+        //- /main.rs      
+        macro_rules! baz {
+            () => {
+                use foo::bar;                
+            }
+        }
+        
+        foo!();              
+        bar!();
+        baz!();
+
+        //- /lib.rs
+        #[macro_export]
+        macro_rules! foo {
+            () => {
+                struct Foo { field: u32 } 
+            }
+        }
+        #[macro_export]
+        macro_rules! bar {
+            () => {
+                use foo::foo;
+            }
+        }
+        ",
+        crate_graph! {
+            "main": ("/main.rs", ["foo"]),
+            "foo": ("/lib.rs", []),
+        },
+    );
+    assert_snapshot_matches!(map, @r###"crate
+Foo: t v
+bar: m
+foo: m"###);
+}