6 files changed, 264 insertions, 81 deletions

diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index 3ced648e5..a7e2e0982 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -5,7 +5,7 @@ use either::Either;
 use hir_expand::{
     hygiene::Hygiene,
     name::{name, AsName, Name},
-    AstId, HirFileId, MacroDefId, MacroDefKind,
+    HirFileId, MacroDefId, MacroDefKind,
 };
 use ra_arena::Arena;
 use ra_syntax::{
@@ -27,7 +27,7 @@ use crate::{
         LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
     },
     item_scope::BuiltinShadowMode,
-    item_tree::{FileItemTreeId, ItemTree, ItemTreeNode},
+    item_tree::{ItemTree, ItemTreeId, ItemTreeNode},
     path::{GenericArgs, Path},
     type_ref::{Mutability, Rawness, TypeRef},
     AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId,
@@ -37,7 +37,7 @@ use crate::{
 use super::{ExprSource, PatSource};
 use ast::AstChildren;
 use rustc_hash::FxHashMap;
-use std::sync::Arc;
+use std::{any::type_name, sync::Arc};
 
 pub(crate) struct LowerCtx {
     hygiene: Hygiene,
@@ -561,17 +561,30 @@ impl ExprCollector<'_> {
         }
     }
 
-    fn find_inner_item<S: ItemTreeNode>(&self, id: AstId<ast::ModuleItem>) -> FileItemTreeId<S> {
+    fn find_inner_item<N: ItemTreeNode>(&self, ast: &N::Source) -> Option<ItemTreeId<N>> {
+        let id = self.expander.ast_id(ast);
         let tree = &self.item_trees[&id.file_id];
 
         // FIXME: This probably breaks with `use` items, since they produce multiple item tree nodes
 
         // Root file (non-macro).
-        tree.all_inner_items()
+        let item_tree_id = tree
+            .all_inner_items()
             .chain(tree.top_level_items().iter().copied())
-            .filter_map(|mod_item| mod_item.downcast::<S>())
-            .find(|tree_id| tree[*tree_id].ast_id().upcast() == id.value)
-            .unwrap_or_else(|| panic!("couldn't find inner item for {:?}", id))
+            .filter_map(|mod_item| mod_item.downcast::<N>())
+            .find(|tree_id| tree[*tree_id].ast_id().upcast() == id.value.upcast())
+            .or_else(|| {
+                log::debug!(
+                    "couldn't find inner {} item for {:?} (AST: `{}` - {:?})",
+                    type_name::<N>(),
+                    id,
+                    ast.syntax(),
+                    ast.syntax(),
+                );
+                None
+            })?;
+
+        Some(ItemTreeId::new(id.file_id, item_tree_id))
     }
 
     fn collect_expr_opt(&mut self, expr: Option<ast::Expr>) -> ExprId {
@@ -611,82 +624,45 @@ impl ExprCollector<'_> {
             .filter_map(|item| {
                 let (def, name): (ModuleDefId, Option<ast::Name>) = match item {
                     ast::ModuleItem::FnDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
+                        let id = self.find_inner_item(&def)?;
                         (
-                            FunctionLoc { container: container.into(), id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
+                            FunctionLoc { container: container.into(), id }.intern(self.db).into(),
                             def.name(),
                         )
                     }
                     ast::ModuleItem::TypeAliasDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
+                        let id = self.find_inner_item(&def)?;
                         (
-                            TypeAliasLoc { container: container.into(), id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
+                            TypeAliasLoc { container: container.into(), id }.intern(self.db).into(),
                             def.name(),
                         )
                     }
                     ast::ModuleItem::ConstDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
+                        let id = self.find_inner_item(&def)?;
                         (
-                            ConstLoc { container: container.into(), id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
+                            ConstLoc { container: container.into(), id }.intern(self.db).into(),
                             def.name(),
                         )
                     }
                     ast::ModuleItem::StaticDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
-                        (
-                            StaticLoc { container, id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
-                            def.name(),
-                        )
+                        let id = self.find_inner_item(&def)?;
+                        (StaticLoc { container, id }.intern(self.db).into(), def.name())
                     }
                     ast::ModuleItem::StructDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
-                        (
-                            StructLoc { container, id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
-                            def.name(),
-                        )
+                        let id = self.find_inner_item(&def)?;
+                        (StructLoc { container, id }.intern(self.db).into(), def.name())
                     }
                     ast::ModuleItem::EnumDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
-                        (
-                            EnumLoc { container, id: ast_id.with_value(id) }.intern(self.db).into(),
-                            def.name(),
-                        )
+                        let id = self.find_inner_item(&def)?;
+                        (EnumLoc { container, id }.intern(self.db).into(), def.name())
                     }
                     ast::ModuleItem::UnionDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
-                        (
-                            UnionLoc { container, id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
-                            def.name(),
-                        )
+                        let id = self.find_inner_item(&def)?;
+                        (UnionLoc { container, id }.intern(self.db).into(), def.name())
                     }
                     ast::ModuleItem::TraitDef(def) => {
-                        let ast_id = self.expander.ast_id(&def);
-                        let id = self.find_inner_item(ast_id.map(|id| id.upcast()));
-                        (
-                            TraitLoc { container, id: ast_id.with_value(id) }
-                                .intern(self.db)
-                                .into(),
-                            def.name(),
-                        )
+                        let id = self.find_inner_item(&def)?;
+                        (TraitLoc { container, id }.intern(self.db).into(), def.name())
                     }
                     ast::ModuleItem::ExternBlock(_) => return None, // FIXME: collect from extern blocks
                     ast::ModuleItem::ImplDef(_)
diff --git a/crates/ra_hir_def/src/body/scope.rs b/crates/ra_hir_def/src/body/scope.rs
index 81397b063..99e876683 100644
--- a/crates/ra_hir_def/src/body/scope.rs
+++ b/crates/ra_hir_def/src/body/scope.rs
@@ -337,6 +337,19 @@ fn foo() {
         );
     }
 
+    #[test]
+    fn broken_inner_item() {
+        do_check(
+            r"
+            fn foo() {
+                trait {}
+                <|>
+            }
+        ",
+            &[],
+        );
+    }
+
     fn do_check_local_name(ra_fixture: &str, expected_offset: u32) {
         let (db, position) = TestDB::with_position(ra_fixture);
         let file_id = position.file_id;
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index c81b966c3..4d446c707 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -4,14 +4,27 @@
 use hir_expand::name::Name;
 use once_cell::sync::Lazy;
 use ra_db::CrateId;
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 use test_utils::mark;
 
 use crate::{
     db::DefDatabase, per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, HasModule, ImplId,
-    Lookup, MacroDefId, ModuleDefId, TraitId,
+    LocalModuleId, Lookup, MacroDefId, ModuleDefId, TraitId,
 };
 
+#[derive(Copy, Clone)]
+pub(crate) enum ImportType {
+    Glob,
+    Named,
+}
+
+#[derive(Debug, Default)]
+pub struct PerNsGlobImports {
+    types: FxHashSet<(LocalModuleId, Name)>,
+    values: FxHashSet<(LocalModuleId, Name)>,
+    macros: FxHashSet<(LocalModuleId, Name)>,
+}
+
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct ItemScope {
     visible: FxHashMap<Name, PerNs>,
@@ -127,16 +140,62 @@ impl ItemScope {
         let mut changed = false;
         let existing = self.visible.entry(name).or_default();
 
+        if existing.types.is_none() && def.types.is_some() {
+            existing.types = def.types;
+            changed = true;
+        }
+
+        if existing.values.is_none() && def.values.is_some() {
+            existing.values = def.values;
+            changed = true;
+        }
+
+        if existing.macros.is_none() && def.macros.is_some() {
+            existing.macros = def.macros;
+            changed = true;
+        }
+
+        changed
+    }
+
+    pub(crate) fn push_res_with_import(
+        &mut self,
+        glob_imports: &mut PerNsGlobImports,
+        lookup: (LocalModuleId, Name),
+        def: PerNs,
+        def_import_type: ImportType,
+    ) -> bool {
+        let mut changed = false;
+        let existing = self.visible.entry(lookup.1.clone()).or_default();
+
         macro_rules! check_changed {
-            ($changed:ident, $existing:expr, $def:expr) => {
-                match ($existing, $def) {
+            (
+                $changed:ident,
+                ( $existing:ident / $def:ident ) . $field:ident,
+                $glob_imports:ident [ $lookup:ident ],
+                $def_import_type:ident
+            ) => {
+                match ($existing.$field, $def.$field) {
                     (None, Some(_)) => {
-                        $existing = $def;
+                        match $def_import_type {
+                            ImportType::Glob => {
+                                $glob_imports.$field.insert($lookup.clone());
+                            }
+                            ImportType::Named => {
+                                $glob_imports.$field.remove(&$lookup);
+                            }
+                        }
+
+                        $existing.$field = $def.$field;
                         $changed = true;
                     }
-                    (Some(e), Some(d)) if e.0 != d.0 => {
+                    (Some(_), Some(_))
+                        if $glob_imports.$field.contains(&$lookup)
+                            && matches!($def_import_type, ImportType::Named) =>
+                    {
                         mark::hit!(import_shadowed);
-                        $existing = $def;
+                        $glob_imports.$field.remove(&$lookup);
+                        $existing.$field = $def.$field;
                         $changed = true;
                     }
                     _ => {}
@@ -144,9 +203,9 @@ impl ItemScope {
             };
         }
 
-        check_changed!(changed, existing.types, def.types);
-        check_changed!(changed, existing.values, def.values);
-        check_changed!(changed, existing.macros, def.macros);
+        check_changed!(changed, (existing / def).types, glob_imports[lookup], def_import_type);
+        check_changed!(changed, (existing / def).values, glob_imports[lookup], def_import_type);
+        check_changed!(changed, (existing / def).macros, glob_imports[lookup], def_import_type);
 
         changed
     }
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index 2ced4f66b..a35ac1024 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -20,6 +20,7 @@ use test_utils::mark;
 use crate::{
     attr::Attrs,
     db::DefDatabase,
+    item_scope::{ImportType, PerNsGlobImports},
     item_tree::{
         self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind,
     },
@@ -80,6 +81,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, mut def_map: CrateDefMap) -> Cr
         mod_dirs: FxHashMap::default(),
         cfg_options,
         proc_macros,
+        from_glob_import: Default::default(),
     };
     collector.collect();
     collector.finish()
@@ -186,6 +188,7 @@ struct DefCollector<'a> {
     mod_dirs: FxHashMap<LocalModuleId, ModDir>,
     cfg_options: &'a CfgOptions,
     proc_macros: Vec<(Name, ProcMacroExpander)>,
+    from_glob_import: PerNsGlobImports,
 }
 
 impl DefCollector<'_> {
@@ -305,6 +308,7 @@ impl DefCollector<'_> {
                 self.def_map.root,
                 &[(name, PerNs::macros(macro_, Visibility::Public))],
                 Visibility::Public,
+                ImportType::Named,
             );
         }
     }
@@ -330,6 +334,7 @@ impl DefCollector<'_> {
             self.def_map.root,
             &[(name, PerNs::macros(macro_, Visibility::Public))],
             Visibility::Public,
+            ImportType::Named,
         );
     }
 
@@ -383,7 +388,6 @@ impl DefCollector<'_> {
             let imports = std::mem::replace(&mut self.unresolved_imports, Vec::new());
             for mut directive in imports {
                 directive.status = self.resolve_import(directive.module_id, &directive.import);
-
                 match directive.status {
                     PartialResolvedImport::Indeterminate(_) => {
                         self.record_resolved_import(&directive);
@@ -477,7 +481,7 @@ impl DefCollector<'_> {
                             .filter(|(_, res)| !res.is_none())
                             .collect::<Vec<_>>();
 
-                        self.update(module_id, &items, vis);
+                        self.update(module_id, &items, vis, ImportType::Glob);
                     } else {
                         // glob import from same crate => we do an initial
                         // import, and then need to propagate any further
@@ -499,7 +503,7 @@ impl DefCollector<'_> {
                             .filter(|(_, res)| !res.is_none())
                             .collect::<Vec<_>>();
 
-                        self.update(module_id, &items, vis);
+                        self.update(module_id, &items, vis, ImportType::Glob);
                         // record the glob import in case we add further items
                         let glob = self.glob_imports.entry(m.local_id).or_default();
                         if !glob.iter().any(|(mid, _)| *mid == module_id) {
@@ -529,7 +533,7 @@ impl DefCollector<'_> {
                             (name, res)
                         })
                         .collect::<Vec<_>>();
-                    self.update(module_id, &resolutions, vis);
+                    self.update(module_id, &resolutions, vis, ImportType::Glob);
                 }
                 Some(d) => {
                     log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@@ -555,15 +559,21 @@ impl DefCollector<'_> {
                         }
                     }
 
-                    self.update(module_id, &[(name, def)], vis);
+                    self.update(module_id, &[(name, def)], vis, ImportType::Named);
                 }
                 None => mark::hit!(bogus_paths),
             }
         }
     }
 
