Skip to content

feat: Make most completions respect #[doc(hidden)] #9715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,20 @@ impl ModuleDef {
}
acc
}

pub fn attrs(&self, db: &dyn HirDatabase) -> Option<AttrsWithOwner> {
Some(match self {
ModuleDef::Module(it) => it.attrs(db),
ModuleDef::Function(it) => it.attrs(db),
ModuleDef::Adt(it) => it.attrs(db),
ModuleDef::Variant(it) => it.attrs(db),
ModuleDef::Const(it) => it.attrs(db),
ModuleDef::Static(it) => it.attrs(db),
ModuleDef::Trait(it) => it.attrs(db),
ModuleDef::TypeAlias(it) => it.attrs(db),
ModuleDef::BuiltinType(_) => return None,
})
}
}

impl HasVisibility for ModuleDef {
Expand Down Expand Up @@ -2725,6 +2739,32 @@ impl ScopeDef {

items
}

pub fn attrs(&self, db: &dyn HirDatabase) -> Option<AttrsWithOwner> {
match self {
ScopeDef::ModuleDef(it) => it.attrs(db),
ScopeDef::MacroDef(it) => Some(it.attrs(db)),
ScopeDef::GenericParam(it) => Some(it.attrs(db)),
ScopeDef::ImplSelfType(_)
| ScopeDef::AdtSelfType(_)
| ScopeDef::Local(_)
| ScopeDef::Label(_)
| ScopeDef::Unknown => None,
}
}

pub fn krate(&self, db: &dyn HirDatabase) -> Option<Crate> {
match self {
ScopeDef::ModuleDef(it) => it.module(db).map(|m| m.krate()),
ScopeDef::MacroDef(it) => it.module(db).map(|m| m.krate()),
ScopeDef::GenericParam(it) => Some(it.module(db).krate()),
ScopeDef::ImplSelfType(_) => None,
ScopeDef::AdtSelfType(it) => Some(it.module(db).krate()),
ScopeDef::Local(it) => Some(it.module(db).krate()),
ScopeDef::Label(it) => Some(it.module(db).krate()),
ScopeDef::Unknown => None,
}
}
}

