Take visibility into account for glob imports

This commit is contained in:
Florian Diebold 2019-12-25 18:05:16 +01:00
parent 8ac25f119e
commit 21359c3ab5
7 changed files with 163 additions and 41 deletions

View File

@ -149,16 +149,8 @@ impl ItemScope {
changed
}
#[cfg(test)]
pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> {
self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).collect()
}
pub(crate) fn collect_resolutions_with_vis(
&self,
vis: ResolvedVisibility,
) -> Vec<(Name, PerNs)> {
self.visible.iter().map(|(name, res)| (name.clone(), res.with_visibility(vis))).collect()
pub(crate) fn resolutions<'a>(&'a self) -> impl Iterator<Item = (Name, PerNs)> + 'a {
self.visible.iter().map(|(name, res)| (name.clone(), res.clone()))
}
pub(crate) fn collect_legacy_macros(&self) -> FxHashMap<Name, MacroDefId> {

View File

@ -109,7 +109,7 @@ struct MacroDirective {
struct DefCollector<'a, DB> {
db: &'a DB,
def_map: CrateDefMap,
glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, raw::Import)>>,
glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, ResolvedVisibility)>>,
unresolved_imports: Vec<ImportDirective>,
resolved_imports: Vec<ImportDirective>,
unexpanded_macros: Vec<MacroDirective>,
@ -218,6 +218,7 @@ where
self.update(
self.def_map.root,
&[(name, PerNs::macros(macro_, ResolvedVisibility::Public))],
ResolvedVisibility::Public,
);
}
}
@ -352,7 +353,6 @@ where
fn record_resolved_import(&mut self, directive: &ImportDirective) {
let module_id = directive.module_id;
let import_id = directive.import_id;
let import = &directive.import;
let def = directive.status.namespaces();
let vis = self
@ -373,28 +373,48 @@ where
let item_map = self.db.crate_def_map(m.krate);
let scope = &item_map[m.local_id].scope;
// TODO: only use names we can see
// Module scoped macros is included
let items = scope.collect_resolutions_with_vis(vis);
let items = scope
.resolutions()
// only keep visible names...
.map(|(n, res)| {
(
n,
res.filter_visibility(|v| {
v.visible_from_def_map(&self.def_map, module_id)
}),
)
})
.filter(|(_, res)| !res.is_none())
.collect::<Vec<_>>();
self.update(module_id, &items);
self.update(module_id, &items, vis);
} else {
// glob import from same crate => we do an initial
// import, and then need to propagate any further
// additions
let scope = &self.def_map[m.local_id].scope;
// TODO: only use names we can see
// Module scoped macros is included
let items = scope.collect_resolutions_with_vis(vis);
let items = scope
.resolutions()
// only keep visible names...
.map(|(n, res)| {
(
n,
res.filter_visibility(|v| {
v.visible_from_def_map(&self.def_map, module_id)
}),
)
})
.filter(|(_, res)| !res.is_none())
.collect::<Vec<_>>();
self.update(module_id, &items);
self.update(module_id, &items, vis);
// 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(|it| *it == (module_id, import_id)) {
glob.push((module_id, import_id));
if !glob.iter().any(|(mid, _)| *mid == module_id) {
glob.push((module_id, vis));
}
}
}
@ -412,7 +432,7 @@ where
(name, res)
})
.collect::<Vec<_>>();
self.update(module_id, &resolutions);
self.update(module_id, &resolutions, vis);
}
Some(d) => {
log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@ -434,21 +454,29 @@ where
}
}
self.update(module_id, &[(name, def.with_visibility(vis))]);
self.update(module_id, &[(name, def)], vis);
}
None => tested_by!(bogus_paths),
}
}
}
fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)]) {
self.update_recursive(module_id, resolutions, 0)
fn update(
&mut self,
module_id: LocalModuleId,
resolutions: &[(Name, PerNs)],
vis: ResolvedVisibility,
) {
self.update_recursive(module_id, resolutions, vis, 0)
}
fn update_recursive(
&mut self,
module_id: LocalModuleId,
resolutions: &[(Name, PerNs)],
// All resolutions are imported with this visibility; the visibilies in
// the `PerNs` values are ignored and overwritten
vis: ResolvedVisibility,
depth: usize,
) {
if depth > 100 {
@ -458,7 +486,7 @@ where
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);
changed |= scope.push_res(name.clone(), res.with_visibility(vis));
}
if !changed {
@ -471,9 +499,13 @@ where
.flat_map(|v| v.iter())
.cloned()
.collect::<Vec<_>>();
for (glob_importing_module, _glob_import) in glob_imports {
// We pass the glob import so that the tracked import in those modules is that glob import
self.update_recursive(glob_importing_module, resolutions, depth + 1);
for (glob_importing_module, glob_import_vis) in glob_imports {
// we know all resolutions have the same visibility (`vis`), so we
// just need to check that once
if !vis.visible_from_def_map(&self.def_map, glob_importing_module) {
continue;
}
self.update_recursive(glob_importing_module, resolutions, glob_import_vis, depth + 1);
}
}
@ -715,7 +747,7 @@ where
let def: ModuleDefId = module.into();
let vis = ResolvedVisibility::Public; // TODO handle module visibility
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))]);
self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis);
res
}
@ -780,7 +812,7 @@ where
.def_map
.resolve_visibility(self.def_collector.db, self.module_id, vis)
.unwrap_or(ResolvedVisibility::Public);
self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))])
self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis)
}
fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) {

View File

@ -12,8 +12,8 @@ use test_utils::covers;
use crate::{db::DefDatabase, nameres::*, test_db::TestDB, LocalModuleId};
fn def_map(fixtute: &str) -> String {
let dm = compute_crate_def_map(fixtute);
fn def_map(fixture: &str) -> String {
let dm = compute_crate_def_map(fixture);
render_crate_def_map(&dm)
}
@ -32,7 +32,7 @@ fn render_crate_def_map(map: &CrateDefMap) -> String {
*buf += path;
*buf += "\n";
let mut entries = map.modules[module].scope.collect_resolutions();
let mut entries: Vec<_> = map.modules[module].scope.resolutions().collect();
entries.sort_by_key(|(name, _)| name.clone());
for (name, def) in entries {

View File

@ -73,6 +73,84 @@ fn glob_2() {
);
}
#[test]
fn glob_privacy_1() {
let map = def_map(
"
//- /lib.rs
mod foo;
use foo::*;
//- /foo/mod.rs
pub mod bar;
pub use self::bar::*;
struct PrivateStructFoo;
//- /foo/bar.rs
pub struct Baz;
struct PrivateStructBar;
pub use super::*;
",
);
assert_snapshot!(map, @r###"
crate
Baz: t v
bar: t
foo: t
crate::foo
Baz: t v
PrivateStructFoo: t v
bar: t
crate::foo::bar
Baz: t v
PrivateStructBar: t v
PrivateStructFoo: t v
bar: t
"###
);
}
#[test]
fn glob_privacy_2() {
let map = def_map(
"
//- /lib.rs
mod foo;
use foo::*;
use foo::bar::*;
//- /foo/mod.rs
pub mod bar;
fn Foo() {};
pub struct Foo {};
//- /foo/bar.rs
pub(super) struct PrivateBaz;
struct PrivateBar;
pub(crate) struct PubCrateStruct;
",
);
assert_snapshot!(map, @r###"
crate
Foo: t
PubCrateStruct: t v
bar: t
foo: t
crate::foo
Foo: t v
bar: t
crate::foo::bar
PrivateBar: t v
PrivateBaz: t v
PubCrateStruct: t v
"###
);
}
#[test]
fn glob_across_crates() {
covers!(glob_across_crates);

View File

@ -116,7 +116,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() {
let events = db.log_executed(|| {
let crate_def_map = db.crate_def_map(krate);
let (_, module_data) = crate_def_map.modules.iter().last().unwrap();
assert_eq!(module_data.scope.collect_resolutions().len(), 1);
assert_eq!(module_data.scope.resolutions().collect::<Vec<_>>().len(), 1);
});
assert!(format!("{:?}", events).contains("crate_def_map"), "{:#?}", events)
}
@ -126,7 +126,7 @@ fn typing_inside_a_macro_should_not_invalidate_def_map() {
let events = db.log_executed(|| {
let crate_def_map = db.crate_def_map(krate);
let (_, module_data) = crate_def_map.modules.iter().last().unwrap();
assert_eq!(module_data.scope.collect_resolutions().len(), 1);
assert_eq!(module_data.scope.resolutions().collect::<Vec<_>>().len(), 1);
});
assert!(!format!("{:?}", events).contains("crate_def_map"), "{:#?}", events)
}

View File

@ -61,6 +61,14 @@ impl PerNs {
self.macros.map(|it| it.0)
}
pub fn filter_visibility(self, mut f: impl FnMut(ResolvedVisibility) -> bool) -> PerNs {
PerNs {
types: self.types.filter(|(_, v)| f(*v)),
values: self.values.filter(|(_, v)| f(*v)),
macros: self.macros.filter(|(_, v)| f(*v)),
}
}
pub fn with_visibility(self, vis: ResolvedVisibility) -> PerNs {
PerNs {
types: self.types.map(|(it, _)| (it, vis)),

View File

@ -134,13 +134,25 @@ impl ResolvedVisibility {
if from_module.krate != to_module.krate {
return false;
}
// from_module needs to be a descendant of to_module
let def_map = db.crate_def_map(from_module.krate);
self.visible_from_def_map(&def_map, from_module.local_id)
}
pub(crate) fn visible_from_def_map(
self,
def_map: &crate::nameres::CrateDefMap,
from_module: crate::LocalModuleId,
) -> bool {
let to_module = match self {
ResolvedVisibility::Module(m) => m,
ResolvedVisibility::Public => return true,
};
// from_module needs to be a descendant of to_module
let mut ancestors = std::iter::successors(Some(from_module), |m| {
let parent_id = def_map[m.local_id].parent?;
Some(ModuleId { local_id: parent_id, ..*m })
let parent_id = def_map[*m].parent?;
Some(parent_id)
});
ancestors.any(|m| m == to_module)
ancestors.any(|m| m == to_module.local_id)
}
}