2667: Visibility r=matklad a=flodiebold

This adds the infrastructure for handling visibility (for fields and methods, not in name resolution) in the HIR and code model, and as a first application hides struct fields from completions if they're not visible from the current module. (We might want to relax this again later, but I think it's ok for now?)

Co-authored-by: Florian Diebold <flodiebold@gmail.com>
This commit is contained in:
bors[bot] 2019-12-29 12:24:19 +00:00 committed by GitHub
commit cdcb3d3833
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 607 additions and 112 deletions

View file

@ -118,7 +118,7 @@ impl_froms!(
BuiltinType
);
pub use hir_def::attr::Attrs;
pub use hir_def::{attr::Attrs, visibility::Visibility};
impl Module {
pub(crate) fn new(krate: Crate, crate_module_id: LocalModuleId) -> Module {
@ -255,6 +255,15 @@ impl StructField {
}
}
impl HasVisibility for StructField {
fn visibility(&self, db: &impl HirDatabase) -> Visibility {
let variant_data = self.parent.variant_data(db);
let visibility = &variant_data.fields()[self.id].visibility;
let parent_id: hir_def::VariantId = self.parent.into();
visibility.resolve(db, &parent_id.resolver(db))
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct Struct {
pub(crate) id: StructId,
@ -1041,3 +1050,11 @@ impl<T: Into<AttrDef> + Copy> Docs for T {
db.documentation(def.into())
}
}
pub trait HasVisibility {
fn visibility(&self, db: &impl HirDatabase) -> Visibility;
fn is_visible_from(&self, db: &impl HirDatabase, module: Module) -> bool {
let vis = self.visibility(db);
vis.is_visible_from(db, module.id)
}
}

View file

@ -40,8 +40,8 @@ mod from_source;
pub use crate::{
code_model::{
Adt, AssocItem, AttrDef, Const, Crate, CrateDependency, DefWithBody, Docs, Enum,
EnumVariant, FieldSource, Function, GenericDef, HasAttrs, ImplBlock, Local, MacroDef,
Module, ModuleDef, ScopeDef, Static, Struct, StructField, Trait, Type, TypeAlias,
EnumVariant, FieldSource, Function, GenericDef, HasAttrs, HasVisibility, ImplBlock, Local,
MacroDef, Module, ModuleDef, ScopeDef, Static, Struct, StructField, Trait, Type, TypeAlias,
TypeParam, Union, VariantDef,
},
from_source::FromSource,

View file

@ -9,11 +9,12 @@ use hir_expand::{
};
use ra_arena::{map::ArenaMap, Arena};
use ra_prof::profile;
use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner};
use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
use crate::{
db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, EnumId,
LocalEnumVariantId, LocalStructFieldId, Lookup, StructId, UnionId, VariantId,
db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef,
visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId,
UnionId, VariantId,
};
/// Note that we use `StructData` for unions as well!
@ -47,13 +48,14 @@ pub enum VariantData {
pub struct StructFieldData {
pub name: Name,
pub type_ref: TypeRef,
pub visibility: RawVisibility,
}
impl StructData {
pub(crate) fn struct_data_query(db: &impl DefDatabase, id: StructId) -> Arc<StructData> {
let src = id.lookup(db).source(db);
let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
let variant_data = VariantData::new(src.value.kind());
let variant_data = VariantData::new(db, src.map(|s| s.kind()));
let variant_data = Arc::new(variant_data);
Arc::new(StructData { name, variant_data })
}
@ -61,10 +63,12 @@ impl StructData {
let src = id.lookup(db).source(db);
let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
let variant_data = VariantData::new(
src.value
.record_field_def_list()
.map(ast::StructKind::Record)
.unwrap_or(ast::StructKind::Unit),
db,
src.map(|s| {
s.record_field_def_list()
.map(ast::StructKind::Record)
.unwrap_or(ast::StructKind::Unit)
}),
);
let variant_data = Arc::new(variant_data);
Arc::new(StructData { name, variant_data })
@ -77,7 +81,7 @@ impl EnumData {
let src = e.lookup(db).source(db);
let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
let mut trace = Trace::new_for_arena();
lower_enum(&mut trace, &src.value);
lower_enum(db, &mut trace, &src);
Arc::new(EnumData { name, variants: trace.into_arena() })
}
@ -93,30 +97,31 @@ impl HasChildSource for EnumId {
fn child_source(&self, db: &impl DefDatabase) -> InFile<ArenaMap<Self::ChildId, Self::Value>> {
let src = self.lookup(db).source(db);
let mut trace = Trace::new_for_map();
lower_enum(&mut trace, &src.value);
lower_enum(db, &mut trace, &src);
src.with_value(trace.into_map())
}
}
fn lower_enum(
db: &impl DefDatabase,
trace: &mut Trace<LocalEnumVariantId, EnumVariantData, ast::EnumVariant>,
ast: &ast::EnumDef,
ast: &InFile<ast::EnumDef>,
) {
for var in ast.variant_list().into_iter().flat_map(|it| it.variants()) {
for var in ast.value.variant_list().into_iter().flat_map(|it| it.variants()) {
trace.alloc(
|| var.clone(),
|| EnumVariantData {
name: var.name().map_or_else(Name::missing, |it| it.as_name()),
variant_data: Arc::new(VariantData::new(var.kind())),
variant_data: Arc::new(VariantData::new(db, ast.with_value(var.kind()))),
},
);
}
}
impl VariantData {
fn new(flavor: ast::StructKind) -> Self {
fn new(db: &impl DefDatabase, flavor: InFile<ast::StructKind>) -> Self {
let mut trace = Trace::new_for_arena();
match lower_struct(&mut trace, &flavor) {
match lower_struct(db, &mut trace, &flavor) {
StructKind::Tuple => VariantData::Tuple(trace.into_arena()),
StructKind::Record => VariantData::Record(trace.into_arena()),
StructKind::Unit => VariantData::Unit,
@ -163,7 +168,7 @@ impl HasChildSource for VariantId {
}),
};
let mut trace = Trace::new_for_map();
lower_struct(&mut trace, &src.value);
lower_struct(db, &mut trace, &src);
src.with_value(trace.into_map())
}
}
@ -175,14 +180,15 @@ enum StructKind {
}
fn lower_struct(
db: &impl DefDatabase,
trace: &mut Trace<
LocalStructFieldId,
StructFieldData,
Either<ast::TupleFieldDef, ast::RecordFieldDef>,
>,
ast: &ast::StructKind,
ast: &InFile<ast::StructKind>,
) -> StructKind {
match ast {
match &ast.value {
ast::StructKind::Tuple(fl) => {
for (i, fd) in fl.fields().enumerate() {
trace.alloc(
@ -190,6 +196,7 @@ fn lower_struct(
|| StructFieldData {
name: Name::new_tuple_field(i),
type_ref: TypeRef::from_ast_opt(fd.type_ref()),
visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())),
},
);
}
@ -202,6 +209,7 @@ fn lower_struct(
|| StructFieldData {
name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing),
type_ref: TypeRef::from_ast_opt(fd.ascribed_type()),
visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())),
},
);
}

View file

@ -543,7 +543,10 @@ where
};
self.body.item_scope.define_def(def);
if let Some(name) = name {
self.body.item_scope.push_res(name.as_name(), def.into());
let vis = crate::visibility::Visibility::Public; // FIXME determine correctly
self.body
.item_scope
.push_res(name.as_name(), crate::per_ns::PerNs::from_def(def, vis));
}
}
}

View file

@ -5,7 +5,10 @@ use hir_expand::name::Name;
use once_cell::sync::Lazy;
use rustc_hash::FxHashMap;
use crate::{per_ns::PerNs, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId, TraitId};
use crate::{
per_ns::PerNs, visibility::Visibility, AdtId, BuiltinType, ImplId, MacroDefId, ModuleDefId,
TraitId,
};
#[derive(Debug, Default, PartialEq, Eq)]
pub struct ItemScope {
@ -30,7 +33,7 @@ pub struct ItemScope {
static BUILTIN_SCOPE: Lazy<FxHashMap<Name, PerNs>> = Lazy::new(|| {
BuiltinType::ALL
.iter()
.map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into())))
.map(|(name, ty)| (name.clone(), PerNs::types(ty.clone().into(), Visibility::Public)))
.collect()
});
@ -144,8 +147,8 @@ impl ItemScope {
changed
}
pub(crate) fn collect_resolutions(&self) -> Vec<(Name, PerNs)> {
self.visible.iter().map(|(name, res)| (name.clone(), res.clone())).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> {
@ -153,20 +156,20 @@ impl ItemScope {
}
}
impl From<ModuleDefId> for PerNs {
fn from(def: ModuleDefId) -> PerNs {
impl PerNs {
pub(crate) fn from_def(def: ModuleDefId, v: Visibility) -> PerNs {
match def {
ModuleDefId::ModuleId(_) => PerNs::types(def),
ModuleDefId::FunctionId(_) => PerNs::values(def),
ModuleDefId::ModuleId(_) => PerNs::types(def, v),
ModuleDefId::FunctionId(_) => PerNs::values(def, v),
ModuleDefId::AdtId(adt) => match adt {
AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def),
AdtId::EnumId(_) => PerNs::types(def),
AdtId::StructId(_) | AdtId::UnionId(_) => PerNs::both(def, def, v),
AdtId::EnumId(_) => PerNs::types(def, v),
},
ModuleDefId::EnumVariantId(_) => PerNs::both(def, def),
ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def),
ModuleDefId::TraitId(_) => PerNs::types(def),
ModuleDefId::TypeAliasId(_) => PerNs::types(def),
ModuleDefId::BuiltinType(_) => PerNs::types(def),
ModuleDefId::EnumVariantId(_) => PerNs::both(def, def, v),
ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => PerNs::values(def, v),
ModuleDefId::TraitId(_) => PerNs::types(def, v),
ModuleDefId::TypeAliasId(_) => PerNs::types(def, v),
ModuleDefId::BuiltinType(_) => PerNs::types(def, v),
}
}
}

View file

@ -36,6 +36,8 @@ pub mod nameres;
pub mod src;
pub mod child_by_source;
pub mod visibility;
#[cfg(test)]
mod test_db;
#[cfg(test)]

View file

@ -24,6 +24,7 @@ use crate::{
},
path::{ModPath, PathKind},
per_ns::PerNs,
visibility::Visibility,
AdtId, AstId, ConstLoc, ContainerId, EnumLoc, EnumVariantId, FunctionLoc, ImplLoc, Intern,
LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
};
@ -108,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, Visibility)>>,
unresolved_imports: Vec<ImportDirective>,
resolved_imports: Vec<ImportDirective>,
unexpanded_macros: Vec<MacroDirective>,
@ -214,7 +215,11 @@ 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_, Visibility::Public))],
Visibility::Public,
);
}
}
@ -348,9 +353,12 @@ 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
.def_map
.resolve_visibility(self.db, module_id, &directive.import.visibility)
.unwrap_or(Visibility::Public);
if import.is_glob {
log::debug!("glob import: {:?}", import);
@ -366,9 +374,16 @@ where
let scope = &item_map[m.local_id].scope;
// Module scoped macros is included
let items = scope.collect_resolutions();
let items = scope
.resolutions()
// only keep visible names...
.map(|(n, res)| {
(n, res.filter_visibility(|v| v.is_visible_from_other_crate()))
})
.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
@ -376,13 +391,25 @@ where
let scope = &self.def_map[m.local_id].scope;
// Module scoped macros is included
let items = scope.collect_resolutions();
let items = scope
.resolutions()
// only keep visible names...
.map(|(n, res)| {
(
n,
res.filter_visibility(|v| {
v.is_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));
}
}
}
@ -396,11 +423,11 @@ 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::<Vec<_>>();
self.update(module_id, &resolutions);
self.update(module_id, &resolutions, vis);
}
Some(d) => {
log::debug!("glob import {:?} from non-module/enum {:?}", import, d);
@ -422,21 +449,24 @@ where
}
}
self.update(module_id, &[(name, def)]);
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: Visibility) {
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: Visibility,
depth: usize,
) {
if depth > 100 {
@ -446,7 +476,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 {
@ -459,9 +489,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.is_visible_from_def_map(&self.def_map, glob_importing_module) {
continue;
}
self.update_recursive(glob_importing_module, resolutions, glob_import_vis, depth + 1);
}
}
@ -633,9 +667,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,
@ -650,7 +688,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,
@ -659,7 +697,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,
@ -690,7 +733,13 @@ where
name: Name,
declaration: AstId<ast::Module>,
definition: Option<FileId>,
visibility: &crate::visibility::RawVisibility,
) -> LocalModuleId {
let vis = self
.def_collector
.def_map
.resolve_visibility(self.def_collector.db, self.module_id, visibility)
.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);
@ -702,7 +751,7 @@ where
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, def.into())]);
self.def_collector.update(self.module_id, &[(name, PerNs::from_def(def, vis))], vis);
res
}
@ -716,6 +765,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 +811,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(Visibility::Public);
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