impl From<ItemInNs> for ScopeDef {
Expand Down
2 changes: 1 addition & 1 deletion crates/ide_completion/src/completions/attribute/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn get_derive_names_in_scope(
ctx: &CompletionContext,
) -> FxHashMap<String, Option<hir::Documentation>> {
let mut result = FxHashMap::default();
ctx.scope.process_all_names(&mut |name, scope_def| {
ctx.process_all_names(&mut |name, scope_def| {
if let hir::ScopeDef::MacroDef(mac) = scope_def {
if mac.kind() == hir::MacroKind::Derive {
result.insert(name.to_string(), mac.docs(ctx.db));
Expand Down
2 changes: 1 addition & 1 deletion crates/ide_completion/src/completions/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) {

// FIXME: ideally, we should look at the type we are matching against and
// suggest variants + auto-imports
ctx.scope.process_all_names(&mut |name, res| {
ctx.process_all_names(&mut |name, res| {
let add_resolution = match &res {
hir::ScopeDef::ModuleDef(def) => match def {
hir::ModuleDef::Adt(hir::Adt::Struct(strukt)) => {
Expand Down
36 changes: 35 additions & 1 deletion crates/ide_completion/src/completions/qualified_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
}
}

if ctx.is_scope_def_hidden(&def) {
cov_mark::hit!(qualified_path_doc_hidden);
continue;
}

let add_resolution = match def {
// Don't suggest attribute macros and derives.
hir::ScopeDef::MacroDef(mac) => mac.is_fn_like(),
Expand All @@ -119,7 +124,6 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
_ => true,
};

// FIXME: respect #[doc(hidden)] (see `CompletionContext::is_visible`)
if add_resolution {
acc.add_resolution(ctx, name, &def);
}
Expand Down Expand Up @@ -666,4 +670,34 @@ fn main() {
"#]],
);
}

#[test]
fn respects_doc_hidden() {
cov_mark::check!(qualified_path_doc_hidden);
check(
r#"
//- /lib.rs crate:lib deps:dep
fn f() {
dep::$0
}

//- /dep.rs crate:dep
#[doc(hidden)]
#[macro_export]
macro_rules! m {
() => {}
}

#[doc(hidden)]
pub fn f() {}

#[doc(hidden)]
pub struct S;

#[doc(hidden)]
pub mod m {}
"#,
expect![[r#""#]],
)
}
}
62 changes: 58 additions & 4 deletions crates/ide_completion/src/completions/unqualified_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
if ctx.in_use_tree() {
// only show modules in a fresh UseTree
cov_mark::hit!(unqualified_path_only_modules_in_import);
ctx.scope.process_all_names(&mut |name, res| {
ctx.process_all_names(&mut |name, res| {
if let ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = res {
acc.add_resolution(ctx, name, &res);
}
Expand All @@ -29,7 +29,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
Some(ImmediateLocation::Visibility(_)) => return,
Some(ImmediateLocation::ItemList | ImmediateLocation::Trait | ImmediateLocation::Impl) => {
// only show macros in {Assoc}ItemList
ctx.scope.process_all_names(&mut |name, res| {
ctx.process_all_names(&mut |name, res| {
if let hir::ScopeDef::MacroDef(mac) = res {
if mac.is_fn_like() {
acc.add_macro(ctx, Some(name.clone()), mac);
Expand All @@ -42,7 +42,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
return;
}
Some(ImmediateLocation::TypeBound) => {
ctx.scope.process_all_names(&mut |name, res| {
ctx.process_all_names(&mut |name, res| {
let add_resolution = match res {
ScopeDef::MacroDef(mac) => mac.is_fn_like(),
ScopeDef::ModuleDef(hir::ModuleDef::Trait(_) | hir::ModuleDef::Module(_)) => {
Expand Down Expand Up @@ -83,7 +83,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
}
}

ctx.scope.process_all_names(&mut |name, res| {
ctx.process_all_names(&mut |name, res| {
if let ScopeDef::GenericParam(hir::GenericParam::LifetimeParam(_)) | ScopeDef::Label(_) =
res
{
Expand Down Expand Up @@ -252,4 +252,58 @@ pub mod prelude {
"#]],
);
}

#[test]
fn respects_doc_hidden() {
check(
r#"
//- /lib.rs crate:lib deps:std
fn f() {
format_$0
}

//- /std.rs crate:std
#[doc(hidden)]
#[macro_export]
macro_rules! format_args_nl {
() => {}
}

pub mod prelude {
pub mod rust_2018 {}
}
"#,
expect![[r#"
fn f() fn()
md std
"#]],
);
}

#[test]
fn respects_doc_hidden_in_assoc_item_list() {
check(
r#"
//- /lib.rs crate:lib deps:std
struct S;
impl S {
format_$0
}

//- /std.rs crate:std
#[doc(hidden)]
#[macro_export]
macro_rules! format_args_nl {
() => {}
}

pub mod prelude {
pub mod rust_2018 {}
}
"#,
expect![[r#"
md std
"#]],
);
}
}
33 changes: 30 additions & 3 deletions crates/ide_completion/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! See `CompletionContext` structure.

use base_db::SourceDatabaseExt;
use hir::{Local, ScopeDef, Semantics, SemanticsScope, Type};
use hir::{Local, Name, ScopeDef, Semantics, SemanticsScope, Type};
use ide_db::{
base_db::{FilePosition, SourceDatabase},
call_info::ActiveParameter,
Expand Down Expand Up @@ -370,6 +370,25 @@ impl<'a> CompletionContext<'a> {
self.is_visible_impl(&item.visibility(self.db), &item.attrs(self.db), item.krate(self.db))
}

pub(crate) fn is_scope_def_hidden(&self, scope_def: &ScopeDef) -> bool {
if let (Some(attrs), Some(krate)) = (scope_def.attrs(self.db), scope_def.krate(self.db)) {
return self.is_doc_hidden(&attrs, krate);
}

false
}

/// A version of [`SemanticsScope::process_all_names`] that filters out `#[doc(hidden)]` items.
pub(crate) fn process_all_names(&self, f: &mut dyn FnMut(Name, ScopeDef)) {
self.scope.process_all_names(&mut |name, def| {
if self.is_scope_def_hidden(&def) {
return;
}

f(name, def);
})
}

fn is_visible_impl(
&self,
vis: &hir::Visibility,
Expand All @@ -388,12 +407,20 @@ impl<'a> CompletionContext<'a> {
return is_editable;
}

!self.is_doc_hidden(attrs, defining_crate)
}

fn is_doc_hidden(&self, attrs: &hir::Attrs, defining_crate: hir::Crate) -> bool {
let module = match self.scope.module() {
Some(it) => it,
None => return true,
};
if module.krate() != defining_crate && attrs.has_doc_hidden() {
// `doc(hidden)` items are only completed within the defining crate.
return false;
return true;
}

true
false
}

fn fill_impl_def(&mut self) {
Expand Down