diff --git a/crates/ra_hir_def/src/body/lower.rs b/crates/ra_hir_def/src/body/lower.rs index d749c828da0..3ced648e56f 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, ImportType}, + item_scope::BuiltinShadowMode, item_tree::{FileItemTreeId, ItemTree, ItemTreeNode}, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, @@ -81,7 +81,6 @@ pub(super) fn lower( map }, expander, - import_types: FxHashMap::default(), } .collect(params, body) } @@ -94,7 +93,6 @@ struct ExprCollector<'a> { source_map: BodySourceMap, item_trees: FxHashMap>, - import_types: FxHashMap, } impl ExprCollector<'_> { @@ -713,10 +711,8 @@ 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 511c08a8d78..d0923df6d8a 100644 --- a/crates/ra_hir_def/src/item_scope.rs +++ b/crates/ra_hir_def/src/item_scope.rs @@ -4,12 +4,12 @@ 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)] @@ -19,13 +19,6 @@ pub(crate) enum ImportType { } 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, @@ -34,6 +27,13 @@ impl ImportType { } } +#[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, @@ -145,30 +145,65 @@ impl ItemScope { self.legacy_macros.insert(name, mac); } - pub(crate) fn push_res( + pub(crate) fn push_res(&mut self, name: Name, def: PerNs) -> bool { + 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, - existing_import_map: &mut FxHashMap, - name: Name, + glob_imports: &mut PerNsGlobImports, + lookup: (LocalModuleId, Name), def: PerNs, def_import_type: ImportType, ) -> bool { let mut changed = false; - let existing = self.visible.entry(name.clone()).or_default(); - let existing_import_type = existing_import_map.entry(name).or_insert(def_import_type); + let existing = self.visible.entry(lookup.1.clone()).or_default(); macro_rules! check_changed { - ($changed:ident, ($existing:ident/$def:ident).$field:ident, $existing_import_type:ident, $def_import_type:ident) => { + ( + $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_import_type = $def_import_type; + 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(_), Some(_)) - if $existing_import_type.is_glob() && $def_import_type.is_named() => + if $glob_imports.$field.contains(&$lookup) + && $def_import_type.is_named() => { mark::hit!(import_shadowed); - *$existing_import_type = $def_import_type; + $glob_imports.$field.remove(&$lookup); $existing.$field = $def.$field; $changed = true; } @@ -177,9 +212,9 @@ impl ItemScope { }; } - 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); + 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 f7b99e0bef7..e74f67b3d9f 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -20,7 +20,7 @@ use test_utils::mark; use crate::{ attr::Attrs, db::DefDatabase, - item_scope::ImportType, + item_scope::{ImportType, PerNsGlobImports}, item_tree::{ self, FileItemTreeId, ItemTree, ItemTreeId, MacroCall, Mod, ModItem, ModKind, StructDefKind, }, @@ -81,7 +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(), + from_glob_import: Default::default(), }; collector.collect(); collector.finish() @@ -188,7 +188,7 @@ struct DefCollector<'a> { mod_dirs: FxHashMap, cfg_options: &'a CfgOptions, proc_macros: Vec<(Name, ProcMacroExpander)>, - import_types: FxHashMap, + from_glob_import: PerNsGlobImports, } impl DefCollector<'_> { @@ -595,9 +595,9 @@ 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( - &mut self.import_types, - name.clone(), + changed |= scope.push_res_with_import( + &mut self.from_glob_import, + (module_id, name.clone()), res.with_visibility(vis), import_type, ); @@ -1184,7 +1184,7 @@ mod tests { mod_dirs: FxHashMap::default(), cfg_options: &CfgOptions::default(), proc_macros: Default::default(), - import_types: FxHashMap::default(), + from_glob_import: Default::default(), }; collector.collect(); collector.def_map diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index d7ef9add667..7d8197f8b02 100644 --- a/crates/ra_hir_ty/src/tests/simple.rs +++ b/crates/ra_hir_ty/src/tests/simple.rs @@ -1739,6 +1739,52 @@ fn main() { assert_eq!(t, "u32"); } +// This test is actually testing the shadowing behavior within ra_hir_def. It +// lives here because the testing infrastructure in ra_hir_def isn't currently +// capable of asserting the necessary conditions. +#[test] +fn should_be_shadowing_imports() { + let t = type_at( + r#" +mod a { + pub fn foo() -> i8 {0} + pub struct foo { a: i8 } +} +mod b { pub fn foo () -> u8 {0} } +mod c { pub struct foo { a: u8 } } +mod d { + pub use super::a::*; + pub use super::c::foo; + pub use super::b::foo; +} + +fn main() { + d::foo()<|>; +}"#, + ); + assert_eq!(t, "u8"); + + let t = type_at( + r#" +mod a { + pub fn foo() -> i8 {0} + pub struct foo { a: i8 } +} +mod b { pub fn foo () -> u8 {0} } +mod c { pub struct foo { a: u8 } } +mod d { + pub use super::a::*; + pub use super::c::foo; + pub use super::b::foo; +} + +fn main() { + d::foo{a:0<|>}; +}"#, + ); + assert_eq!(t, "u8"); +} + #[test] fn closure_return() { assert_snapshot!(