Merge pull request #19034 from ChayimFriedman2/complete-hidden-variant

fix: Don't complete doc(hidden) enum variants and use trees

Author: Veykril

Diff for: crates/ide-completion/src/completions.rs

@@ -188,9 +188,6 @@ impl Completions {
         resolution: hir::ScopeDef,
         doc_aliases: Vec<syntax::SmolStr>,
     ) {
-        if !ctx.check_stability(resolution.attrs(ctx.db).as_deref()) {
-            return;
-        }
         let is_private_editable = match ctx.def_is_visible(&resolution) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -216,9 +213,6 @@ impl Completions {
         local_name: hir::Name,
         resolution: hir::ScopeDef,
     ) {
-        if !ctx.check_stability(resolution.attrs(ctx.db).as_deref()) {
-            return;
-        }
         let is_private_editable = match ctx.def_is_visible(&resolution) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -241,7 +235,7 @@ impl Completions {
         path_ctx: &PathCompletionCtx,
         e: hir::Enum,
     ) {
-        if !ctx.check_stability(Some(&e.attrs(ctx.db))) {
+        if !ctx.check_stability_and_hidden(e) {
             return;
         }
         e.variants(ctx.db)
@@ -257,9 +251,6 @@ impl Completions {
         local_name: hir::Name,
         doc_aliases: Vec<syntax::SmolStr>,
     ) {
-        if !ctx.check_stability(Some(&module.attrs(ctx.db))) {
-            return;
-        }
         self.add_path_resolution(
             ctx,
             path_ctx,
@@ -276,9 +267,6 @@ impl Completions {
         mac: hir::Macro,
         local_name: hir::Name,
     ) {
-        if !ctx.check_stability(Some(&mac.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&mac) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -302,9 +290,6 @@ impl Completions {
         func: hir::Function,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&func.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&func) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -332,9 +317,6 @@ impl Completions {
         receiver: Option<SmolStr>,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&func.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&func) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -362,9 +344,6 @@ impl Completions {
         func: hir::Function,
         import: LocatedImport,
     ) {
-        if !ctx.check_stability(Some(&func.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&func) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -387,9 +366,6 @@ impl Completions {
     }
 
     pub(crate) fn add_const(&mut self, ctx: &CompletionContext<'_>, konst: hir::Const) {
-        if !ctx.check_stability(Some(&konst.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&konst) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -406,9 +382,6 @@ impl Completions {
         ctx: &CompletionContext<'_>,
         type_alias: hir::TypeAlias,
     ) {
-        if !ctx.check_stability(Some(&type_alias.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&type_alias) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -438,7 +411,7 @@ impl Completions {
         variant: hir::Variant,
         path: hir::ModPath,
     ) {
-        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+        if !ctx.check_stability_and_hidden(variant) {
             return;
         }
         if let Some(builder) =
@@ -455,7 +428,7 @@ impl Completions {
         variant: hir::Variant,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+        if !ctx.check_stability_and_hidden(variant) {
             return;
         }
         if let PathCompletionCtx { kind: PathKind::Pat { pat_ctx }, .. } = path_ctx {
@@ -479,9 +452,6 @@ impl Completions {
         field: hir::Field,
         ty: &hir::Type,
     ) {
-        if !ctx.check_stability(Some(&field.attrs(ctx.db))) {
-            return;
-        }
         let is_private_editable = match ctx.is_visible(&field) {
             Visible::Yes => false,
             Visible::Editable => true,
@@ -506,12 +476,18 @@ impl Completions {
         path: Option<hir::ModPath>,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&strukt.attrs(ctx.db))) {
-            return;
-        }
-        if let Some(builder) =
-            render_struct_literal(RenderContext::new(ctx), path_ctx, strukt, path, local_name)
-        {
+        let is_private_editable = match ctx.is_visible(&strukt) {
+            Visible::Yes => false,
+            Visible::Editable => true,
+            Visible::No => return,
+        };
+        if let Some(builder) = render_struct_literal(
+            RenderContext::new(ctx).private_editable(is_private_editable),
+            path_ctx,
+            strukt,
+            path,
+            local_name,
+        ) {
             self.add(builder.build(ctx.db));
         }
     }
@@ -523,10 +499,17 @@ impl Completions {
         path: Option<hir::ModPath>,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&un.attrs(ctx.db))) {
-            return;
-        }
-        let item = render_union_literal(RenderContext::new(ctx), un, path, local_name);
+        let is_private_editable = match ctx.is_visible(&un) {
+            Visible::Yes => false,
+            Visible::Editable => true,
+            Visible::No => return,
+        };
+        let item = render_union_literal(
+            RenderContext::new(ctx).private_editable(is_private_editable),
+            un,
+            path,
+            local_name,
+        );
         self.add_opt(item);
     }
 
@@ -571,7 +554,7 @@ impl Completions {
         variant: hir::Variant,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+        if !ctx.check_stability_and_hidden(variant) {
             return;
         }
         self.add_opt(render_variant_pat(
@@ -591,7 +574,7 @@ impl Completions {
         variant: hir::Variant,
         path: hir::ModPath,
     ) {
-        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+        if !ctx.check_stability_and_hidden(variant) {
             return;
         }
         let path = Some(&path);
@@ -612,10 +595,17 @@ impl Completions {
         strukt: hir::Struct,
         local_name: Option<hir::Name>,
     ) {
-        if !ctx.check_stability(Some(&strukt.attrs(ctx.db))) {
-            return;
-        }
-        self.add_opt(render_struct_pat(RenderContext::new(ctx), pattern_ctx, strukt, local_name));
+        let is_private_editable = match ctx.is_visible(&strukt) {
+            Visible::Yes => false,
+            Visible::Editable => true,
+            Visible::No => return,
+        };
+        self.add_opt(render_struct_pat(
+            RenderContext::new(ctx).private_editable(is_private_editable),
+            pattern_ctx,
+            strukt,
+            local_name,
+        ));
     }
 
     pub(crate) fn suggest_name(&mut self, ctx: &CompletionContext<'_>, name: &str) {

Diff for: crates/ide-completion/src/completions/item_list/trait_impl.rs

@@ -31,7 +31,7 @@
 //! }
 //! ```
 
-use hir::{db::ExpandDatabase, HasAttrs, MacroFileId, Name};
+use hir::{db::ExpandDatabase, MacroFileId, Name};
 use ide_db::text_edit::TextEdit;
 use ide_db::{
     documentation::HasDocs, path_transform::PathTransform,
@@ -155,7 +155,7 @@ fn complete_trait_impl(
     if let Some(hir_impl) = ctx.sema.to_def(impl_def) {
         get_missing_assoc_items(&ctx.sema, impl_def)
             .into_iter()
-            .filter(|item| ctx.check_stability(Some(&item.attrs(ctx.db))))
+            .filter(|item| ctx.check_stability_and_hidden(*item))
             .for_each(|item| {
                 use self::ImplCompletionKind::*;
                 match (item, kind) {

Diff for: crates/ide-completion/src/completions/use_.rs

@@ -52,8 +52,14 @@ pub(crate) fn complete_use_path(
                         )
                     };
                     for (name, def) in module_scope {
-                        if !ctx.check_stability(def.attrs(ctx.db).as_deref()) {
-                            continue;
+                        if let (Some(attrs), Some(defining_crate)) =
+                            (def.attrs(ctx.db), def.krate(ctx.db))
+                        {
+                            if !ctx.check_stability(Some(&attrs))
+                                || ctx.is_doc_hidden(&attrs, defining_crate)
+                            {
+                                continue;
+                            }
                         }
                         let is_name_already_imported =
                             already_imported_names.contains(name.as_str());

Diff for: crates/ide-completion/src/context.rs

@@ -534,7 +534,7 @@ impl CompletionContext<'_> {
         }
     }
 
-    /// Checks if an item is visible and not `doc(hidden)` at the completion site.
+    /// Checks if an item is visible, not `doc(hidden)` and stable at the completion site.
     pub(crate) fn is_visible<I>(&self, item: &I) -> Visible
     where
         I: hir::HasVisibility + hir::HasAttrs + hir::HasCrate + Copy,
@@ -570,6 +570,15 @@ impl CompletionContext<'_> {
         !attrs.is_unstable() || self.is_nightly
     }
 
+    pub(crate) fn check_stability_and_hidden<I>(&self, item: I) -> bool
+    where
+        I: hir::HasAttrs + hir::HasCrate,
+    {
+        let defining_crate = item.krate(self.db);
+        let attrs = item.attrs(self.db);
+        self.check_stability(Some(&attrs)) && !self.is_doc_hidden(&attrs, defining_crate)
+    }
+
     /// Whether the given trait is an operator trait or not.
     pub(crate) fn is_ops_trait(&self, trait_: hir::Trait) -> bool {
         match trait_.attrs(self.db).lang() {
@@ -647,6 +656,10 @@ impl CompletionContext<'_> {
         attrs: &hir::Attrs,
         defining_crate: hir::Crate,
     ) -> Visible {
+        if !self.check_stability(Some(attrs)) {
+            return Visible::No;
+        }
+
         if !vis.is_visible_from(self.db, self.module.into()) {
             if !self.config.enable_private_editable {
                 return Visible::No;
@@ -666,7 +679,7 @@ impl CompletionContext<'_> {
         }
     }
 
-    fn is_doc_hidden(&self, attrs: &hir::Attrs, defining_crate: hir::Crate) -> bool {
+    pub(crate) fn is_doc_hidden(&self, attrs: &hir::Attrs, defining_crate: hir::Crate) -> bool {
         // `doc(hidden)` items are only completed within the defining crate.
         self.krate != defining_crate && attrs.has_doc_hidden()
     }

Diff for: crates/ide-completion/src/tests/expression.rs

@@ -1965,3 +1965,24 @@ fn bar() {
         "#]],
     );
 }
+
+#[test]
+fn doc_hidden_enum_variant() {
+    check(
+        r#"
+//- /foo.rs crate:foo
+pub enum Enum {
+    #[doc(hidden)] Hidden,
+    Visible,
+}
+
+//- /lib.rs crate:lib deps:foo
+fn foo() {
+    let _ = foo::Enum::$0;
+}
+    "#,
+        expect![[r#"
+            ev Visible Visible
+        "#]],
+    );
+}

Diff for: crates/ide-completion/src/tests/use_tree.rs

@@ -451,3 +451,20 @@ marco_rules! m { () => {} }
         "#]],
     );
 }
+
+#[test]
+fn use_tree_doc_hidden() {
+    check(
+        r#"
+//- /foo.rs crate:foo
+#[doc(hidden)] pub struct Hidden;
+pub struct Visible;
+
+//- /lib.rs crate:lib deps:foo
+use foo::$0;
+    "#,
+        expect![[r#"
+            st Visible Visible
+        "#]],
+    );
+}