@ -21,6 +21,7 @@ use crate::{
nameres::{BuiltinShadowMode, CrateDefMap},
path::{ModPath, PathKind},
per_ns::PerNs,
visibility::{RawVisibility, Visibility},
AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId,
};
@ -61,7 +62,35 @@ 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, Visibility::Public))
}
pub(crate) fn resolve_visibility(
&self,
db: &impl DefDatabase,
original_module: LocalModuleId,
visibility: &RawVisibility,
) -> Option<Visibility> {
match visibility {
RawVisibility::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(Visibility::Module(m)),
_ => {
// error: visibility needs to refer to module
None
}
}
}
RawVisibility::Public => Some(Visibility::Public),
}
}
// Returns Yes if we are sure that additions to `ItemMap` wouldn't change
@ -88,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(),
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())
PerNs::types(module.into(), Visibility::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(),
Visibility::Public,
),
// plain import or absolute path in 2015: crate-relative with
// fallback to extern prelude (with the simplification in
// rust-lang/rust#57745)
@ -126,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(),
Visibility::Public,
)
} else {
log::debug!("super path in root module");
return ResolvePathResult::empty(ReachedFixedPoint::Yes);
@ -140,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, Visibility::Public)
} else {
return ResolvePathResult::empty(ReachedFixedPoint::No); // extern crate declarations can add to the extern prelude
}
@ -148,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
@ -189,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(), Visibility::Public)
}
None => {
return ResolvePathResult::with(
PerNs::types(e.into()),
PerNs::types(e.into(), vis),
ReachedFixedPoint::Yes,
Some(i),
Some(self.krate),
@ -211,7 +247,7 @@ impl CrateDefMap {
);
return ResolvePathResult::with(
PerNs::types(s),
PerNs::types(s, vis),
ReachedFixedPoint::Yes,
Some(i),
Some(self.krate),
@ -235,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, 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));
let from_extern_prelude = self
.extern_prelude
.get(name)
.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)

