Track import type outside of , use enum rather than bool to improve readability

4 files changed, 73 insertions, 40 deletions

diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs
index 3ced648e5..d749c828d 100644
--- a/crates/ra_hir_def/src/body/lower.rs
+++ b/crates/ra_hir_def/src/body/lower.rs
@@ -26,7 +26,7 @@ use crate::{
         dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal,
         LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement,
     },
-    item_scope::BuiltinShadowMode,
+    item_scope::{BuiltinShadowMode, ImportType},
     item_tree::{FileItemTreeId, ItemTree, ItemTreeNode},
     path::{GenericArgs, Path},
     type_ref::{Mutability, Rawness, TypeRef},
@@ -81,6 +81,7 @@ pub(super) fn lower(
             map
         },
         expander,
+        import_types: FxHashMap::default(),
     }
     .collect(params, body)
 }
@@ -93,6 +94,7 @@ struct ExprCollector<'a> {
     source_map: BodySourceMap,
 
     item_trees: FxHashMap<HirFileId, Arc<ItemTree>>,
+    import_types: FxHashMap<Name, ImportType>,
 }
 
 impl ExprCollector<'_> {
@@ -711,8 +713,10 @@ impl ExprCollector<'_> {
                     _ => true,
                 };
                 self.body.item_scope.push_res(
+                    &mut self.import_types,
                     name.as_name(),
                     crate::per_ns::PerNs::from_def(def, vis, has_constructor),
+                    ImportType::Named,
                 );
             }
         }
diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs
index 0184b6af9..511c08a8d 100644
--- a/crates/ra_hir_def/src/item_scope.rs
+++ b/crates/ra_hir_def/src/item_scope.rs
@@ -12,6 +12,28 @@ use crate::{
     Lookup, MacroDefId, ModuleDefId, TraitId,
 };
 
+#[derive(Copy, Clone)]
+pub(crate) enum ImportType {
+    Glob,
+    Named,
+}
+
+impl ImportType {
+    fn is_glob(&self) -> bool {
+        match self {
+            ImportType::Glob => true,
+            ImportType::Named => false,
+        }
+    }
+
+    fn is_named(&self) -> bool {
+        match self {
+            ImportType::Glob => false,
+            ImportType::Named => true,
+        }
+    }
+}
+
 #[derive(Debug, Default, PartialEq, Eq)]
 pub struct ItemScope {
     visible: FxHashMap<Name, PerNs>,
@@ -123,23 +145,30 @@ impl ItemScope {
         self.legacy_macros.insert(name, mac);
     }
 
-    pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool {
+    pub(crate) fn push_res(
+        &mut self,
+        existing_import_map: &mut FxHashMap<Name, ImportType>,
+        name: Name,
+        def: PerNs,
+        def_import_type: ImportType,
+    ) -> bool {
         let mut changed = false;
-        let existing = self.visible.entry(name).or_default();
+        let existing = self.visible.entry(name.clone()).or_default();
+        let existing_import_type = existing_import_map.entry(name).or_insert(def_import_type);
 
         macro_rules! check_changed {
-            ($changed:ident, ($existing:ident/$def:ident).$field:ident) => {
+            ($changed:ident, ($existing:ident/$def:ident).$field:ident, $existing_import_type:ident, $def_import_type:ident) => {
                 match ($existing.$field, $def.$field) {
                     (None, Some(_)) => {
-                        $existing.from_glob = $def.from_glob;
+                        *existing_import_type = $def_import_type;
                         $existing.$field = $def.$field;
                         $changed = true;
                     }
-                    // Only update if the new def came from a specific import and the existing
-                    // import came from a glob import.
-                    (Some(_), Some(_)) if $existing.from_glob && !$def.from_glob => {
+                    (Some(_), Some(_))
+                        if $existing_import_type.is_glob() && $def_import_type.is_named() =>
+                    {
                         mark::hit!(import_shadowed);
-                        $existing.from_glob = $def.from_glob;
+                        *$existing_import_type = $def_import_type;
                         $existing.$field = $def.$field;
                         $changed = true;
                     }
@@ -148,9 +177,9 @@ impl ItemScope {
             };
         }
 
-        check_changed!(changed, (existing / def).types);
-        check_changed!(changed, (existing / def).values);
-        check_changed!(changed, (existing / def).macros);
+        check_changed!(changed, (existing / def).types, existing_import_type, def_import_type);
+        check_changed!(changed, (existing / def).values, existing_import_type, def_import_type);
+        check_changed!(changed, (existing / def).macros, existing_import_type, 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 93f58e2c7..f7b99e0be 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,
     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,
+        import_types: FxHashMap::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)>,
+    import_types: FxHashMap<Name, ImportType>,
 }
 
 impl DefCollector<'_> {
@@ -305,7 +308,7 @@ impl DefCollector<'_> {
                 self.def_map.root,
                 &[(name, PerNs::macros(macro_, Visibility::Public))],
                 Visibility::Public,
-                false,
+                ImportType::Named,
             );
         }
     }
@@ -331,7 +334,7 @@ impl DefCollector<'_> {
             self.def_map.root,
             &[(name, PerNs::macros(macro_, Visibility::Public))],
             Visibility::Public,
-            false,
+            ImportType::Named,
         );
     }
 
@@ -478,7 +481,7 @@ impl DefCollector<'_> {
                             .filter(|(_, res)| !res.is_none())
                             .collect::<Vec<_>>();
 
-                        self.update(module_id, &items, vis, true);
+                        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
@@ -500,7 +503,7 @@ impl DefCollector<'_> {
                             .filter(|(_, res)| !res.is_none())
                             .collect::<Vec<_>>();
 
-                        self.update(module_id, &items, vis, true);
+                        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) {
@@ -530,7 +533,7 @@ impl DefCollector<'_> {
                             (name, res)
                         })
                         .collect::<Vec<_>>();
