From b0f7aac72f0ca2f973ce1ad7bd3004ef8e61a3b4 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 23 Jul 2021 15:36:43 +0200 Subject: [PATCH 1/2] Respect `#[doc(hidden)]` in dot-completion --- crates/hir/src/attrs.rs | 35 +++++++++++++++++-- crates/hir/src/lib.rs | 29 +++++++++++++++ crates/hir_def/src/attr.rs | 9 ++++- crates/ide_completion/src/completions/dot.rs | 35 ++++++++++++++++--- .../src/completions/qualified_path.rs | 10 +++--- crates/ide_completion/src/context.rs | 31 ++++++++++++++++ 6 files changed, 136 insertions(+), 13 deletions(-) diff --git a/crates/hir/src/attrs.rs b/crates/hir/src/attrs.rs index 859dcfc1c791..9332cfc06749 100644 --- a/crates/hir/src/attrs.rs +++ b/crates/hir/src/attrs.rs @@ -11,8 +11,8 @@ use hir_ty::db::HirDatabase; use syntax::ast; use crate::{ - Adt, Const, ConstParam, Enum, Field, Function, GenericParam, Impl, LifetimeParam, MacroDef, - Module, ModuleDef, Static, Struct, Trait, TypeAlias, TypeParam, Union, Variant, + Adt, AssocItem, Const, ConstParam, Enum, Field, Function, GenericParam, Impl, LifetimeParam, + MacroDef, Module, ModuleDef, Static, Struct, Trait, TypeAlias, TypeParam, Union, Variant, }; pub trait HasAttrs { @@ -86,6 +86,37 @@ macro_rules! impl_has_attrs_enum { impl_has_attrs_enum![Struct, Union, Enum for Adt]; impl_has_attrs_enum![TypeParam, ConstParam, LifetimeParam for GenericParam]; +impl HasAttrs for AssocItem { + fn attrs(self, db: &dyn HirDatabase) -> AttrsWithOwner { + match self { + AssocItem::Function(it) => it.attrs(db), + AssocItem::Const(it) => it.attrs(db), + AssocItem::TypeAlias(it) => it.attrs(db), + } + } + + fn docs(self, db: &dyn HirDatabase) -> Option { + match self { + AssocItem::Function(it) => it.docs(db), + AssocItem::Const(it) => it.docs(db), + AssocItem::TypeAlias(it) => it.docs(db), + } + } + + fn resolve_doc_path( + self, + db: &dyn HirDatabase, + link: &str, + ns: Option, + ) -> Option { + match self { + AssocItem::Function(it) => it.resolve_doc_path(db, link, ns), + AssocItem::Const(it) => it.resolve_doc_path(db, link, ns), + AssocItem::TypeAlias(it) => it.resolve_doc_path(db, link, ns), + } + } +} + fn resolve_doc_path( db: &dyn HirDatabase, def: AttrDefId, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 79c67bfc2c47..9673885e7394 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -2744,3 +2744,32 @@ pub trait HasVisibility { vis.is_visible_from(db.upcast(), module.id) } } + +/// Trait for obtaining the defining crate of an item. +pub trait HasCrate { + fn krate(&self, db: &dyn HirDatabase) -> Crate; +} + +impl HasCrate for T { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db.upcast()).krate().into() + } +} + +impl HasCrate for AssocItem { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db).krate() + } +} + +impl HasCrate for Field { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.parent_def(db).module(db).krate() + } +} + +impl HasCrate for Function { + fn krate(&self, db: &dyn HirDatabase) -> Crate { + self.module(db).krate() + } +} diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 741a4266d24a..4471cdd8bc18 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -12,7 +12,7 @@ use either::Either; use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile}; use itertools::Itertools; use la_arena::ArenaMap; -use mbe::ast_to_token_tree; +use mbe::{ast_to_token_tree, DelimiterKind}; use smallvec::{smallvec, SmallVec}; use syntax::{ ast::{self, AstNode, AttrsOwner}, @@ -290,6 +290,13 @@ impl Attrs { Some(Documentation(buf)) } } + + pub fn has_doc_hidden(&self) -> bool { + self.by_key("doc").tt_values().find(|tt| { + tt.delimiter_kind() == Some(DelimiterKind::Parenthesis) && + matches!(&*tt.token_trees, [tt::TokenTree::Leaf(tt::Leaf::Ident(ident))] if ident.text == "hidden") + }).is_some() + } } impl AttrsWithOwner { diff --git a/crates/ide_completion/src/completions/dot.rs b/crates/ide_completion/src/completions/dot.rs index 67ffafa6c2bc..3494dfd30650 100644 --- a/crates/ide_completion/src/completions/dot.rs +++ b/crates/ide_completion/src/completions/dot.rs @@ -1,7 +1,7 @@ //! Completes references after dot (fields and method calls). use either::Either; -use hir::{HasVisibility, ScopeDef}; +use hir::ScopeDef; use rustc_hash::FxHashSet; use crate::{context::CompletionContext, patterns::ImmediateLocation, Completions}; @@ -63,9 +63,7 @@ fn complete_fields( ) { for receiver in receiver.autoderef(ctx.db) { for (field, ty) in receiver.fields(ctx.db) { - if ctx.scope.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 + if !ctx.is_visible(&field) { continue; } f(Either::Left(field), ty); @@ -87,7 +85,7 @@ fn complete_methods( let traits_in_scope = ctx.scope.traits_in_scope(); receiver.iterate_method_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, func| { if func.self_param(ctx.db).is_some() - && ctx.scope.module().map_or(true, |m| func.is_visible_from(ctx.db, m)) + && ctx.is_visible(&func) && seen_methods.insert(func.name(ctx.db)) { f(func); @@ -210,6 +208,33 @@ fn foo(a: A) { a.$0 } ); } + #[test] + fn test_doc_hidden_filtering() { + check( + r#" +//- /lib.rs crate:lib deps:dep +fn foo(a: dep::A) { a.$0 } +//- /dep.rs crate:dep +pub struct A { + #[doc(hidden)] + pub hidden_field: u32, + pub pub_field: u32, +} + +impl A { + pub fn pub_method(&self) {} + + #[doc(hidden)] + pub fn hidden_method(&self) {} +} + "#, + expect![[r#" + fd pub_field u32 + me pub_method() fn(&self) + "#]] + ) + } + #[test] fn test_union_field_completion() { check( diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs index 03b079ed7a0b..608398584dff 100644 --- a/crates/ide_completion/src/completions/qualified_path.rs +++ b/crates/ide_completion/src/completions/qualified_path.rs @@ -2,7 +2,6 @@ use std::iter; -use hir::HasVisibility; use rustc_hash::FxHashSet; use syntax::{ast, AstNode}; @@ -120,6 +119,7 @@ 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); } @@ -163,7 +163,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon if let Some(krate) = krate { let traits_in_scope = ctx.scope.traits_in_scope(); ty.iterate_path_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, item| { - if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + if !ctx.is_visible(&item) { return None; } add_assoc_item(acc, ctx, item); @@ -172,7 +172,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon // Iterate assoc types separately ty.iterate_assoc_items(ctx.db, krate, |item| { - if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + if !ctx.is_visible(&item) { return None; } if let hir::AssocItem::TypeAlias(ty) = item { @@ -185,7 +185,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon hir::PathResolution::Def(hir::ModuleDef::Trait(t)) => { // Handles `Trait::assoc` as well as `::assoc`. for item in t.items(ctx.db) { - if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + if !ctx.is_visible(&item) { continue; } add_assoc_item(acc, ctx, item); @@ -206,7 +206,7 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon let traits_in_scope = ctx.scope.traits_in_scope(); let mut seen = FxHashSet::default(); ty.iterate_path_candidates(ctx.db, krate, &traits_in_scope, None, |_ty, item| { - if context_module.map_or(false, |m| !item.is_visible_from(ctx.db, m)) { + if !ctx.is_visible(&item) { return None; } diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs index 061c082ab89b..a056b5405a52 100644 --- a/crates/ide_completion/src/context.rs +++ b/crates/ide_completion/src/context.rs @@ -361,6 +361,37 @@ impl<'a> CompletionContext<'a> { self.path_context.as_ref().and_then(|it| it.qualifier.as_ref()) } + /// Checks if an item is visible and not `doc(hidden)` at the completion site. + pub(crate) fn is_visible(&self, item: &I) -> bool + where + I: hir::HasVisibility + hir::HasAttrs + hir::HasCrate + Copy, + { + self.is_visible_impl(&item.visibility(self.db), &item.attrs(self.db), item.krate(self.db)) + } + + fn is_visible_impl( + &self, + vis: &hir::Visibility, + attrs: &hir::Attrs, + defining_crate: hir::Crate, + ) -> bool { + let module = match self.scope.module() { + Some(it) => it, + None => return false, + }; + if !vis.is_visible_from(self.db, module.into()) { + // FIXME: if the definition location is editable, also show private items + return false; + } + + if module.krate() != defining_crate && attrs.has_doc_hidden() { + // `doc(hidden)` items are only completed within the defining crate. + return false; + } + + true + } + fn fill_impl_def(&mut self) { self.impl_def = self .sema From c8d915e2ea704ab349600b81bdad18e75d67cbd6 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 23 Jul 2021 16:45:14 +0200 Subject: [PATCH 2/2] Fix formatting and use `Iterator::any` --- crates/hir_def/src/attr.rs | 4 ++-- crates/ide_completion/src/completions/dot.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs index 4471cdd8bc18..e79b301c8c3b 100644 --- a/crates/hir_def/src/attr.rs +++ b/crates/hir_def/src/attr.rs @@ -292,10 +292,10 @@ impl Attrs { } pub fn has_doc_hidden(&self) -> bool { - self.by_key("doc").tt_values().find(|tt| { + self.by_key("doc").tt_values().any(|tt| { tt.delimiter_kind() == Some(DelimiterKind::Parenthesis) && matches!(&*tt.token_trees, [tt::TokenTree::Leaf(tt::Leaf::Ident(ident))] if ident.text == "hidden") - }).is_some() + }) } } diff --git a/crates/ide_completion/src/completions/dot.rs b/crates/ide_completion/src/completions/dot.rs index 3494dfd30650..97be0f0e4fa0 100644 --- a/crates/ide_completion/src/completions/dot.rs +++ b/crates/ide_completion/src/completions/dot.rs @@ -231,7 +231,7 @@ impl A { expect![[r#" fd pub_field u32 me pub_method() fn(&self) - "#]] + "#]], ) }