-    fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)], vis: Visibility) {
-        self.update_recursive(module_id, resolutions, vis, 0)
+    fn update(
+        &mut self,
+        module_id: LocalModuleId,
+        resolutions: &[(Name, PerNs)],
+        vis: Visibility,
+        import_type: ImportType,
+    ) {
+        self.update_recursive(module_id, resolutions, vis, import_type, 0)
     }
 
     fn update_recursive(
@@ -573,6 +583,7 @@ impl DefCollector<'_> {
         // All resolutions are imported with this visibility; the visibilies in
         // the `PerNs` values are ignored and overwritten
         vis: Visibility,
+        import_type: ImportType,
         depth: usize,
     ) {
         if depth > 100 {
@@ -582,7 +593,12 @@ impl DefCollector<'_> {
         let scope = &mut self.def_map.modules[module_id].scope;
         let mut changed = false;
         for (name, res) in resolutions {
-            changed |= scope.push_res(name.clone(), res.with_visibility(vis));
+            changed |= scope.push_res_with_import(
+                &mut self.from_glob_import,
+                (module_id, name.clone()),
+                res.with_visibility(vis),
+                import_type,
+            );
         }
 
         if !changed {
@@ -601,7 +617,13 @@ impl DefCollector<'_> {
             if !vis.is_visible_from_def_map(&self.def_map, glob_importing_module) {
                 continue;
             }
-            self.update_recursive(glob_importing_module, resolutions, glob_import_vis, depth + 1);
+            self.update_recursive(
+                glob_importing_module,
+                resolutions,
+                glob_import_vis,
+                ImportType::Glob,
+                depth + 1,
+            );
         }
     }
 
@@ -923,6 +945,7 @@ impl ModCollector<'_, '_> {
                         self.module_id,
                         &[(name.clone(), PerNs::from_def(id, vis, has_constructor))],
                         vis,
+                        ImportType::Named,
                     )
                 }
             }
