Skip to content

feat: Don't suggest unstable items on stable toolchain #14549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 11, 2023
Merged

feat: Don't suggest unstable items on stable toolchain #14549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
584d269
Add `toolchain` meta for tests
lowr Apr 10, 2023
0ce71dd
completion: check stability
lowr Apr 10, 2023
e6e4872
Add tests for stability check in completion
lowr Apr 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions crates/base-db/src/fixture.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -247,7 +255,7 @@ impl ChangeFixture {
false,
CrateOrigin::Lang(LangCrateOrigin::Core),
target_layout.clone(),
None,
Some(toolchain),
);

for krate in all_crates {
Expand Down Expand Up @@ -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));

Expand Down
4 changes: 4 additions & 0 deletions crates/hir-def/src/attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
61 changes: 60 additions & 1 deletion crates/ide-completion/src/completions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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));
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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));
}

Expand All @@ -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))
{
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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)
{
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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,
Expand All @@ -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),
Expand All @@ -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));
}
}
Expand Down
37 changes: 37 additions & 0 deletions crates/ide-completion/src/completions/dot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 8 additions & 4 deletions crates/ide-completion/src/completions/flyimport.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here as well

&& ctx.check_stability(item.attrs(ctx.db).as_deref())
})
.sorted_by_key(|located_import| {
compute_fuzzy_completion_order_key(
Expand Down Expand Up @@ -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)
Copy link
Contributor

@hecatia-elegua hecatia-elegua Apr 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to do !ctx.is_item_hidden(&import.original_item) here?
The preview window doesn't show it but you're doing !ctx.is_item_hidden(item) twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, you're right! Thanks for catching this. Will file another PR shortly!

&& !ctx.is_item_hidden(item)
&& ctx.check_stability(item.attrs(ctx.db).as_deref())
})
.sorted_by_key(|located_import| {
compute_fuzzy_completion_order_key(
Expand Down
31 changes: 17 additions & 14 deletions crates/ide-completion/src/completions/item_list/trait_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
_ => {}
}
});
});
}
}

Expand Down
Loading