Auto merge of rust-lang#14549 - lowr:patch/no-unstable-item-compl-on-stable, r=Veykril

Don't suggest unstable items on stable toolchain

Closes rust-lang#3020

This PR implements stability check in `ide-completion` so that unstable items are only suggested if you're on nightly toolchain.

It's a bit unfortunate `CompletionContext::check_stability()` is spammed all over the crate, but we should call it before building `CompletionItem` as you cannot get attributes on the item it's completing from that struct. I looked up every callsite of `Builder::add_to()`, `Completions::add[_opt]()`, and`Completions::add_all()` and inserted the check wherever necessary.

The tests are admittedly incomplete in that I didn't add tests for every kind of item as I thought that would be too big and not worthwhile. I copy-pasted some existing basic tests in every test module and adjusted them.

Author: bors

crates/base-db/src/fixture.rs

@@ -4,15 +4,16 @@ use std::{mem, str::FromStr, sync::Arc};
 use cfg::CfgOptions;
 use rustc_hash::FxHashMap;
 use test_utils::{
-    extract_range_or_offset, Fixture, RangeOrOffset, CURSOR_MARKER, ESCAPED_CURSOR_MARKER,
+    extract_range_or_offset, Fixture, FixtureWithProjectMeta, RangeOrOffset, CURSOR_MARKER,
+    ESCAPED_CURSOR_MARKER,
 };
 use tt::token_id::{Leaf, Subtree, TokenTree};
 use vfs::{file_set::FileSet, VfsPath};
 
 use crate::{
     input::{CrateName, CrateOrigin, LangCrateOrigin},
     Change, CrateDisplayName, CrateGraph, CrateId, Dependency, Edition, Env, FileId, FilePosition,
-    FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacros,
+    FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacros, ReleaseChannel,
     SourceDatabaseExt, SourceRoot, SourceRootId,
 };
 