View file

@ -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::RawVisibility, FileAstId, HirFileId,
InFile,
};
/// `RawItems` is a set of top-level items in a file (except for impls).
///
@ -122,8 +125,17 @@ impl_arena_id!(Module);
#[derive(Debug, PartialEq, Eq)]
pub(super) enum ModuleData {
Declaration { name: Name, ast_id: FileAstId<ast::Module> },
Definition { name: Name, ast_id: FileAstId<ast::Module>, items: Vec<RawItem> },
Declaration {
name: Name,
visibility: RawVisibility,
ast_id: FileAstId<ast::Module>,
},
Definition {
name: Name,
visibility: RawVisibility,
ast_id: FileAstId<ast::Module>,
items: Vec<RawItem>,
},
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -138,6 +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: RawVisibility,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -148,6 +161,7 @@ impl_arena_id!(Def);
pub(super) struct DefData {
pub(super) name: Name,
pub(super) kind: DefKind,
pub(super) visibility: RawVisibility,
}
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
@ -218,6 +232,7 @@ impl RawItemsCollector {
fn add_item(&mut self, current_module: Option<Module>, item: ast::ModuleItem) {
let attrs = self.parse_attrs(&item);
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);
@ -266,7 +281,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));
}
}
@ -277,10 +292,12 @@ impl RawItemsCollector {
None => return,
};
let attrs = self.parse_attrs(&module);
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() {
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;
}
@ -288,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(),
});
@ -302,6 +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 = RawVisibility::from_ast_with_hygiene(use_item.visibility(), &self.hygiene);
let mut buf = Vec::new();
ModPath::expand_use_item(
@ -315,6 +334,7 @@ impl RawItemsCollector {
is_prelude,
is_extern_crate: false,
is_macro_use: false,
visibility: visibility.clone(),
};
buf.push(import_data);
},
@ -331,6 +351,8 @@ impl RawItemsCollector {
) {
if let Some(name_ref) = extern_crate.name_ref() {
let path = ModPath::from_name_ref(&name_ref);
let visibility =
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
@ -342,6 +364,7 @@ impl RawItemsCollector {
is_prelude: false,
is_extern_crate: true,
is_macro_use,
visibility,
};
self.push_import(current_module, attrs, import_data);
}

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,83 @@ 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
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
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);
@ -92,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);

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

