Skip to content

Commit fd27621

Browse files
borsbors
committed
Auto merge of #14549 - lowr:patch/no-unstable-item-compl-on-stable, r=Veykril
Don't suggest unstable items on stable toolchain Closes #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.
2 parents 600283f + e6e4872 commit fd27621

File tree

21 files changed

+602
-80
lines changed

21 files changed

+602
-80
lines changed

‎crates/base-db/src/fixture.rs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,16 @@ use std::{mem, str::FromStr, sync::Arc};
44
use cfg::CfgOptions;
55
use rustc_hash::FxHashMap;
66
use test_utils::{
7-
extract_range_or_offset, Fixture, RangeOrOffset, CURSOR_MARKER, ESCAPED_CURSOR_MARKER,
7+
extract_range_or_offset, Fixture, FixtureWithProjectMeta, RangeOrOffset, CURSOR_MARKER,
8+
ESCAPED_CURSOR_MARKER,
89
};
910
use tt::token_id::{Leaf, Subtree, TokenTree};
1011
use vfs::{file_set::FileSet, VfsPath};
1112

1213
use crate::{
1314
input::{CrateName, CrateOrigin, LangCrateOrigin},
1415
Change, CrateDisplayName, CrateGraph, CrateId, Dependency, Edition, Env, FileId, FilePosition,
15-
FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacros,
16+
FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacros, ReleaseChannel,
1617
SourceDatabaseExt, SourceRoot, SourceRootId,
1718
};
1819