@@ -102,7 +103,14 @@ impl ChangeFixture {
         ra_fixture: &str,
         mut proc_macro_defs: Vec<(String, ProcMacro)>,
     ) -> ChangeFixture {
-        let (mini_core, proc_macro_names, fixture) = Fixture::parse(ra_fixture);
+        let FixtureWithProjectMeta { fixture, mini_core, proc_macro_names, toolchain } =
+            FixtureWithProjectMeta::parse(ra_fixture);
+        let toolchain = toolchain
+            .map(|it| {
+                ReleaseChannel::from_str(&it)
+                    .unwrap_or_else(|| panic!("unknown release channel found: {it}"))
+            })
+            .unwrap_or(ReleaseChannel::Stable);
         let mut change = Change::new();
 
         let mut files = Vec::new();
@@ -166,7 +174,7 @@ impl ChangeFixture {
                         .as_deref()
                         .map(Arc::from)
                         .ok_or_else(|| "target_data_layout unset".into()),
-                    None,
+                    Some(toolchain),
                 );
                 let prev = crates.insert(crate_name.clone(), crate_id);
                 assert!(prev.is_none());
@@ -205,7 +213,7 @@ impl ChangeFixture {
                 default_target_data_layout
                     .map(|x| x.into())
                     .ok_or_else(|| "target_data_layout unset".into()),
-                None,
+                Some(toolchain),
             );
         } else {
             for (from, to, prelude) in crate_deps {
@@ -247,7 +255,7 @@ impl ChangeFixture {
                 false,
                 CrateOrigin::Lang(LangCrateOrigin::Core),
                 target_layout.clone(),
-                None,
+                Some(toolchain),
             );
 
             for krate in all_crates {
@@ -286,7 +294,7 @@ impl ChangeFixture {
                 true,
                 CrateOrigin::Local { repo: None, name: None },
                 target_layout,
-                None,
+                Some(toolchain),
             );
             proc_macros.insert(proc_macros_crate, Ok(proc_macro));
 

crates/hir-def/src/attr.rs

@@ -269,6 +269,10 @@ impl Attrs {
     pub fn is_proc_macro_derive(&self) -> bool {
         self.by_key("proc_macro_derive").exists()
     }
+
+    pub fn is_unstable(&self) -> bool {
+        self.by_key("unstable").exists()
+    }
 }
 
 use std::slice::Iter as SliceIter;

crates/ide-completion/src/completions.rs

@@ -23,7 +23,7 @@ pub(crate) mod env_vars;
 
 use std::iter;
 
-use hir::{known, ScopeDef, Variant};
+use hir::{known, HasAttrs, ScopeDef, Variant};
 use ide_db::{imports::import_assets::LocatedImport, SymbolKind};
 use syntax::ast;
 
@@ -181,6 +181,9 @@ 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,
@@ -206,6 +209,9 @@ 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,
@@ -228,6 +234,9 @@ impl Completions {
         path_ctx: &PathCompletionCtx,
         e: hir::Enum,
     ) {
+        if !ctx.check_stability(Some(&e.attrs(ctx.db))) {
+            return;
+        }
         e.variants(ctx.db)
             .into_iter()
             .for_each(|variant| self.add_enum_variant(ctx, path_ctx, variant, None));
@@ -241,6 +250,9 @@ 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,
@@ -257,6 +269,9 @@ 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,
@@ -280,6 +295,9 @@ 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,
@@ -304,6 +322,9 @@ impl Completions {
         receiver: Option<hir::Name>,
         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,
@@ -328,6 +349,9 @@ 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,
@@ -348,6 +372,9 @@ 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,
@@ -364,6 +391,9 @@ 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,
@@ -380,6 +410,9 @@ impl Completions {
         ctx: &CompletionContext<'_>,
         type_alias: hir::TypeAlias,
     ) {
+        if !ctx.check_stability(Some(&type_alias.attrs(ctx.db))) {
+            return;
+        }
         self.add_opt(render_type_alias_with_eq(RenderContext::new(ctx), type_alias));
     }
 
@@ -390,6 +423,9 @@ impl Completions {
         variant: hir::Variant,
         path: hir::ModPath,
     ) {
+        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+            return;
+        }
         if let Some(builder) =
             render_variant_lit(RenderContext::new(ctx), path_ctx, None, variant, Some(path))
         {
@@ -404,6 +440,9 @@ impl Completions {
         variant: hir::Variant,
         local_name: Option<hir::Name>,
     ) {
+        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+            return;
+        }
         if let PathCompletionCtx { kind: PathKind::Pat { pat_ctx }, .. } = path_ctx {
             cov_mark::hit!(enum_variant_pattern_path);
             self.add_variant_pat(ctx, pat_ctx, Some(path_ctx), variant, local_name);
@@ -425,6 +464,9 @@ 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,
@@ -448,6 +490,9 @@ 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)
         {
@@ -462,6 +507,9 @@ 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);
         self.add_opt(item);
     }
@@ -473,6 +521,8 @@ impl Completions {
         field: usize,
         ty: &hir::Type,
     ) {
+        // Only used for (unnamed) tuples, whose all fields *are* stable. No need to check
+        // stability here.
         let item = render_tuple_field(RenderContext::new(ctx), receiver, field, ty);
         self.add(item);
     }
@@ -494,6 +544,9 @@ impl Completions {
         variant: hir::Variant,
         local_name: Option<hir::Name>,
     ) {
+        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+            return;
+        }
         self.add_opt(render_variant_pat(
             RenderContext::new(ctx),
             pattern_ctx,
@@ -511,6 +564,9 @@ impl Completions {
         variant: hir::Variant,
         path: hir::ModPath,
     ) {
+        if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
+            return;
+        }
         let path = Some(&path);
         self.add_opt(render_variant_pat(
             RenderContext::new(ctx),
@@ -529,6 +585,9 @@ 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));
     }
 }

crates/ide-completion/src/completions/dot.rs

@@ -172,6 +172,43 @@ fn foo(s: S) { s.$0 }
         );
     }
 
+    #[test]
+    fn no_unstable_method_on_stable() {
+        check(
+            r#"
+//- /main.rs crate:main deps:std
+fn foo(s: std::S) { s.$0 }
+//- /std.rs crate:std
+pub struct S;
+impl S {
+    #[unstable]
+    pub fn bar(&self) {}
+}
+"#,
+            expect![""],
+        );
+    }
+
+    #[test]
+    fn unstable_method_on_nightly() {
+        check(
+            r#"
+//- toolchain:nightly
+//- /main.rs crate:main deps:std
+fn foo(s: std::S) { s.$0 }
+//- /std.rs crate:std
+pub struct S;
+impl S {
+    #[unstable]
+    pub fn bar(&self) {}
+}
+"#,
+            expect![[r#"
+                me bar() fn(&self)
+            "#]],
+        );
+    }
+
     #[test]
     fn test_struct_field_completion_self() {
         check(

crates/ide-completion/src/completions/flyimport.rs

@@ -267,8 +267,10 @@ fn import_on_the_fly(
             .into_iter()
             .filter(ns_filter)
             .filter(|import| {
-                !ctx.is_item_hidden(&import.item_to_import)
-                    && !ctx.is_item_hidden(&import.original_item)
+                let item = &import.item_to_import;
+                !ctx.is_item_hidden(item)
+                    && !ctx.is_item_hidden(item)
+                    && ctx.check_stability(item.attrs(ctx.db).as_deref())
             })
             .sorted_by_key(|located_import| {
                 compute_fuzzy_completion_order_key(
@@ -315,8 +317,10 @@ fn import_on_the_fly_pat_(
             .into_iter()
             .filter(ns_filter)
             .filter(|import| {
-                !ctx.is_item_hidden(&import.item_to_import)
-                    && !ctx.is_item_hidden(&import.original_item)
+                let item = &import.item_to_import;
+                !ctx.is_item_hidden(item)
+                    && !ctx.is_item_hidden(item)
+                    && ctx.check_stability(item.attrs(ctx.db).as_deref())
             })
             .sorted_by_key(|located_import| {
                 compute_fuzzy_completion_order_key(

crates/ide-completion/src/completions/item_list/trait_impl.rs

@@ -150,21 +150,24 @@ fn complete_trait_impl(
     impl_def: &ast::Impl,
 ) {
     if let Some(hir_impl) = ctx.sema.to_def(impl_def) {
-        get_missing_assoc_items(&ctx.sema, impl_def).into_iter().for_each(|item| {
-            use self::ImplCompletionKind::*;
-            match (item, kind) {
-                (hir::AssocItem::Function(func), All | Fn) => {
-                    add_function_impl(acc, ctx, replacement_range, func, hir_impl)
+        get_missing_assoc_items(&ctx.sema, impl_def)
+            .into_iter()
+            .filter(|item| ctx.check_stability(Some(&item.attrs(ctx.db))))
+            .for_each(|item| {
+                use self::ImplCompletionKind::*;
+                match (item, kind) {
+                    (hir::AssocItem::Function(func), All | Fn) => {
+                        add_function_impl(acc, ctx, replacement_range, func, hir_impl)
+                    }
+                    (hir::AssocItem::TypeAlias(type_alias), All | TypeAlias) => {
+                        add_type_alias_impl(acc, ctx, replacement_range, type_alias, hir_impl)
+                    }
+                    (hir::AssocItem::Const(const_), All | Const) => {
+                        add_const_impl(acc, ctx, replacement_range, const_, hir_impl)
+                    }
+                    _ => {}
                 }
-                (hir::AssocItem::TypeAlias(type_alias), All | TypeAlias) => {
-                    add_type_alias_impl(acc, ctx, replacement_range, type_alias, hir_impl)
-                }
-                (hir::AssocItem::Const(const_), All | Const) => {
-                    add_const_impl(acc, ctx, replacement_range, const_, hir_impl)
-                }
-                _ => {}
-            }
-        });
+            });
     }
 }