@ -5,13 +5,13 @@
use hir_expand::MacroDefId;
use crate::ModuleDefId;
use crate::{visibility::Visibility, ModuleDefId};
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct PerNs {
pub types: Option<ModuleDefId>,
pub values: Option<ModuleDefId>,
pub macros: Option<MacroDefId>,
pub types: Option<(ModuleDefId, Visibility)>,
pub values: Option<(ModuleDefId, Visibility)>,
pub macros: Option<(MacroDefId, Visibility)>,
}
impl Default for PerNs {
@ -25,20 +25,20 @@ impl PerNs {
PerNs { types: None, values: None, macros: None }
}
pub fn values(t: ModuleDefId) -> PerNs {
PerNs { types: None, values: Some(t), macros: None }
pub fn values(t: ModuleDefId, v: Visibility) -> PerNs {
PerNs { types: None, values: Some((t, v)), macros: None }
}
pub fn types(t: ModuleDefId) -> PerNs {
PerNs { types: Some(t), values: None, macros: None }
pub fn types(t: ModuleDefId, v: Visibility) -> PerNs {
PerNs { types: Some((t, v)), values: None, macros: None }
}
pub fn both(types: ModuleDefId, values: ModuleDefId) -> PerNs {
PerNs { types: Some(types), values: Some(values), macros: None }
pub fn both(types: ModuleDefId, values: ModuleDefId, v: Visibility) -> PerNs {
PerNs { types: Some((types, v)), values: Some((values, v)), macros: None }
}
pub fn macros(macro_: MacroDefId) -> PerNs {
PerNs { types: None, values: None, macros: Some(macro_) }
pub fn macros(macro_: MacroDefId, v: Visibility) -> PerNs {
PerNs { types: None, values: None, macros: Some((macro_, v)) }
}
pub fn is_none(&self) -> bool {
@ -46,15 +46,35 @@ impl PerNs {
}
pub fn take_types(self) -> Option<ModuleDefId> {
self.types.map(|it| it.0)
}
pub fn take_types_vis(self) -> Option<(ModuleDefId, Visibility)> {
self.types
}
pub fn take_values(self) -> Option<ModuleDefId> {
self.values
self.values.map(|it| it.0)
}
pub fn take_macros(self) -> Option<MacroDefId> {
self.macros
self.macros.map(|it| it.0)
}
pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> 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: Visibility) -> PerNs {
PerNs {
types: self.types.map(|(it, _)| (it, vis)),
values: self.values.map(|(it, _)| (it, vis)),
macros: self.macros.map(|(it, _)| (it, vis)),
}
}
pub fn or(self, other: PerNs) -> PerNs {

View file

@ -19,6 +19,7 @@ use crate::{
nameres::CrateDefMap,
path::{ModPath, PathKind},
per_ns::PerNs,
visibility::{RawVisibility, Visibility},
AdtId, AssocContainerId, ConstId, ContainerId, DefWithBodyId, EnumId, EnumVariantId,
FunctionId, GenericDefId, HasModule, ImplId, LocalModuleId, Lookup, ModuleDefId, ModuleId,
StaticId, StructId, TraitId, TypeAliasId, TypeParamId, VariantId,
@ -231,6 +232,23 @@ impl Resolver {
Some(res)
}
pub fn resolve_visibility(
&self,
db: &impl DefDatabase,
visibility: &RawVisibility,
) -> Option<Visibility> {
match visibility {
RawVisibility::Module(_) => {
let (item_map, module) = match self.module() {
Some(it) => it,
None => return None,
};
item_map.resolve_visibility(db, module, visibility)
}
RawVisibility::Public => Some(Visibility::Public),
}
}
pub fn resolve_path_in_value_ns(
&self,
db: &impl DefDatabase,
@ -448,10 +466,10 @@ impl Scope {
f(name.clone(), ScopeDef::PerNs(def));
});
m.crate_def_map[m.module_id].scope.legacy_macros().for_each(|(name, macro_)| {
f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_)));
f(name.clone(), ScopeDef::PerNs(PerNs::macros(macro_, Visibility::Public)));
});
m.crate_def_map.extern_prelude.iter().for_each(|(name, &def)| {
f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into())));
f(name.clone(), ScopeDef::PerNs(PerNs::types(def.into(), Visibility::Public)));
});
if let Some(prelude) = m.crate_def_map.prelude {
let prelude_def_map = db.crate_def_map(prelude.krate);

View file

@ -0,0 +1,120 @@
//! Defines hir-level representation of visibility (e.g. `pub` and `pub(crate)`).
use hir_expand::{hygiene::Hygiene, InFile};
use ra_syntax::ast;
use crate::{
db::DefDatabase,
path::{ModPath, PathKind},
ModuleId,
};
/// Visibility of an item, not yet resolved.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum RawVisibility {
/// `pub(in module)`, `pub(crate)` or `pub(super)`. Also private, which is
/// equivalent to `pub(self)`.
Module(ModPath),
/// `pub`.
Public,
}
impl RawVisibility {
const fn private() -> RawVisibility {
let path = ModPath { kind: PathKind::Super(0), segments: Vec::new() };
RawVisibility::Module(path)
}
pub(crate) fn from_ast(
db: &impl DefDatabase,
node: InFile<Option<ast::Visibility>>,
) -> RawVisibility {
Self::from_ast_with_hygiene(node.value, &Hygiene::new(db, node.file_id))
}
pub(crate) fn from_ast_with_hygiene(
node: Option<ast::Visibility>,
hygiene: &Hygiene,
) -> RawVisibility {
let node = match node {
None => return RawVisibility::private(),
Some(node) => node,
};
match node.kind() {
ast::VisibilityKind::In(path) => {
let path = ModPath::from_src(path, hygiene);
let path = match path {
None => return RawVisibility::private(),
Some(path) => path,
};
RawVisibility::Module(path)
}
ast::VisibilityKind::PubCrate => {
let path = ModPath { kind: PathKind::Crate, segments: Vec::new() };
RawVisibility::Module(path)
}
ast::VisibilityKind::PubSuper => {
let path = ModPath { kind: PathKind::Super(1), segments: Vec::new() };
RawVisibility::Module(path)
}
ast::VisibilityKind::Pub => RawVisibility::Public,
}
}
pub fn resolve(
&self,
db: &impl DefDatabase,
resolver: &crate::resolver::Resolver,
) -> Visibility {
// we fall back to public visibility (i.e. fail open) if the path can't be resolved
resolver.resolve_visibility(db, self).unwrap_or(Visibility::Public)
}
}
/// Visibility of an item, with the path resolved.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum Visibility {
/// Visibility is restricted to a certain module.
Module(ModuleId),
/// Visibility is unrestricted.
Public,
}
impl Visibility {
pub fn is_visible_from(self, db: &impl DefDatabase, from_module: ModuleId) -> bool {
let to_module = match self {
Visibility::Module(m) => m,
Visibility::Public => return true,
};
// if they're not in the same crate, it can't be visible
if from_module.krate != to_module.krate {
return false;
}
let def_map = db.crate_def_map(from_module.krate);
self.is_visible_from_def_map(&def_map, from_module.local_id)
}
pub(crate) fn is_visible_from_other_crate(self) -> bool {
match self {
Visibility::Module(_) => false,
Visibility::Public => true,
}
}
pub(crate) fn is_visible_from_def_map(
self,
def_map: &crate::nameres::CrateDefMap,
from_module: crate::LocalModuleId,
) -> bool {
let to_module = match self {
Visibility::Module(m) => m,
Visibility::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].parent?;
Some(parent_id)
});
ancestors.any(|m| m == to_module.local_id)
}
}