@@ -102,7 +103,14 @@ impl ChangeFixture {
102103
ra_fixture: &str,
103104
mut proc_macro_defs: Vec<(String, ProcMacro)>,
104105
) -> ChangeFixture {
105-
let (mini_core, proc_macro_names, fixture) = Fixture::parse(ra_fixture);
106+
let FixtureWithProjectMeta { fixture, mini_core, proc_macro_names, toolchain } =
107+
FixtureWithProjectMeta::parse(ra_fixture);
108+
let toolchain = toolchain
109+
.map(|it| {
110+
ReleaseChannel::from_str(&it)
111+
.unwrap_or_else(|| panic!("unknown release channel found: {it}"))
112+
})
113+
.unwrap_or(ReleaseChannel::Stable);
106114
let mut change = Change::new();
107115

108116
let mut files = Vec::new();
@@ -166,7 +174,7 @@ impl ChangeFixture {
166174
.as_deref()
167175
.map(Arc::from)
168176
.ok_or_else(|| "target_data_layout unset".into()),
169-
None,
177+
Some(toolchain),
170178
);
171179
let prev = crates.insert(crate_name.clone(), crate_id);
172180
assert!(prev.is_none());
@@ -205,7 +213,7 @@ impl ChangeFixture {
205213
default_target_data_layout
206214
.map(|x| x.into())
207215
.ok_or_else(|| "target_data_layout unset".into()),
208-
None,
216+
Some(toolchain),
209217
);
210218
} else {
211219
for (from, to, prelude) in crate_deps {
@@ -247,7 +255,7 @@ impl ChangeFixture {
247255
false,
248256
CrateOrigin::Lang(LangCrateOrigin::Core),
249257
target_layout.clone(),
250-
None,
258+
Some(toolchain),
251259
);
252260

253261
for krate in all_crates {
@@ -286,7 +294,7 @@ impl ChangeFixture {
286294
true,
287295
CrateOrigin::Local { repo: None, name: None },
288296
target_layout,
289-
None,
297+
Some(toolchain),
290298
);
291299
proc_macros.insert(proc_macros_crate, Ok(proc_macro));
292300

‎crates/hir-def/src/attr.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,10 @@ impl Attrs {
269269
pub fn is_proc_macro_derive(&self) -> bool {
270270
self.by_key("proc_macro_derive").exists()
271271
}
272+
273+
pub fn is_unstable(&self) -> bool {
274+
self.by_key("unstable").exists()
275+
}
272276
}
273277

274278
use std::slice::Iter as SliceIter;

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

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ pub(crate) mod env_vars;
2323

2424
use std::iter;
2525

26-
use hir::{known, ScopeDef, Variant};
26+
use hir::{known, HasAttrs, ScopeDef, Variant};
2727
use ide_db::{imports::import_assets::LocatedImport, SymbolKind};
2828
use syntax::ast;
2929

@@ -181,6 +181,9 @@ impl Completions {
181181
resolution: hir::ScopeDef,
182182
doc_aliases: Vec<syntax::SmolStr>,
183183
) {
184+
if !ctx.check_stability(resolution.attrs(ctx.db).as_deref()) {
185+
return;
186+
}
184187
let is_private_editable = match ctx.def_is_visible(&resolution) {
185188
Visible::Yes => false,
186189
Visible::Editable => true,
@@ -206,6 +209,9 @@ impl Completions {
206209
local_name: hir::Name,
207210
resolution: hir::ScopeDef,
208211
) {
212+
if !ctx.check_stability(resolution.attrs(ctx.db).as_deref()) {
213+
return;
214+
}
209215
let is_private_editable = match ctx.def_is_visible(&resolution) {
210216
Visible::Yes => false,
211217
Visible::Editable => true,
@@ -228,6 +234,9 @@ impl Completions {
228234
path_ctx: &PathCompletionCtx,
229235
e: hir::Enum,
230236
) {
237+
if !ctx.check_stability(Some(&e.attrs(ctx.db))) {
238+
return;
239+
}
231240
e.variants(ctx.db)
232241
.into_iter()
233242
.for_each(|variant| self.add_enum_variant(ctx, path_ctx, variant, None));
@@ -241,6 +250,9 @@ impl Completions {
241250
local_name: hir::Name,
242251
doc_aliases: Vec<syntax::SmolStr>,
243252
) {
253+
if !ctx.check_stability(Some(&module.attrs(ctx.db))) {
254+
return;
255+
}
244256
self.add_path_resolution(
245257
ctx,
246258
path_ctx,
@@ -257,6 +269,9 @@ impl Completions {
257269
mac: hir::Macro,
258270
local_name: hir::Name,
259271
) {
272+
if !ctx.check_stability(Some(&mac.attrs(ctx.db))) {
273+
return;
274+
}
260275
let is_private_editable = match ctx.is_visible(&mac) {
261276
Visible::Yes => false,
262277
Visible::Editable => true,
@@ -280,6 +295,9 @@ impl Completions {
280295
func: hir::Function,
281296
local_name: Option<hir::Name>,
282297
) {
298+
if !ctx.check_stability(Some(&func.attrs(ctx.db))) {
299+
return;
300+
}
283301
let is_private_editable = match ctx.is_visible(&func) {
284302
Visible::Yes => false,
285303
Visible::Editable => true,
@@ -304,6 +322,9 @@ impl Completions {
304322
receiver: Option<hir::Name>,
305323
local_name: Option<hir::Name>,
306324
) {
325+
if !ctx.check_stability(Some(&func.attrs(ctx.db))) {
326+
return;
327+
}
307328
let is_private_editable = match ctx.is_visible(&func) {
308329
Visible::Yes => false,
309330
Visible::Editable => true,
@@ -328,6 +349,9 @@ impl Completions {
328349
func: hir::Function,
329350
import: LocatedImport,
330351
) {
352+
if !ctx.check_stability(Some(&func.attrs(ctx.db))) {
353+
return;
354+
}
331355
let is_private_editable = match ctx.is_visible(&func) {
332356
Visible::Yes => false,
333357
Visible::Editable => true,
@@ -348,6 +372,9 @@ impl Completions {
348372
}
349373

350374
pub(crate) fn add_const(&mut self, ctx: &CompletionContext<'_>, konst: hir::Const) {
375+
if !ctx.check_stability(Some(&konst.attrs(ctx.db))) {
376+
return;
377+
}
351378
let is_private_editable = match ctx.is_visible(&konst) {
352379
Visible::Yes => false,
353380
Visible::Editable => true,
@@ -364,6 +391,9 @@ impl Completions {
364391
ctx: &CompletionContext<'_>,
365392
type_alias: hir::TypeAlias,
366393
) {
394+
if !ctx.check_stability(Some(&type_alias.attrs(ctx.db))) {
395+
return;
396+
}
367397
let is_private_editable = match ctx.is_visible(&type_alias) {
368398
Visible::Yes => false,
369399
Visible::Editable => true,
@@ -380,6 +410,9 @@ impl Completions {
380410
ctx: &CompletionContext<'_>,
381411
type_alias: hir::TypeAlias,
382412
) {
413+
if !ctx.check_stability(Some(&type_alias.attrs(ctx.db))) {
414+
return;
415+
}
383416
self.add_opt(render_type_alias_with_eq(RenderContext::new(ctx), type_alias));
384417
}
385418

@@ -390,6 +423,9 @@ impl Completions {
390423
variant: hir::Variant,
391424
path: hir::ModPath,
392425
) {
426+
if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
427+
return;
428+
}
393429
if let Some(builder) =
394430
render_variant_lit(RenderContext::new(ctx), path_ctx, None, variant, Some(path))
395431
{
@@ -404,6 +440,9 @@ impl Completions {
404440
variant: hir::Variant,
405441
local_name: Option<hir::Name>,
406442
) {
443+
if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
444+
return;
445+
}
407446
if let PathCompletionCtx { kind: PathKind::Pat { pat_ctx }, .. } = path_ctx {
408447
cov_mark::hit!(enum_variant_pattern_path);
409448
self.add_variant_pat(ctx, pat_ctx, Some(path_ctx), variant, local_name);
@@ -425,6 +464,9 @@ impl Completions {
425464
field: hir::Field,
426465
ty: &hir::Type,
427466
) {
467+
if !ctx.check_stability(Some(&field.attrs(ctx.db))) {
468+
return;
469+
}
428470
let is_private_editable = match ctx.is_visible(&field) {
429471
Visible::Yes => false,
430472
Visible::Editable => true,
@@ -448,6 +490,9 @@ impl Completions {
448490
path: Option<hir::ModPath>,
449491
local_name: Option<hir::Name>,
450492
) {
493+
if !ctx.check_stability(Some(&strukt.attrs(ctx.db))) {
494+
return;
495+
}
451496
if let Some(builder) =
452497
render_struct_literal(RenderContext::new(ctx), path_ctx, strukt, path, local_name)
453498
{
@@ -462,6 +507,9 @@ impl Completions {
462507
path: Option<hir::ModPath>,
463508
local_name: Option<hir::Name>,
464509
) {
510+
if !ctx.check_stability(Some(&un.attrs(ctx.db))) {
511+
return;
512+
}
465513
let item = render_union_literal(RenderContext::new(ctx), un, path, local_name);
466514
self.add_opt(item);
467515
}
@@ -473,6 +521,8 @@ impl Completions {
473521
field: usize,
474522
ty: &hir::Type,
475523
) {
524+
// Only used for (unnamed) tuples, whose all fields *are* stable. No need to check
525+
// stability here.
476526
let item = render_tuple_field(RenderContext::new(ctx), receiver, field, ty);
477527
self.add(item);
478528
}
@@ -494,6 +544,9 @@ impl Completions {
494544
variant: hir::Variant,
495545
local_name: Option<hir::Name>,
496546
) {
547+
if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
548+
return;
549+
}
497550
self.add_opt(render_variant_pat(
498551
RenderContext::new(ctx),
499552
pattern_ctx,
@@ -511,6 +564,9 @@ impl Completions {
511564
variant: hir::Variant,
512565
path: hir::ModPath,
513566
) {
567+
if !ctx.check_stability(Some(&variant.attrs(ctx.db))) {
568+
return;
569+
}
514570
let path = Some(&path);
515571
self.add_opt(render_variant_pat(
516572
RenderContext::new(ctx),
@@ -529,6 +585,9 @@ impl Completions {
529585
strukt: hir::Struct,
530586
local_name: Option<hir::Name>,
531587
) {
588+
if !ctx.check_stability(Some(&strukt.attrs(ctx.db))) {
589+
return;
590+
}
532591
self.add_opt(render_struct_pat(RenderContext::new(ctx), pattern_ctx, strukt, local_name));
533592
}
534593
}

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,43 @@ fn foo(s: S) { s.$0 }
172172
);
173173
}
174174

175+
#[test]
176+
fn no_unstable_method_on_stable() {
177+
check(
178+
r#"
179+
//- /main.rs crate:main deps:std
180+
fn foo(s: std::S) { s.$0 }
181+
//- /std.rs crate:std
182+
pub struct S;
183+
impl S {
184+
#[unstable]
185+
pub fn bar(&self) {}
186+
}
187+
"#,
188+
expect![""],
189+
);
190+
}
191+
192+
#[test]
193+
fn unstable_method_on_nightly() {
194+
check(
195+
r#"
196+
//- toolchain:nightly
197+
//- /main.rs crate:main deps:std
198+
fn foo(s: std::S) { s.$0 }
199+
//- /std.rs crate:std
200+
pub struct S;
201+
impl S {
202+
#[unstable]
203+
pub fn bar(&self) {}
204+
}
205+
"#,
206+
expect![[r#"
207+
me bar() fn(&self)
208+
"#]],
209+
);
210+
}
211+
175212
#[test]
176213
fn test_struct_field_completion_self() {
177214
check(

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,10 @@ fn import_on_the_fly(
267267
.into_iter()
268268
.filter(ns_filter)
269269
.filter(|import| {
270-
!ctx.is_item_hidden(&import.item_to_import)
271-
&& !ctx.is_item_hidden(&import.original_item)
270+
let item = &import.item_to_import;
271+
!ctx.is_item_hidden(item)
272+
&& !ctx.is_item_hidden(item)
273+
&& ctx.check_stability(item.attrs(ctx.db).as_deref())
272274
})
273275
.sorted_by_key(|located_import| {
274276
compute_fuzzy_completion_order_key(
@@ -315,8 +317,10 @@ fn import_on_the_fly_pat_(
315317
.into_iter()
316318
.filter(ns_filter)
317319
.filter(|import| {
318-
!ctx.is_item_hidden(&import.item_to_import)
319-
&& !ctx.is_item_hidden(&import.original_item)
320+
let item = &import.item_to_import;
321+
!ctx.is_item_hidden(item)
322+
&& !ctx.is_item_hidden(item)
323+
&& ctx.check_stability(item.attrs(ctx.db).as_deref())
320324
})
321325
.sorted_by_key(|located_import| {
322326
compute_fuzzy_completion_order_key(

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

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -150,21 +150,24 @@ fn complete_trait_impl(
150150
impl_def: &ast::Impl,
151151
) {
152152
if let Some(hir_impl) = ctx.sema.to_def(impl_def) {
153-
get_missing_assoc_items(&ctx.sema, impl_def).into_iter().for_each(|item| {
154-
use self::ImplCompletionKind::*;
155-
match (item, kind) {
156-
(hir::AssocItem::Function(func), All | Fn) => {
157-
add_function_impl(acc, ctx, replacement_range, func, hir_impl)
153+
get_missing_assoc_items(&ctx.sema, impl_def)
154+
.into_iter()
155+
.filter(|item| ctx.check_stability(Some(&item.attrs(ctx.db))))
156+
.for_each(|item| {
157+
use self::ImplCompletionKind::*;
158+
match (item, kind) {
159+
(hir::AssocItem::Function(func), All | Fn) => {
160+
add_function_impl(acc, ctx, replacement_range, func, hir_impl)
161+
}
162+
(hir::AssocItem::TypeAlias(type_alias), All | TypeAlias) => {
163+
add_type_alias_impl(acc, ctx, replacement_range, type_alias, hir_impl)
164+
}
165+
(hir::AssocItem::Const(const_), All | Const) => {
166+
add_const_impl(acc, ctx, replacement_range, const_, hir_impl)
167+
}
168+
_ => {}
158169
}
159-
(hir::AssocItem::TypeAlias(type_alias), All | TypeAlias) => {
160-
add_type_alias_impl(acc, ctx, replacement_range, type_alias, hir_impl)
161-
}
162-
(hir::AssocItem::Const(const_), All | Const) => {
163-
add_const_impl(acc, ctx, replacement_range, const_, hir_impl)
164-
}
165-
_ => {}
166-
}
167-
});
170+
});
168171
}
169172
}
170173

0 commit comments

Comments
 (0)