@@ -1025,7 +1048,12 @@ impl ModCollector<'_, '_> {
         let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res };
         let def: ModuleDefId = module.into();
         self.def_collector.def_map.modules[self.module_id].scope.define_def(def);
-        self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis, false))], vis);
+        self.def_collector.update(
+            self.module_id,
+            &[(name, PerNs::from_def(def, vis, false))],
+            vis,
+            ImportType::Named,
+        );
         res
     }
 
@@ -1154,6 +1182,7 @@ mod tests {
             mod_dirs: FxHashMap::default(),
             cfg_options: &CfgOptions::default(),
             proc_macros: Default::default(),
+            from_glob_import: Default::default(),
         };
         collector.collect();
         collector.def_map
diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs
index 2f440975a..7f3d7509c 100644
--- a/crates/ra_hir_def/src/nameres/tests/globs.rs
+++ b/crates/ra_hir_def/src/nameres/tests/globs.rs
@@ -276,3 +276,93 @@ fn glob_shadowed_def() {
     "###
     );
 }
+
+#[test]
+fn glob_shadowed_def_reversed() {
+    let map = def_map(
+        r###"
+        //- /lib.rs
+        mod foo;
+        mod bar;
+
+        use bar::baz;
+        use foo::*;
+
+        use baz::Bar;
+
+        //- /foo.rs
+        pub mod baz {
+            pub struct Foo;
+        }
+
+        //- /bar.rs
+        pub mod baz {
+            pub struct Bar;
+        }
+        "###,
+    );
+    assert_snapshot!(map, @r###"
+        ⋮crate
+        ⋮Bar: t v
+        ⋮bar: t
+        ⋮baz: t
+        ⋮foo: t
+        ⋮
+        ⋮crate::bar
+        ⋮baz: t
+        ⋮
+        ⋮crate::bar::baz
+        ⋮Bar: t v
+        ⋮
+        ⋮crate::foo
+        ⋮baz: t
+        ⋮
+        ⋮crate::foo::baz
+        ⋮Foo: t v
+    "###
+    );
+}
+
+#[test]
+fn glob_shadowed_def_dependencies() {
+    let map = def_map(
+        r###"
+        //- /lib.rs
+        mod a { pub mod foo { pub struct X; } }
+        mod b { pub use super::a::foo; }
+        mod c { pub mod foo { pub struct Y; } }
+        mod d {
+            use super::c::foo;
+            use super::b::*;
+            use foo::Y;
+        }
+        "###,
+    );
+    assert_snapshot!(map, @r###"
+        ⋮crate
+        ⋮a: t
+        ⋮b: t
+        ⋮c: t
+        ⋮d: t
+        ⋮
+        ⋮crate::d
+        ⋮Y: t v
+        ⋮foo: t
+        ⋮
+        ⋮crate::c
+        ⋮foo: t
+        ⋮
+        ⋮crate::c::foo
+        ⋮Y: t v
+        ⋮
+        ⋮crate::b
+        ⋮foo: t
+        ⋮
+        ⋮crate::a
+        ⋮foo: t
+        ⋮
+        ⋮crate::a::foo
+        ⋮X: t v
+    "###
+    );
+}
diff --git a/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs b/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
index e9a5e4cba..753684201 100644
--- a/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
@@ -335,6 +335,22 @@ fn module_resolution_relative_path_2() {
 }
 
 #[test]
+fn module_resolution_relative_path_outside_root() {
+    let map = def_map(
+        r###"
+        //- /main.rs
+
+        #[path="../../../../../outside.rs"]
+        mod foo;
+        "###,
+    );
+
+    assert_snapshot!(map, @r###"
+        ⋮crate
+    "###);
+}
+
+#[test]
 fn module_resolution_explicit_path_mod_rs_2() {
     let map = def_map(
         r###"