Skip to content

Commit 981ea19

Browse files
authored
Merge pull request rust-lang#18390 from ShoyuVanilla/issue-18308
fix: Prevent public re-export of private item
2 parents 7147bc9 + eee4037 commit 981ea19

File tree

4 files changed

+98
-9
lines changed

4 files changed

+98
-9
lines changed

src/tools/rust-analyzer/crates/hir-def/src/find_path.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1025,7 +1025,7 @@ pub mod ast {
10251025
check_found_path(
10261026
r#"
10271027
mod bar {
1028-
mod foo { pub(super) struct S; }
1028+
mod foo { pub(crate) struct S; }
10291029
pub(crate) use foo::*;
10301030
}
10311031
$0
@@ -1047,7 +1047,7 @@ $0
10471047
check_found_path(
10481048
r#"
10491049
mod bar {
1050-
mod foo { pub(super) struct S; }
1050+
mod foo { pub(crate) struct S; }
10511051
pub(crate) use foo::S as U;
10521052
}
10531053
$0

src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs

+15-7
Original file line numberDiff line numberDiff line change
@@ -985,12 +985,8 @@ impl DefCollector<'_> {
985985
for (name, res) in resolutions {
986986
match name {
987987
Some(name) => {
988-
changed |= self.push_res_and_update_glob_vis(
989-
module_id,
990-
name,
991-
res.with_visibility(vis),
992-
import,
993-
);
988+
changed |=
989+
self.push_res_and_update_glob_vis(module_id, name, *res, vis, import);
994990
}
995991
None => {
996992
let tr = match res.take_types() {
@@ -1043,10 +1039,11 @@ impl DefCollector<'_> {
10431039
.collect::<Vec<_>>();
10441040

10451041
for (glob_importing_module, glob_import_vis, use_) in glob_imports {
1042+
let vis = glob_import_vis.min(vis, &self.def_map).unwrap_or(glob_import_vis);
10461043
self.update_recursive(
10471044
glob_importing_module,
10481045
resolutions,
1049-
glob_import_vis,
1046+
vis,
10501047
Some(ImportType::Glob(use_)),
10511048
depth + 1,
10521049
);
@@ -1058,8 +1055,19 @@ impl DefCollector<'_> {
10581055
module_id: LocalModuleId,
10591056
name: &Name,
10601057
mut defs: PerNs,
1058+
vis: Visibility,
10611059
def_import_type: Option<ImportType>,
10621060
) -> bool {
1061+
if let Some((_, v, _)) = defs.types.as_mut() {
1062+
*v = v.min(vis, &self.def_map).unwrap_or(vis);
1063+
}
1064+
if let Some((_, v, _)) = defs.values.as_mut() {
1065+
*v = v.min(vis, &self.def_map).unwrap_or(vis);
1066+
}
1067+
if let Some((_, v, _)) = defs.macros.as_mut() {
1068+
*v = v.min(vis, &self.def_map).unwrap_or(vis);
1069+
}
1070+
10631071
let mut changed = false;
10641072

10651073
if let Some(ImportType::Glob(_)) = def_import_type {

src/tools/rust-analyzer/crates/hir-def/src/nameres/tests/globs.rs

+39
Original file line numberDiff line numberDiff line change
@@ -412,3 +412,42 @@ use reexport::*;
412412
"#]],
413413
);
414414
}
415+
416+
#[test]
417+
fn regression_18308() {
418+
check(
419+
r#"
420+
use outer::*;
421+
422+
mod outer {
423+
mod inner_superglob {
424+
pub use super::*;
425+
}
426+
427+
// The importing order matters!
428+
pub use inner_superglob::*;
429+
use super::glob_target::*;
430+
}
431+
432+
mod glob_target {
433+
pub struct ShouldBePrivate;
434+
}
435+
"#,
436+
expect![[r#"
437+
crate
438+
glob_target: t
439+
outer: t
440+
441+
crate::glob_target
442+
ShouldBePrivate: t v
443+
444+
crate::outer
445+
ShouldBePrivate: t v
446+
inner_superglob: t
447+
448+
crate::outer::inner_superglob
449+
ShouldBePrivate: t v
450+
inner_superglob: t
451+
"#]],
452+
);
453+
}

src/tools/rust-analyzer/crates/hir-def/src/visibility.rs

+42
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,11 @@ impl Visibility {
191191
return None;
192192
}
193193

194+
let def_block = def_map.block_id();
195+
if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
196+
return None;
197+
}
198+
194199
let mut a_ancestors =
195200
iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
196201
let mut b_ancestors =
@@ -210,6 +215,43 @@ impl Visibility {
210215
}
211216
}
212217
}
218+
219+
/// Returns the least permissive visibility of `self` and `other`.
220+
///
221+
/// If there is no subset relation between `self` and `other`, returns `None` (ie. they're only
222+
/// visible in unrelated modules).
223+
pub(crate) fn min(self, other: Visibility, def_map: &DefMap) -> Option<Visibility> {
224+
match (self, other) {
225+
(vis, Visibility::Public) | (Visibility::Public, vis) => Some(vis),
226+
(Visibility::Module(mod_a, expl_a), Visibility::Module(mod_b, expl_b)) => {
227+
if mod_a.krate != mod_b.krate {
228+
return None;
229+
}
230+
231+
let def_block = def_map.block_id();
232+
if (mod_a.containing_block(), mod_b.containing_block()) != (def_block, def_block) {
233+
return None;
234+
}
235+
236+
let mut a_ancestors =
237+
iter::successors(Some(mod_a.local_id), |&m| def_map[m].parent);
238+
let mut b_ancestors =
239+
iter::successors(Some(mod_b.local_id), |&m| def_map[m].parent);
240+
241+
if a_ancestors.any(|m| m == mod_b.local_id) {
242+
// B is above A
243+
return Some(Visibility::Module(mod_a, expl_b));
244+
}
245+
246+
if b_ancestors.any(|m| m == mod_a.local_id) {
247+
// A is above B
248+
return Some(Visibility::Module(mod_b, expl_a));
249+
}
250+
251+
None
252+
}
253+
}
254+
}
213255
}
214256

215257
/// Whether the item was imported through an explicit `pub(crate) use` or just a `use` without

0 commit comments

Comments
 (0)