From 79c90b5641d2934864c587380e4f050ab63ac029 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 24 Dec 2019 23:45:14 +0100 Subject: Collect visibility of items during nameres --- crates/ra_hir_def/src/nameres/path_resolution.rs | 27 ++++++++++++++++++++++++ crates/ra_hir_def/src/nameres/raw.rs | 17 ++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index 695014c7b..d88076aa7 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -21,6 +21,7 @@ use crate::{ nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, + visibility::{ResolvedVisibility, Visibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, }; @@ -64,6 +65,32 @@ impl CrateDefMap { self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)) } + pub(crate) fn resolve_visibility( + &self, + db: &impl DefDatabase, + original_module: LocalModuleId, + visibility: &Visibility, + ) -> Option { + match visibility { + Visibility::Module(path) => { + let (result, remaining) = + self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module); + if remaining.is_some() { + return None; + } + let types = result.take_types()?; + match types { + ModuleDefId::ModuleId(m) => Some(ResolvedVisibility::Module(m)), + _ => { + // error: visibility needs to refer to module + None + } + } + } + Visibility::Public => Some(ResolvedVisibility::Public), + } + } + // Returns Yes if we are sure that additions to `ItemMap` wouldn't change // the result. pub(super) fn resolve_path_fp_with_macro( diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs index 73dc08745..9dabb5b6d 100644 --- a/crates/ra_hir_def/src/nameres/raw.rs +++ b/crates/ra_hir_def/src/nameres/raw.rs @@ -16,12 +16,15 @@ use hir_expand::{ use ra_arena::{impl_arena_id, Arena, RawId}; use ra_prof::profile; use ra_syntax::{ - ast::{self, AttrsOwner, NameOwner}, + ast::{self, AttrsOwner, NameOwner, VisibilityOwner}, AstNode, }; use test_utils::tested_by; -use crate::{attr::Attrs, db::DefDatabase, path::ModPath, FileAstId, HirFileId, InFile}; +use crate::{ + attr::Attrs, db::DefDatabase, path::ModPath, visibility::Visibility, FileAstId, HirFileId, + InFile, +}; /// `RawItems` is a set of top-level items in a file (except for impls). /// @@ -138,6 +141,7 @@ pub struct ImportData { pub(super) is_prelude: bool, pub(super) is_extern_crate: bool, pub(super) is_macro_use: bool, + pub(super) visibility: Visibility, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -148,6 +152,7 @@ impl_arena_id!(Def); pub(super) struct DefData { pub(super) name: Name, pub(super) kind: DefKind, + pub(super) visibility: Visibility, } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -218,6 +223,7 @@ impl RawItemsCollector { fn add_item(&mut self, current_module: Option, item: ast::ModuleItem) { let attrs = self.parse_attrs(&item); + let visibility = Visibility::from_ast_with_hygiene(item.visibility(), &self.hygiene); let (kind, name) = match item { ast::ModuleItem::Module(module) => { self.add_module(current_module, module); @@ -266,7 +272,7 @@ impl RawItemsCollector { }; if let Some(name) = name { let name = name.as_name(); - let def = self.raw_items.defs.alloc(DefData { name, kind }); + let def = self.raw_items.defs.alloc(DefData { name, kind, visibility }); self.push_item(current_module, attrs, RawItemKind::Def(def)); } } @@ -302,6 +308,7 @@ impl RawItemsCollector { // FIXME: cfg_attr let is_prelude = use_item.has_atom_attr("prelude_import"); let attrs = self.parse_attrs(&use_item); + let visibility = Visibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene); let mut buf = Vec::new(); ModPath::expand_use_item( @@ -315,6 +322,7 @@ impl RawItemsCollector { is_prelude, is_extern_crate: false, is_macro_use: false, + visibility: visibility.clone(), }; buf.push(import_data); }, @@ -331,6 +339,8 @@ impl RawItemsCollector { ) { if let Some(name_ref) = extern_crate.name_ref() { let path = ModPath::from_name_ref(&name_ref); + let visibility = + Visibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene); let alias = extern_crate.alias().and_then(|a| a.name()).map(|it| it.as_name()); let attrs = self.parse_attrs(&extern_crate); // FIXME: cfg_attr @@ -342,6 +352,7 @@ impl RawItemsCollector { is_prelude: false, is_extern_crate: true, is_macro_use, + visibility, }; self.push_import(current_module, attrs, import_data); } -- cgit v1.2.3 From 8ac25f119eb45d425370d9f7f093bc206e6c4a9f Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 25 Dec 2019 15:00:10 +0100 Subject: Keep track of visibility during def collection --- crates/ra_hir_def/src/nameres/collector.rs | 33 +++++++++++++---- crates/ra_hir_def/src/nameres/path_resolution.rs | 45 +++++++++++++++--------- 2 files changed, 55 insertions(+), 23 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index b9f40d3dd..5b8478037 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -24,6 +24,7 @@ use crate::{ }, path::{ModPath, PathKind}, per_ns::PerNs, + visibility::ResolvedVisibility, AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; @@ -214,7 +215,10 @@ where // In Rust, `#[macro_export]` macros are unconditionally visible at the // crate root, even if the parent modules is **not** visible. if export { - self.update(self.def_map.root, &[(name, PerNs::macros(macro_))]); + self.update( + self.def_map.root, + &[(name, PerNs::macros(macro_, ResolvedVisibility::Public))], + ); } } @@ -351,6 +355,10 @@ where let import_id = directive.import_id; let import = &directive.import; let def = directive.status.namespaces(); + let vis = self + .def_map + .resolve_visibility(self.db, module_id, &directive.import.visibility) + .unwrap_or(ResolvedVisibility::Public); if import.is_glob { log::debug!("glob import: {:?}", import); @@ -365,8 +373,10 @@ 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(); + let items = scope.collect_resolutions_with_vis(vis); self.update(module_id, &items); } else { @@ -375,8 +385,10 @@ where // 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(); + let items = scope.collect_resolutions_with_vis(vis); self.update(module_id, &items); // record the glob import in case we add further items @@ -396,7 +408,7 @@ where .map(|(local_id, variant_data)| { let name = variant_data.name.clone(); let variant = EnumVariantId { parent: e, local_id }; - let res = PerNs::both(variant.into(), variant.into()); + let res = PerNs::both(variant.into(), variant.into(), vis); (name, res) }) .collect::>(); @@ -422,7 +434,7 @@ where } } - self.update(module_id, &[(name, def)]); + self.update(module_id, &[(name, def.with_visibility(vis))]); } None => tested_by!(bogus_paths), } @@ -701,8 +713,9 @@ where modules[self.module_id].children.insert(name.clone(), res); let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res }; 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, def.into())]); + self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))]); res } @@ -716,6 +729,7 @@ where let name = def.name.clone(); let container = ContainerId::ModuleId(module); + let vis = &def.visibility; let def: ModuleDefId = match def.kind { raw::DefKind::Function(ast_id) => FunctionLoc { container: container.into(), @@ -761,7 +775,12 @@ where .into(), }; self.def_collector.def_map.modules[self.module_id].scope.define_def(def); - self.def_collector.update(self.module_id, &[(name, def.into())]) + let vis = self + .def_collector + .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))]) } fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) { diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index d88076aa7..a56e3f08b 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -62,7 +62,9 @@ impl ResolvePathResult { impl CrateDefMap { pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs { - self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)) + self.extern_prelude + .get(name) + .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)) } pub(crate) fn resolve_visibility( @@ -115,17 +117,21 @@ impl CrateDefMap { PathKind::DollarCrate(krate) => { if krate == self.krate { tested_by!(macro_dollar_crate_self); - PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into()) + PerNs::types( + ModuleId { krate: self.krate, local_id: self.root }.into(), + ResolvedVisibility::Public, + ) } else { let def_map = db.crate_def_map(krate); let module = ModuleId { krate, local_id: def_map.root }; tested_by!(macro_dollar_crate_other); - PerNs::types(module.into()) + PerNs::types(module.into(), ResolvedVisibility::Public) } } - PathKind::Crate => { - PerNs::types(ModuleId { krate: self.krate, local_id: self.root }.into()) - } + PathKind::Crate => PerNs::types( + ModuleId { krate: self.krate, local_id: self.root }.into(), + ResolvedVisibility::Public, + ), // plain import or absolute path in 2015: crate-relative with // fallback to extern prelude (with the simplification in // rust-lang/rust#57745) @@ -153,7 +159,10 @@ impl CrateDefMap { let m = successors(Some(original_module), |m| self.modules[*m].parent) .nth(lvl as usize); if let Some(local_id) = m { - PerNs::types(ModuleId { krate: self.krate, local_id }.into()) + PerNs::types( + ModuleId { krate: self.krate, local_id }.into(), + ResolvedVisibility::Public, + ) } else { log::debug!("super path in root module"); return ResolvePathResult::empty(ReachedFixedPoint::Yes); @@ -167,7 +176,7 @@ impl CrateDefMap { }; if let Some(def) = self.extern_prelude.get(&segment) { log::debug!("absolute path {:?} resolved to crate {:?}", path, def); - PerNs::types(*def) + PerNs::types(*def, ResolvedVisibility::Public) } else { return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude } @@ -175,7 +184,7 @@ impl CrateDefMap { }; for (i, segment) in segments { - let curr = match curr_per_ns.take_types() { + let (curr, vis) = match curr_per_ns.take_types_vis() { Some(r) => r, None => { // we still have path segments left, but the path so far @@ -216,11 +225,11 @@ impl CrateDefMap { match enum_data.variant(&segment) { Some(local_id) => { let variant = EnumVariantId { parent: e, local_id }; - PerNs::both(variant.into(), variant.into()) + PerNs::both(variant.into(), variant.into(), ResolvedVisibility::Public) } None => { return ResolvePathResult::with( - PerNs::types(e.into()), + PerNs::types(e.into(), vis), ReachedFixedPoint::Yes, Some(i), Some(self.krate), @@ -238,7 +247,7 @@ impl CrateDefMap { ); return ResolvePathResult::with( - PerNs::types(s), + PerNs::types(s, vis), ReachedFixedPoint::Yes, Some(i), Some(self.krate), @@ -262,11 +271,15 @@ impl CrateDefMap { // - current module / scope // - extern prelude // - std prelude - let from_legacy_macro = - self[module].scope.get_legacy_macro(name).map_or_else(PerNs::none, PerNs::macros); + let from_legacy_macro = self[module] + .scope + .get_legacy_macro(name) + .map_or_else(PerNs::none, |m| PerNs::macros(m, ResolvedVisibility::Public)); let from_scope = self[module].scope.get(name, shadow); - let from_extern_prelude = - self.extern_prelude.get(name).map_or(PerNs::none(), |&it| PerNs::types(it)); + let from_extern_prelude = self + .extern_prelude + .get(name) + .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)); let from_prelude = self.resolve_in_prelude(db, name, shadow); from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude) -- cgit v1.2.3 From 21359c3ab5fc497d11b2c0f0435c7635336a726e Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 25 Dec 2019 18:05:16 +0100 Subject: Take visibility into account for glob imports --- crates/ra_hir_def/src/nameres/collector.rs | 80 +++++++++++++++------- crates/ra_hir_def/src/nameres/tests.rs | 6 +- crates/ra_hir_def/src/nameres/tests/globs.rs | 78 +++++++++++++++++++++ crates/ra_hir_def/src/nameres/tests/incremental.rs | 4 +- 4 files changed, 139 insertions(+), 29 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 5b8478037..51df44d2f 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -109,7 +109,7 @@ struct MacroDirective { struct DefCollector<'a, DB> { db: &'a DB, def_map: CrateDefMap, - glob_imports: FxHashMap>, + glob_imports: FxHashMap>, unresolved_imports: Vec, resolved_imports: Vec, unexpanded_macros: Vec, @@ -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); - - self.update(module_id, &items); + 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::>(); + + 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); - - self.update(module_id, &items); + 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::>(); + + 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::>(); - 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::>(); - 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) { diff --git a/crates/ra_hir_def/src/nameres/tests.rs b/crates/ra_hir_def/src/nameres/tests.rs index ff474b53b..78bcdc850 100644 --- a/crates/ra_hir_def/src/nameres/tests.rs +++ b/crates/ra_hir_def/src/nameres/tests.rs @@ -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 { diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs index 5e24cb94d..5f137d3a6 100644 --- a/crates/ra_hir_def/src/nameres/tests/globs.rs +++ b/crates/ra_hir_def/src/nameres/tests/globs.rs @@ -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); diff --git a/crates/ra_hir_def/src/nameres/tests/incremental.rs b/crates/ra_hir_def/src/nameres/tests/incremental.rs index ef2e9435c..faeb7aa4d 100644 --- a/crates/ra_hir_def/src/nameres/tests/incremental.rs +++ b/crates/ra_hir_def/src/nameres/tests/incremental.rs @@ -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::>().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::>().len(), 1); }); assert!(!format!("{:?}", events).contains("crate_def_map"), "{:#?}", events) } -- cgit v1.2.3 From 04e8eaa14b11c432d43ad95f3766f8649da30347 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 15:49:13 +0100 Subject: Handle privacy for modules --- crates/ra_hir_def/src/nameres/collector.rs | 26 ++++++++++++++++++++------ crates/ra_hir_def/src/nameres/raw.rs | 18 +++++++++++++++--- crates/ra_hir_def/src/nameres/tests/globs.rs | 3 +-- 3 files changed, 36 insertions(+), 11 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 51df44d2f..a80c4f8e9 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -677,9 +677,13 @@ where let is_macro_use = attrs.by_key("macro_use").exists(); match module { // inline module, just recurse - raw::ModuleData::Definition { name, items, ast_id } => { - let module_id = - self.push_child_module(name.clone(), AstId::new(self.file_id, *ast_id), None); + raw::ModuleData::Definition { name, visibility, items, ast_id } => { + let module_id = self.push_child_module( + name.clone(), + AstId::new(self.file_id, *ast_id), + None, + &visibility, + ); ModCollector { def_collector: &mut *self.def_collector, @@ -694,7 +698,7 @@ where } } // out of line module, resolve, parse and recurse - raw::ModuleData::Declaration { name, ast_id } => { + raw::ModuleData::Declaration { name, visibility, ast_id } => { let ast_id = AstId::new(self.file_id, *ast_id); match self.mod_dir.resolve_declaration( self.def_collector.db, @@ -703,7 +707,12 @@ where path_attr, ) { Ok((file_id, mod_dir)) => { - let module_id = self.push_child_module(name.clone(), ast_id, Some(file_id)); + let module_id = self.push_child_module( + name.clone(), + ast_id, + Some(file_id), + &visibility, + ); let raw_items = self.def_collector.db.raw_items(file_id.into()); ModCollector { def_collector: &mut *self.def_collector, @@ -734,7 +743,13 @@ where name: Name, declaration: AstId, definition: Option, + visibility: &crate::visibility::Visibility, ) -> LocalModuleId { + let vis = self + .def_collector + .def_map + .resolve_visibility(self.def_collector.db, self.module_id, visibility) + .unwrap_or(ResolvedVisibility::Public); let modules = &mut self.def_collector.def_map.modules; let res = modules.alloc(ModuleData::default()); modules[res].parent = Some(self.module_id); @@ -745,7 +760,6 @@ where modules[self.module_id].children.insert(name.clone(), res); let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res }; 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))], vis); res diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs index 9dabb5b6d..59f79f7cd 100644 --- a/crates/ra_hir_def/src/nameres/raw.rs +++ b/crates/ra_hir_def/src/nameres/raw.rs @@ -125,8 +125,17 @@ impl_arena_id!(Module); #[derive(Debug, PartialEq, Eq)] pub(super) enum ModuleData { - Declaration { name: Name, ast_id: FileAstId }, - Definition { name: Name, ast_id: FileAstId, items: Vec }, + Declaration { + name: Name, + visibility: Visibility, + ast_id: FileAstId, + }, + Definition { + name: Name, + visibility: Visibility, + ast_id: FileAstId, + items: Vec, + }, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -283,10 +292,12 @@ impl RawItemsCollector { None => return, }; let attrs = self.parse_attrs(&module); + let visibility = Visibility::from_ast_with_hygiene(module.visibility(), &self.hygiene); let ast_id = self.source_ast_id_map.ast_id(&module); if module.has_semi() { - let item = self.raw_items.modules.alloc(ModuleData::Declaration { name, ast_id }); + let item = + self.raw_items.modules.alloc(ModuleData::Declaration { name, visibility, ast_id }); self.push_item(current_module, attrs, RawItemKind::Module(item)); return; } @@ -294,6 +305,7 @@ impl RawItemsCollector { if let Some(item_list) = module.item_list() { let item = self.raw_items.modules.alloc(ModuleData::Definition { name, + visibility, ast_id, items: Vec::new(), }); diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs index 5f137d3a6..82d947b78 100644 --- a/crates/ra_hir_def/src/nameres/tests/globs.rs +++ b/crates/ra_hir_def/src/nameres/tests/globs.rs @@ -122,7 +122,7 @@ fn glob_privacy_2() { use foo::bar::*; //- /foo/mod.rs - pub mod bar; + mod bar; fn Foo() {}; pub struct Foo {}; @@ -136,7 +136,6 @@ fn glob_privacy_2() { crate Foo: t PubCrateStruct: t v - bar: t foo: t crate::foo -- cgit v1.2.3 From e1a2961273bdf7ef24c81f22fe86041a20812365 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 15:57:14 +0100 Subject: Rename Visibility -> RawVisibility --- crates/ra_hir_def/src/nameres/collector.rs | 2 +- crates/ra_hir_def/src/nameres/path_resolution.rs | 8 ++++---- crates/ra_hir_def/src/nameres/raw.rs | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index a80c4f8e9..f4678d145 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -743,7 +743,7 @@ where name: Name, declaration: AstId, definition: Option, - visibility: &crate::visibility::Visibility, + visibility: &crate::visibility::RawVisibility, ) -> LocalModuleId { let vis = self .def_collector diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index a56e3f08b..8a6256eee 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -21,7 +21,7 @@ use crate::{ nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{ResolvedVisibility, Visibility}, + visibility::{RawVisibility, ResolvedVisibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, }; @@ -71,10 +71,10 @@ impl CrateDefMap { &self, db: &impl DefDatabase, original_module: LocalModuleId, - visibility: &Visibility, + visibility: &RawVisibility, ) -> Option { match visibility { - Visibility::Module(path) => { + RawVisibility::Module(path) => { let (result, remaining) = self.resolve_path(db, original_module, &path, BuiltinShadowMode::Module); if remaining.is_some() { @@ -89,7 +89,7 @@ impl CrateDefMap { } } } - Visibility::Public => Some(ResolvedVisibility::Public), + RawVisibility::Public => Some(ResolvedVisibility::Public), } } diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs index 59f79f7cd..fac1169ef 100644 --- a/crates/ra_hir_def/src/nameres/raw.rs +++ b/crates/ra_hir_def/src/nameres/raw.rs @@ -22,7 +22,7 @@ use ra_syntax::{ use test_utils::tested_by; use crate::{ - attr::Attrs, db::DefDatabase, path::ModPath, visibility::Visibility, FileAstId, HirFileId, + attr::Attrs, db::DefDatabase, path::ModPath, visibility::RawVisibility, FileAstId, HirFileId, InFile, }; @@ -127,12 +127,12 @@ impl_arena_id!(Module); pub(super) enum ModuleData { Declaration { name: Name, - visibility: Visibility, + visibility: RawVisibility, ast_id: FileAstId, }, Definition { name: Name, - visibility: Visibility, + visibility: RawVisibility, ast_id: FileAstId, items: Vec, }, @@ -150,7 +150,7 @@ pub struct ImportData { pub(super) is_prelude: bool, pub(super) is_extern_crate: bool, pub(super) is_macro_use: bool, - pub(super) visibility: Visibility, + pub(super) visibility: RawVisibility, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -161,7 +161,7 @@ impl_arena_id!(Def); pub(super) struct DefData { pub(super) name: Name, pub(super) kind: DefKind, - pub(super) visibility: Visibility, + pub(super) visibility: RawVisibility, } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -232,7 +232,7 @@ impl RawItemsCollector { fn add_item(&mut self, current_module: Option, item: ast::ModuleItem) { let attrs = self.parse_attrs(&item); - let visibility = Visibility::from_ast_with_hygiene(item.visibility(), &self.hygiene); + let visibility = RawVisibility::from_ast_with_hygiene(item.visibility(), &self.hygiene); let (kind, name) = match item { ast::ModuleItem::Module(module) => { self.add_module(current_module, module); @@ -292,7 +292,7 @@ impl RawItemsCollector { None => return, }; let attrs = self.parse_attrs(&module); - let visibility = Visibility::from_ast_with_hygiene(module.visibility(), &self.hygiene); + let visibility = RawVisibility::from_ast_with_hygiene(module.visibility(), &self.hygiene); let ast_id = self.source_ast_id_map.ast_id(&module); if module.has_semi() { @@ -320,7 +320,7 @@ impl RawItemsCollector { // FIXME: cfg_attr let is_prelude = use_item.has_atom_attr("prelude_import"); let attrs = self.parse_attrs(&use_item); - let visibility = Visibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene); + let visibility = RawVisibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene); let mut buf = Vec::new(); ModPath::expand_use_item( @@ -352,7 +352,7 @@ impl RawItemsCollector { if let Some(name_ref) = extern_crate.name_ref() { let path = ModPath::from_name_ref(&name_ref); let visibility = - Visibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene); + RawVisibility::from_ast_with_hygiene(extern_crate.visibility(), &self.hygiene); let alias = extern_crate.alias().and_then(|a| a.name()).map(|it| it.as_name()); let attrs = self.parse_attrs(&extern_crate); // FIXME: cfg_attr -- cgit v1.2.3 From 50ebff257dafe6e820f002241466ff4a98aa1f32 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 16:00:10 +0100 Subject: Rename ResolvedVisibility -> Visibility --- crates/ra_hir_def/src/nameres/collector.rs | 23 ++++++++------------- crates/ra_hir_def/src/nameres/path_resolution.rs | 26 ++++++++++++------------ 2 files changed, 22 insertions(+), 27 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index f4678d145..63beaedc5 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -24,7 +24,7 @@ use crate::{ }, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::ResolvedVisibility, + visibility::Visibility, AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; @@ -109,7 +109,7 @@ struct MacroDirective { struct DefCollector<'a, DB> { db: &'a DB, def_map: CrateDefMap, - glob_imports: FxHashMap>, + glob_imports: FxHashMap>, unresolved_imports: Vec, resolved_imports: Vec, unexpanded_macros: Vec, @@ -217,8 +217,8 @@ where if export { self.update( self.def_map.root, - &[(name, PerNs::macros(macro_, ResolvedVisibility::Public))], - ResolvedVisibility::Public, + &[(name, PerNs::macros(macro_, Visibility::Public))], + Visibility::Public, ); } } @@ -358,7 +358,7 @@ where let vis = self .def_map .resolve_visibility(self.db, module_id, &directive.import.visibility) - .unwrap_or(ResolvedVisibility::Public); + .unwrap_or(Visibility::Public); if import.is_glob { log::debug!("glob import: {:?}", import); @@ -461,12 +461,7 @@ where } } - fn update( - &mut self, - module_id: LocalModuleId, - resolutions: &[(Name, PerNs)], - vis: ResolvedVisibility, - ) { + fn update(&mut self, module_id: LocalModuleId, resolutions: &[(Name, PerNs)], vis: Visibility) { self.update_recursive(module_id, resolutions, vis, 0) } @@ -476,7 +471,7 @@ where resolutions: &[(Name, PerNs)], // All resolutions are imported with this visibility; the visibilies in // the `PerNs` values are ignored and overwritten - vis: ResolvedVisibility, + vis: Visibility, depth: usize, ) { if depth > 100 { @@ -749,7 +744,7 @@ where .def_collector .def_map .resolve_visibility(self.def_collector.db, self.module_id, visibility) - .unwrap_or(ResolvedVisibility::Public); + .unwrap_or(Visibility::Public); let modules = &mut self.def_collector.def_map.modules; let res = modules.alloc(ModuleData::default()); modules[res].parent = Some(self.module_id); @@ -825,7 +820,7 @@ where .def_collector .def_map .resolve_visibility(self.def_collector.db, self.module_id, vis) - .unwrap_or(ResolvedVisibility::Public); + .unwrap_or(Visibility::Public); self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis) } diff --git a/crates/ra_hir_def/src/nameres/path_resolution.rs b/crates/ra_hir_def/src/nameres/path_resolution.rs index 8a6256eee..fd6422d60 100644 --- a/crates/ra_hir_def/src/nameres/path_resolution.rs +++ b/crates/ra_hir_def/src/nameres/path_resolution.rs @@ -21,7 +21,7 @@ use crate::{ nameres::{BuiltinShadowMode, CrateDefMap}, path::{ModPath, PathKind}, per_ns::PerNs, - visibility::{RawVisibility, ResolvedVisibility}, + visibility::{RawVisibility, Visibility}, AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, }; @@ -64,7 +64,7 @@ impl CrateDefMap { pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs { self.extern_prelude .get(name) - .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)) + .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public)) } pub(crate) fn resolve_visibility( @@ -72,7 +72,7 @@ impl CrateDefMap { db: &impl DefDatabase, original_module: LocalModuleId, visibility: &RawVisibility, - ) -> Option { + ) -> Option { match visibility { RawVisibility::Module(path) => { let (result, remaining) = @@ -82,14 +82,14 @@ impl CrateDefMap { } let types = result.take_types()?; match types { - ModuleDefId::ModuleId(m) => Some(ResolvedVisibility::Module(m)), + ModuleDefId::ModuleId(m) => Some(Visibility::Module(m)), _ => { // error: visibility needs to refer to module None } } } - RawVisibility::Public => Some(ResolvedVisibility::Public), + RawVisibility::Public => Some(Visibility::Public), } } @@ -119,18 +119,18 @@ impl CrateDefMap { tested_by!(macro_dollar_crate_self); PerNs::types( ModuleId { krate: self.krate, local_id: self.root }.into(), - ResolvedVisibility::Public, + Visibility::Public, ) } else { let def_map = db.crate_def_map(krate); let module = ModuleId { krate, local_id: def_map.root }; tested_by!(macro_dollar_crate_other); - PerNs::types(module.into(), ResolvedVisibility::Public) + PerNs::types(module.into(), Visibility::Public) } } PathKind::Crate => PerNs::types( ModuleId { krate: self.krate, local_id: self.root }.into(), - ResolvedVisibility::Public, + Visibility::Public, ), // plain import or absolute path in 2015: crate-relative with // fallback to extern prelude (with the simplification in @@ -161,7 +161,7 @@ impl CrateDefMap { if let Some(local_id) = m { PerNs::types( ModuleId { krate: self.krate, local_id }.into(), - ResolvedVisibility::Public, + Visibility::Public, ) } else { log::debug!("super path in root module"); @@ -176,7 +176,7 @@ impl CrateDefMap { }; if let Some(def) = self.extern_prelude.get(&segment) { log::debug!("absolute path {:?} resolved to crate {:?}", path, def); - PerNs::types(*def, ResolvedVisibility::Public) + PerNs::types(*def, Visibility::Public) } else { return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude } @@ -225,7 +225,7 @@ impl CrateDefMap { match enum_data.variant(&segment) { Some(local_id) => { let variant = EnumVariantId { parent: e, local_id }; - PerNs::both(variant.into(), variant.into(), ResolvedVisibility::Public) + PerNs::both(variant.into(), variant.into(), Visibility::Public) } None => { return ResolvePathResult::with( @@ -274,12 +274,12 @@ impl CrateDefMap { let from_legacy_macro = self[module] .scope .get_legacy_macro(name) - .map_or_else(PerNs::none, |m| PerNs::macros(m, ResolvedVisibility::Public)); + .map_or_else(PerNs::none, |m| PerNs::macros(m, Visibility::Public)); let from_scope = self[module].scope.get(name, shadow); let from_extern_prelude = self .extern_prelude .get(name) - .map_or(PerNs::none(), |&it| PerNs::types(it, ResolvedVisibility::Public)); + .map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public)); let from_prelude = self.resolve_in_prelude(db, name, shadow); from_legacy_macro.or(from_scope).or(from_extern_prelude).or(from_prelude) -- cgit v1.2.3 From 04cf98f8a6a67c899dd290d4b66c37794b24a568 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Thu, 26 Dec 2019 16:31:38 +0100 Subject: Fix cross-crate glob privacy handling --- crates/ra_hir_def/src/nameres/collector.rs | 7 +------ crates/ra_hir_def/src/nameres/tests/globs.rs | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 63beaedc5..30771d510 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -378,12 +378,7 @@ where .resolutions() // only keep visible names... .map(|(n, res)| { - ( - n, - res.filter_visibility(|v| { - v.visible_from_def_map(&self.def_map, module_id) - }), - ) + (n, res.filter_visibility(|v| v.visible_from_other_crate())) }) .filter(|(_, res)| !res.is_none()) .collect::>(); diff --git a/crates/ra_hir_def/src/nameres/tests/globs.rs b/crates/ra_hir_def/src/nameres/tests/globs.rs index 82d947b78..71fa0abe8 100644 --- a/crates/ra_hir_def/src/nameres/tests/globs.rs +++ b/crates/ra_hir_def/src/nameres/tests/globs.rs @@ -169,6 +169,26 @@ fn glob_across_crates() { ); } +#[test] +fn glob_privacy_across_crates() { + covers!(glob_across_crates); + let map = def_map( + " + //- /main.rs crate:main deps:test_crate + use test_crate::*; + + //- /lib.rs crate:test_crate + pub struct Baz; + struct Foo; + ", + ); + assert_snapshot!(map, @r###" + ⋮crate + ⋮Baz: t v + "### + ); +} + #[test] fn glob_enum() { covers!(glob_enum); -- cgit v1.2.3 From 9fd2c813ca355c3a1f10f54993c16e81778b867b Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 27 Dec 2019 11:24:31 +0100 Subject: visible_from -> is_visible_from --- crates/ra_hir_def/src/nameres/collector.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'crates/ra_hir_def/src/nameres') diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs index 30771d510..8a22b0585 100644 --- a/crates/ra_hir_def/src/nameres/collector.rs +++ b/crates/ra_hir_def/src/nameres/collector.rs @@ -378,7 +378,7 @@ where .resolutions() // only keep visible names... .map(|(n, res)| { - (n, res.filter_visibility(|v| v.visible_from_other_crate())) + (n, res.filter_visibility(|v| v.is_visible_from_other_crate())) }) .filter(|(_, res)| !res.is_none()) .collect::>(); @@ -398,7 +398,7 @@ where ( n, res.filter_visibility(|v| { - v.visible_from_def_map(&self.def_map, module_id) + v.is_visible_from_def_map(&self.def_map, module_id) }), ) }) @@ -492,7 +492,7 @@ where 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) { + 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); -- cgit v1.2.3