3707: Add ItemScope::visibility_of r=matklad a=edwin0cheng

~This PR implements `HasVisibility` for various constructs and change `Definition::search_scope` to use `Visibility` directly instead of depends on ad-hoc string parsing.~

This PR added `visibility_of`  in `ItemScope` and `Module` and use it directly directly instead of depends on ad-hoc string parsing.

And also add a FIXME to indicate that there is a bug which do not search child-submodules in other files recursively in `Definition::search_scope`.

I will submit another PR to fix that bug after this is merged.

cc @flodiebold 

Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
This commit is contained in:
bors[bot] 2020-03-25 08:09:26 +00:00 committed by GitHub
commit 8a73a8937d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 50 additions and 59 deletions

View file

@ -234,6 +234,10 @@ impl Module {
.collect()
}
pub fn visibility_of(self, db: &dyn HirDatabase, def: &ModuleDef) -> Option<Visibility> {
db.crate_def_map(self.id.krate)[self.id.local_id].scope.visbility_of(def.clone().into())
}
pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) {
let _p = profile("Module::diagnostics");
let crate_def_map = db.crate_def_map(self.id.krate);

View file

@ -54,7 +54,7 @@ pub use crate::{
Adt, AsAssocItem, AssocItem, AssocItemContainer, AttrDef, Const, Crate, CrateDependency,
DefWithBody, Docs, Enum, EnumVariant, FieldSource, Function, GenericDef, HasAttrs,
HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, ScopeDef, Static, Struct,
StructField, Trait, Type, TypeAlias, TypeParam, Union, VariantDef,
StructField, Trait, Type, TypeAlias, TypeParam, Union, VariantDef, Visibility,
},
has_source::HasSource,
semantics::{original_range, PathResolution, Semantics, SemanticsScope},

View file

@ -68,6 +68,12 @@ impl ItemScope {
self.impls.iter().copied()
}
pub fn visbility_of(&self, def: ModuleDefId) -> Option<Visibility> {
self.name_of(ItemInNs::Types(def))
.or_else(|| self.name_of(ItemInNs::Values(def)))
.map(|(_, v)| v)
}
/// Iterate over all module scoped macros
pub(crate) fn macros<'a>(&'a self) -> impl Iterator<Item = (&'a Name, MacroDefId)> + 'a {
self.visible.iter().filter_map(|(name, def)| def.take_macros().map(|macro_| (name, macro_)))

View file

@ -6,12 +6,12 @@
// FIXME: this badly needs rename/rewrite (matklad, 2020-02-06).
use hir::{
Adt, FieldSource, HasSource, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution,
Semantics, StructField, TypeParam,
HasVisibility, ImplDef, Local, MacroDef, Module, ModuleDef, Name, PathResolution, Semantics,
StructField, TypeParam, Visibility,
};
use ra_prof::profile;
use ra_syntax::{
ast::{self, AstNode, VisibilityOwner},
ast::{self, AstNode},
match_ast,
};
use test_utils::tested_by;
@ -41,28 +41,13 @@ impl Definition {
}
}
pub fn visibility(&self, db: &RootDatabase) -> Option<ast::Visibility> {
pub fn visibility(&self, db: &RootDatabase) -> Option<Visibility> {
let module = self.module(db);
match self {
Definition::Macro(_) => None,
Definition::StructField(sf) => match sf.source(db).value {
FieldSource::Named(it) => it.visibility(),
FieldSource::Pos(it) => it.visibility(),
},
Definition::ModuleDef(def) => match def {
ModuleDef::Module(it) => it.declaration_source(db)?.value.visibility(),
ModuleDef::Function(it) => it.source(db).value.visibility(),
ModuleDef::Adt(adt) => match adt {
Adt::Struct(it) => it.source(db).value.visibility(),
Adt::Union(it) => it.source(db).value.visibility(),
Adt::Enum(it) => it.source(db).value.visibility(),
},
ModuleDef::Const(it) => it.source(db).value.visibility(),
ModuleDef::Static(it) => it.source(db).value.visibility(),
ModuleDef::Trait(it) => it.source(db).value.visibility(),
ModuleDef::TypeAlias(it) => it.source(db).value.visibility(),
ModuleDef::EnumVariant(_) => None,
ModuleDef::BuiltinType(_) => None,
},
Definition::StructField(sf) => Some(sf.visibility(db)),
Definition::ModuleDef(def) => module?.visibility_of(db, def),
Definition::SelfType(_) => None,
Definition::Local(_) => None,
Definition::TypeParam(_) => None,

View file

@ -6,7 +6,7 @@
use std::mem;
use hir::{DefWithBody, HasSource, ModuleSource, Semantics};
use hir::{DefWithBody, HasSource, Module, ModuleSource, Semantics, Visibility};
use once_cell::unsync::Lazy;
use ra_db::{FileId, FileRange, SourceDatabaseExt};
use ra_prof::profile;
@ -123,22 +123,26 @@ impl Definition {
return SearchScope::new(res);
}
let vis = self.visibility(db).as_ref().map(|v| v.syntax().to_string()).unwrap_or_default();
let vis = self.visibility(db);
if vis.as_str() == "pub(super)" {
if let Some(parent_module) = module.parent(db) {
// FIXME:
// The following logic are wrong that it does not search
// for submodules within other files recursively.
if let Some(Visibility::Module(module)) = vis.and_then(|it| it.into()) {
let module: Module = module.into();
let mut res = FxHashMap::default();
let parent_src = parent_module.definition_source(db);
let file_id = parent_src.file_id.original_file(db);
let src = module.definition_source(db);
let file_id = src.file_id.original_file(db);
match parent_src.value {
match src.value {
ModuleSource::Module(m) => {
let range = Some(m.syntax().text_range());
res.insert(file_id, range);
}
ModuleSource::SourceFile(_) => {
res.insert(file_id, None);
res.extend(parent_module.children(db).map(|m| {
res.extend(module.children(db).map(|m| {
let src = m.definition_source(db);
(src.file_id.original_file(db), None)
}));
@ -146,19 +150,12 @@ impl Definition {
}
return SearchScope::new(res);
}
}
if vis.as_str() != "" {
if let Some(Visibility::Public) = vis {
let source_root_id = db.file_source_root(file_id);
let source_root = db.source_root(source_root_id);
let mut res = source_root.walk().map(|id| (id, None)).collect::<FxHashMap<_, _>>();
// FIXME: add "pub(in path)"
if vis.as_str() == "pub(crate)" {
return SearchScope::new(res);
}
if vis.as_str() == "pub" {
let krate = module.krate();
for rev_dep in krate.reverse_dependencies(db) {
let root_file = rev_dep.root_file(db);
@ -168,7 +165,6 @@ impl Definition {
}
return SearchScope::new(res);
}
}
let mut res = FxHashMap::default();
let range = match module_src.value {