-                    self.update(module_id, &resolutions, vis, true);
+                    self.update(module_id, &resolutions, vis, ImportType::Glob);
                 }
                 Some(d) => {
                     log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@@ -556,7 +559,7 @@ impl DefCollector<'_> {
                         }
                     }
 
-                    self.update(module_id, &[(name, def)], vis, false);
+                    self.update(module_id, &[(name, def)], vis, ImportType::Named);
                 }
                 None => mark::hit!(bogus_paths),
             }
@@ -568,9 +571,9 @@ impl DefCollector<'_> {
         module_id: LocalModuleId,
         resolutions: &[(Name, PerNs)],
         vis: Visibility,
-        is_from_glob: bool,
+        import_type: ImportType,
     ) {
-        self.update_recursive(module_id, resolutions, vis, is_from_glob, 0)
+        self.update_recursive(module_id, resolutions, vis, import_type, 0)
     }
 
     fn update_recursive(
@@ -582,7 +585,7 @@ impl DefCollector<'_> {
         vis: Visibility,
         // All resolutions are imported with this glob status; the glob status
         // in the `PerNs` values are ignored and overwritten
-        is_from_glob: bool,
+        import_type: ImportType,
         depth: usize,
     ) {
         if depth > 100 {
@@ -592,8 +595,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).from_glob(is_from_glob));
+            changed |= scope.push_res(
+                &mut self.import_types,
+                name.clone(),
+                res.with_visibility(vis),
+                import_type,
+            );
         }
 
         if !changed {
@@ -616,7 +623,7 @@ impl DefCollector<'_> {
                 glob_importing_module,
                 resolutions,
                 glob_import_vis,
-                true,
+                ImportType::Glob,
                 depth + 1,
             );
         }
@@ -940,7 +947,7 @@ impl ModCollector<'_, '_> {
                         self.module_id,
                         &[(name.clone(), PerNs::from_def(id, vis, has_constructor))],
                         vis,
-                        false,
+                        ImportType::Named,
                     )
                 }
             }
