Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/hir-def/src/find_path.rs | 4 | ||||
| -rw-r--r-- | crates/hir-def/src/nameres/collector.rs | 22 | ||||
| -rw-r--r-- | crates/hir-def/src/nameres/tests/globs.rs | 39 | ||||
| -rw-r--r-- | crates/hir-def/src/visibility.rs | 42 |
4 files changed, 98 insertions, 9 deletions
diff --git a/crates/hir-def/src/find_path.rs b/crates/hir-def/src/find_path.rs index f5e03e5281..a615abd1bb 100644 --- a/crates/hir-def/src/find_path.rs +++ b/crates/hir-def/src/find_path.rs @@ -1025,7 +1025,7 @@ pub mod ast { check_found_path( r#" mod bar { - mod foo { pub(super) struct S; } + mod foo { pub(crate) struct S; } pub(crate) use foo::*; } $0 @@ -1047,7 +1047,7 @@ $0 check_found_path( r#" mod bar { - mod foo { pub(super) struct S; } + mod foo { pub(crate) struct S; } pub(crate) use foo::S as U; } $0 diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 22b9c2b4e3..b9aec93690 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -985,12 +985,8 @@ impl DefCollector<'_> { for (name, res) in resolutions { match name { Some(name) => { - changed |= self.push_res_and_update_glob_vis( - module_id, - name, - res.with_visibility(vis), - import, - ); + changed |= + self.push_res_and_update_glob_vis(module_id, name, *res, vis, import); } None => { let tr = match res.take_types() { @@ -1043,10 +1039,11 @@ impl DefCollector<'_> { .collect::<Vec<_>>(); for (glob_importing_module, glob_import_vis, use_) in glob_imports { + let vis = glob_import_vis.min(vis, &self.def_map).unwrap_or(glob_import_vis); self.update_recursive( glob_importing_module, resolutions, - glob_import_vis, + vis, Some(ImportType::Glob(use_)), depth + 1, ); @@ -1058,8 +1055,19 @@ impl DefCollector<'_> { module_id: LocalModuleId, name: &Name, mut defs: PerNs, + vis: Visibility, def_import_type: Option<ImportType>, ) -> bool { + if let Some((_, v, _)) = defs.types.as_mut() { + *v = v.min(vis, &self.def_map).unwrap_or(vis); + } + if let Some((_, v, _)) = defs.values.as_mut() { + *v = v.min(vis, &self.def_map).unwrap_or(vis); + } + if let Some((_, v, _)) = defs.macros.as_mut() { + *v = v.min(vis, &self.def_map).unwrap_or(vis); + } + let mut changed = false; if let Some(ImportType::Glob(_)) = def_import_type { diff --git a/crates/hir-def/src/nameres/tests/globs.rs b/crates/hir-def/src/nameres/tests/globs.rs index a2696055ca..543ab41cd5 100644 --- a/crates/hir-def/src/nameres/tests/globs.rs +++ b/crates/hir-def/src/nameres/tests/globs.rs @@ -412,3 +412,42 @@ use reexport::*; "#]], ); } + +#[test] +fn regression_18308() { + check( + r#" +use outer::*; + +mod outer { + mod inner_superglob { + pub use super::*; + } + + // The importing order matters! + pub use inner_superglob::*; + use super::glob_target::*; +} + +mod glob_target { + pub struct ShouldBePrivate; +} +"#, + expect![[r#" + crate + glob_target: t + outer: t + + crate::glob_target + ShouldBePrivate: t v + + crate::outer + ShouldBePrivate: t v + inner_superglob: t + + crate::outer::inner_superglob + ShouldBePrivate: t v + inner_superglob: t + "#]], + ); +} diff --git a/crates/hir-def/src/visibility.rs b/crates/hir-def/src/visibility.rs index 3aeb88047a..4edb683592 100644 --- a/crates/hir-def/src/visibility.rs +++ b/crates/hir-def/src/visibility.rs @@ -191,6 +191,11 @@ impl Visibility { return None; } + let def_block = def_map.block_id(); + if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) { + return None; + } + let mut a_ancestors = iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent); let mut b_ancestors = @@ -210,6 +215,43 @@ impl Visibility { } } } + + /// Returns the least permissive visibility of `self` and `other`. + /// + /// If there is no subset relation between `self` and `other`, returns `None` (ie. they're only + /// visible in unrelated modules). + pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> { + match (self, other) { + (vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis), + (Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => { + if mod_a.krate != mod_b.krate { + return None; + } + + let def_block = def_map.block_id(); + if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) { + return None; + } + + let mut a_ancestors = + iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent); + let mut b_ancestors = + iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent); + + if a_ancestors.any(|m| m == mod_b.local_id) { + // B is above A + return Some(Visibility::Module(mod_a, expl_b)); + } + + if b_ancestors.any(|m| m == mod_a.local_id) { + // A is above B + return Some(Visibility::Module(mod_b, expl_a)); + } + + None + } + } + } } /// Whether the item was imported through an explicit `pub(crate) use` or just a `use` without |