View file

@ -1,6 +1,6 @@
//! FIXME: write short doc here
use hir::Type;
use hir::{HasVisibility, Type};
use crate::completion::completion_item::CompletionKind;
use crate::{
@ -38,9 +38,15 @@ pub(super) fn complete_dot(acc: &mut Completions, ctx: &CompletionContext) {
fn complete_fields(acc: &mut Completions, ctx: &CompletionContext, receiver: &Type) {
for receiver in receiver.autoderef(ctx.db) {
for (field, ty) in receiver.fields(ctx.db) {
if ctx.module.map_or(false, |m| !field.is_visible_from(ctx.db, m)) {
// Skip private field. FIXME: If the definition location of the
// field is editable, we should show the completion
continue;
}
acc.add_field(ctx, field, &ty);
}
for (i, ty) in receiver.tuple_fields(ctx.db).into_iter().enumerate() {
// FIXME: Handle visibility
acc.add_tuple_field(ctx, i, &ty);
}
}
@ -186,6 +192,55 @@ mod tests {
);
}
#[test]
fn test_struct_field_visibility_private() {
assert_debug_snapshot!(
do_ref_completion(
r"
mod inner {
struct A {
private_field: u32,
pub pub_field: u32,
pub(crate) crate_field: u32,
pub(super) super_field: u32,
}
}
fn foo(a: inner::A) {
a.<|>
}
",
),
@r###"
[
CompletionItem {
label: "crate_field",
source_range: [313; 313),
delete: [313; 313),
insert: "crate_field",
kind: Field,
detail: "u32",
},
CompletionItem {
label: "pub_field",
source_range: [313; 313),
delete: [313; 313),
insert: "pub_field",
kind: Field,
detail: "u32",
},
CompletionItem {
label: "super_field",
source_range: [313; 313),
delete: [313; 313),
insert: "super_field",
kind: Field,
detail: "u32",
},
]
"###
);
}
#[test]
fn test_method_completion() {
assert_debug_snapshot!(

View file

@ -17,7 +17,9 @@ use crate::{
pub use self::{
expr_extensions::{ArrayExprKind, BinOp, ElseBranch, LiteralKind, PrefixOp, RangeOp},
extensions::{FieldKind, PathSegmentKind, SelfParamKind, StructKind, TypeBoundKind},
extensions::{
FieldKind, PathSegmentKind, SelfParamKind, StructKind, TypeBoundKind, VisibilityKind,
},
generated::*,
tokens::*,
traits::*,

View file

@ -413,3 +413,32 @@ impl ast::TraitDef {
self.syntax().children_with_tokens().any(|t| t.kind() == T![auto])
}
}
pub enum VisibilityKind {
In(ast::Path),
PubCrate,
PubSuper,
Pub,
}
impl ast::Visibility {
pub fn kind(&self) -> VisibilityKind {
if let Some(path) = children(self).next() {
VisibilityKind::In(path)
} else if self.is_pub_crate() {
VisibilityKind::PubCrate
} else if self.is_pub_super() {
VisibilityKind::PubSuper
} else {
VisibilityKind::Pub
}
}
fn is_pub_crate(&self) -> bool {
self.syntax().children_with_tokens().any(|it| it.kind() == T![crate])
}
fn is_pub_super(&self) -> bool {
self.syntax().children_with_tokens().any(|it| it.kind() == T![super])
}
}

View file

@ -1064,6 +1064,7 @@ impl AstNode for ExternCrateItem {
}
}
impl ast::AttrsOwner for ExternCrateItem {}
impl ast::VisibilityOwner for ExternCrateItem {}
impl ExternCrateItem {
pub fn name_ref(&self) -> Option<NameRef> {
AstChildren::new(&self.syntax).next()
@ -2006,6 +2007,7 @@ impl AstNode for ModuleItem {
}
}
impl ast::AttrsOwner for ModuleItem {}
impl ast::VisibilityOwner for ModuleItem {}
impl ModuleItem {}
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Name {
@ -3893,6 +3895,7 @@ impl AstNode for UseItem {
}
}
impl ast::AttrsOwner for UseItem {}
impl ast::VisibilityOwner for UseItem {}
impl UseItem {
pub fn use_tree(&self) -> Option<UseTree> {
AstChildren::new(&self.syntax).next()

View file

@ -412,7 +412,7 @@ Grammar(
"ModuleItem": (
enum: ["StructDef", "UnionDef", "EnumDef", "FnDef", "TraitDef", "TypeAliasDef", "ImplBlock",
"UseItem", "ExternCrateItem", "ConstDef", "StaticDef", "Module" ],
traits: ["AttrsOwner"],
traits: ["AttrsOwner", "VisibilityOwner"],
),
"ImplItem": (
enum: ["FnDef", "TypeAliasDef", "ConstDef"],
@ -683,7 +683,7 @@ Grammar(
]
),
"UseItem": (
traits: ["AttrsOwner"],
traits: ["AttrsOwner", "VisibilityOwner"],
options: [ "UseTree" ],
),
"UseTree": (
@ -696,7 +696,7 @@ Grammar(
collections: [("use_trees", "UseTree")]
),
"ExternCrateItem": (
traits: ["AttrsOwner"],
traits: ["AttrsOwner", "VisibilityOwner"],
options: ["NameRef", "Alias"],
),
"ArgList": (

View file

@ -43,7 +43,7 @@ fn no_todo() {
return;
}
let text = std::fs::read_to_string(e.path()).unwrap();
if text.contains("TODO") || text.contains("TOOD") {
if text.contains("TODO") || text.contains("TOOD") || text.contains("todo!") {
panic!(
"\nTODO markers should not be committed to the master branch,\n\
use FIXME instead\n\