@@ -1047,7 +1054,7 @@ impl ModCollector<'_, '_> {
             self.module_id,
             &[(name, PerNs::from_def(def, vis, false))],
             vis,
-            false,
+            ImportType::Named,
         );
         res
     }
@@ -1177,6 +1184,7 @@ mod tests {
             mod_dirs: FxHashMap::default(),
             cfg_options: &CfgOptions::default(),
             proc_macros: Default::default(),
+            import_types: FxHashMap::default(),
         };
         collector.collect();
         collector.def_map
diff --git a/crates/ra_hir_def/src/per_ns.rs b/crates/ra_hir_def/src/per_ns.rs
index e5cbca71d..74665c588 100644
--- a/crates/ra_hir_def/src/per_ns.rs
+++ b/crates/ra_hir_def/src/per_ns.rs
@@ -9,7 +9,6 @@ use crate::{item_scope::ItemInNs, visibility::Visibility, ModuleDefId};
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct PerNs {
-    pub from_glob: bool,
     pub types: Option<(ModuleDefId, Visibility)>,
     pub values: Option<(ModuleDefId, Visibility)>,
     pub macros: Option<(MacroDefId, Visibility)>,
@@ -17,29 +16,29 @@ pub struct PerNs {
 
 impl Default for PerNs {
     fn default() -> Self {
-        PerNs { from_glob: false, types: None, values: None, macros: None }
+        PerNs { types: None, values: None, macros: None }
     }
 }
 
 impl PerNs {
     pub fn none() -> PerNs {
-        PerNs { from_glob: false, types: None, values: None, macros: None }
+        PerNs { types: None, values: None, macros: None }
     }
 
     pub fn values(t: ModuleDefId, v: Visibility) -> PerNs {
-        PerNs { from_glob: false, types: None, values: Some((t, v)), macros: None }
+        PerNs { types: None, values: Some((t, v)), macros: None }
     }
 
     pub fn types(t: ModuleDefId, v: Visibility) -> PerNs {
-        PerNs { from_glob: false, types: Some((t, v)), values: None, macros: None }
+        PerNs { types: Some((t, v)), values: None, macros: None }
     }
 
     pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs {
-        PerNs { from_glob: false, types: Some((types, v)), values: Some((values, v)), macros: None }
+        PerNs { types: Some((types, v)), values: Some((values, v)), macros: None }
     }
 
     pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs {
-        PerNs { from_glob: false, types: None, values: None, macros: Some((macro_, v)) }
+        PerNs { types: None, values: None, macros: Some((macro_, v)) }
     }
 
     pub fn is_none(&self) -> bool {
@@ -64,7 +63,6 @@ impl PerNs {
 
     pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs {
         PerNs {
-            from_glob: self.from_glob,
             types: self.types.filter(|(_, v)| f(*v)),
             values: self.values.filter(|(_, v)| f(*v)),
             macros: self.macros.filter(|(_, v)| f(*v)),
@@ -73,7 +71,6 @@ impl PerNs {
 
     pub fn with_visibility(self, vis: Visibility) -> PerNs {
         PerNs {
-            from_glob: self.from_glob,
             types: self.types.map(|(it, _)| (it, vis)),
             values: self.values.map(|(it, _)| (it, vis)),
             macros: self.macros.map(|(it, _)| (it, vis)),
@@ -82,7 +79,6 @@ impl PerNs {
 
     pub fn or(self, other: PerNs) -> PerNs {
         PerNs {
-            from_glob: self.from_glob,
             types: self.types.or(other.types),
             values: self.values.or(other.values),
             macros: self.macros.or(other.macros),
@@ -96,8 +92,4 @@ impl PerNs {
             .chain(self.values.map(|it| ItemInNs::Values(it.0)).into_iter())
             .chain(self.macros.map(|it| ItemInNs::Macros(it.0)).into_iter())
     }
-
-    pub fn from_glob(self, from_glob: bool) -> PerNs {
-        PerNs { from_glob, ..self }
-    }
 }