Merge #9681

9681: Respect `#[doc(hidden)]` in dot-completion r=jonas-schievink a=jonas-schievink

This adds `CompletionContext::is_visible` as a convenience method that checks visibility, presence of `doc(hidden)`, and whether the completed item is in the same crate as the completion site or not. We only complete `doc(hidden)` items from the same crate.

This doesn't yet work for *all* completions: `qualified_path` completions use `Module::scope` and `ScopeDef`, which doesn't work with this.

Part of #7718

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

Author: bors[bot]

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<Documentation> {
+        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<Namespace>,
+    ) -> Option<ModuleDef> {
+        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,

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<T: hir_def::HasModule> 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()
+    }
+}

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().any(|tt| {
+            tt.delimiter_kind() == Some(DelimiterKind::Parenthesis) &&
+                matches!(&*tt.token_trees, [tt::TokenTree::Leaf(tt::Leaf::Ident(ident))] if ident.text == "hidden")
+        })
+    }
 }
 
 impl AttrsWithOwner {

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(

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 `<Ty as Trait>::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;
                     }
 

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<I>(&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