diff --git a/crates/ra_hir_def/src/item_scope.rs b/crates/ra_hir_def/src/item_scope.rs index c81b966c39..4d446c7073 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, @@ -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 2ced4f66bb..a35ac1024b 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, 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::>(); - 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::>(); - 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::>(); - 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 2f440975a9..7f3d7509c9 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_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index d7ef9add66..7d8197f8b0 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!(