Auto merge of #12418 - Veykril:completions, r=Veykril

internal: More precise completion filtering with existing item qualifiers

Now we are approaching the more complex cases for filtering completions
This commit is contained in:
bors 2022-05-30 14:05:37 +00:00
commit a5d7ab54f9
11 changed files with 131 additions and 51 deletions

View File

@ -8,21 +8,18 @@ use crate::{
pub(crate) fn complete_item_list(acc: &mut Completions, ctx: &CompletionContext) {
let _p = profile::span("complete_item_list");
if ctx.is_path_disallowed() || ctx.has_unfinished_impl_or_trait_prev_sibling() {
return;
}
let (&is_absolute_path, qualifier) = match ctx.path_context() {
let (&is_absolute_path, path_qualifier, _kind) = match ctx.path_context() {
Some(PathCompletionCtx {
kind: PathKind::Item { .. },
kind: PathKind::Item { kind },
is_absolute_path,
qualifier,
..
}) => (is_absolute_path, qualifier),
}) => (is_absolute_path, qualifier, kind),
_ => return,
};
match qualifier {
match path_qualifier {
Some(PathQualifierCtx { resolution, is_super_chain, .. }) => {
if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) = resolution {
for (name, def) in module.scope(ctx.db, Some(ctx.module)) {
@ -39,7 +36,7 @@ pub(crate) fn complete_item_list(acc: &mut Completions, ctx: &CompletionContext)
None if is_absolute_path => {
acc.add_crate_roots(ctx);
}
None => {
None if ctx.qualifier_ctx.none() => {
ctx.process_all_names(&mut |name, def| {
if let Some(def) = module_or_fn_macro(ctx.db, def) {
acc.add_resolution(ctx, name, def);
@ -47,5 +44,6 @@ pub(crate) fn complete_item_list(acc: &mut Completions, ctx: &CompletionContext)
});
acc.add_nameref_keywords_with_colon(ctx);
}
None => {}
}
}

View File

@ -51,7 +51,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
return;
}
if !ctx.has_visibility_prev_sibling()
if ctx.qualifier_ctx.vis_node.is_none()
&& (expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_field())
{
add_keyword("pub(crate)", "pub(crate)");
@ -67,7 +67,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
}
if expects_item || has_block_expr_parent {
if !ctx.has_visibility_prev_sibling() {
if ctx.qualifier_ctx.vis_node.is_none() {
add_keyword("impl", "impl $1 {\n $0\n}");
add_keyword("extern", "extern $0");
}

View File

@ -2,7 +2,6 @@
use hir::Documentation;
use ide_db::{imports::insert_use::ImportScope, SnippetCap};
use syntax::T;
use crate::{
context::{ItemListKind, PathCompletionCtx, PathKind},
@ -52,10 +51,10 @@ pub(crate) fn complete_item_snippet(acc: &mut Completions, ctx: &CompletionConte
}) => kind,
_ => return,
};
if ctx.previous_token_is(T![unsafe]) || ctx.has_unfinished_impl_or_trait_prev_sibling() {
if !ctx.qualifier_ctx.none() {
return;
}
if ctx.has_visibility_prev_sibling() {
if ctx.qualifier_ctx.vis_node.is_some() {
return; // technically we could do some of these snippet completions if we were to put the
// attributes before the vis node.
}

View File

@ -75,6 +75,18 @@ pub(super) enum ItemListKind {
ExternBlock,
}
#[derive(Debug, Default)]
pub(super) struct QualifierCtx {
pub(super) unsafe_tok: Option<SyntaxToken>,
pub(super) vis_node: Option<ast::Visibility>,
}
impl QualifierCtx {
pub(super) fn none(&self) -> bool {
self.unsafe_tok.is_none() && self.vis_node.is_none()
}
}
#[derive(Debug)]
pub(crate) struct PathCompletionCtx {
/// If this is a call with () already there (or {} in case of record patterns)
@ -253,6 +265,7 @@ pub(crate) struct CompletionContext<'a> {
pub(super) ident_ctx: IdentContext,
pub(super) pattern_ctx: Option<PatternContext>,
pub(super) qualifier_ctx: QualifierCtx,
pub(super) existing_derives: FxHashSet<hir::Macro>,
@ -363,17 +376,13 @@ impl<'a> CompletionContext<'a> {
matches!(self.prev_sibling, Some(ImmediatePrevSibling::ImplDefType))
}
pub(crate) fn has_visibility_prev_sibling(&self) -> bool {
matches!(self.prev_sibling, Some(ImmediatePrevSibling::Visibility))
}
pub(crate) fn after_if(&self) -> bool {
matches!(self.prev_sibling, Some(ImmediatePrevSibling::IfExpr))
}
// FIXME: This shouldn't exist
pub(crate) fn is_path_disallowed(&self) -> bool {
self.previous_token_is(T![unsafe])
!self.qualifier_ctx.none()
|| matches!(self.prev_sibling, Some(ImmediatePrevSibling::Visibility))
|| (matches!(self.name_ctx(), Some(NameContext { .. })) && self.pattern_ctx.is_none())
|| matches!(self.pattern_ctx, Some(PatternContext { record_pat: Some(_), .. }))
@ -555,6 +564,7 @@ impl<'a> CompletionContext<'a> {
// dummy value, will be overwritten
ident_ctx: IdentContext::UnexpandedAttrTT { fake_attribute_under_caret: None },
pattern_ctx: None,
qualifier_ctx: Default::default(),
existing_derives: Default::default(),
locals,
};
@ -865,7 +875,7 @@ impl<'a> CompletionContext<'a> {
offset: TextSize,
derive_ctx: Option<(SyntaxNode, SyntaxNode, TextSize, ast::Attr)>,
) -> Option<()> {
let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap();
let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased()?;
let syntax_element = NodeOrToken::Token(fake_ident_token);
if is_in_token_of_for_loop(syntax_element.clone()) {
// for pat $0
@ -967,7 +977,49 @@ impl<'a> CompletionContext<'a> {
ast::NameLike::NameRef(name_ref) => {
let parent = name_ref.syntax().parent()?;
let (nameref_ctx, pat_ctx) =
Self::classify_name_ref(&self.sema, &original_file, name_ref, parent);
Self::classify_name_ref(&self.sema, &original_file, name_ref, parent.clone());
// Extract qualifiers
if let Some(path_ctx) = &nameref_ctx.path_ctx {
if path_ctx.qualifier.is_none() {
let top = match path_ctx.kind {
PathKind::Expr { in_block_expr: true, .. } => parent
.ancestors()
.find(|it| ast::PathExpr::can_cast(it.kind()))
.and_then(|p| {
let parent = p.parent()?;
if ast::StmtList::can_cast(parent.kind()) {
Some(p)
} else if ast::ExprStmt::can_cast(parent.kind()) {
Some(parent)
} else {
None
}
}),
PathKind::Item { .. } => {
parent.ancestors().find(|it| ast::MacroCall::can_cast(it.kind()))
}
_ => None,
};
if let Some(top) = top {
if let Some(NodeOrToken::Node(error_node)) =
syntax::algo::non_trivia_sibling(
top.into(),
syntax::Direction::Prev,
)
{
if error_node.kind() == SyntaxKind::ERROR {
self.qualifier_ctx.unsafe_tok = error_node
.children_with_tokens()
.filter_map(NodeOrToken::into_token)
.find(|it| it.kind() == T![unsafe]);
self.qualifier_ctx.vis_node =
error_node.children().find_map(ast::Visibility::cast);
}
}
}
}
}
self.ident_ctx = IdentContext::NameRef(nameref_ctx);
self.pattern_ctx = pat_ctx;
}
@ -1145,12 +1197,54 @@ impl<'a> CompletionContext<'a> {
}
};
// We do not want to generate path completions when we are sandwiched between an item decl signature and its body.
// ex. trait Foo $0 {}
// in these cases parser recovery usually kicks in for our inserted identifier, causing it
// to either be parsed as an ExprStmt or a MacroCall, depending on whether it is in a block
// expression or an item list.
// The following code checks if the body is missing, if it is we either cut off the body
// from the item or it was missing in the first place
let inbetween_body_and_decl_check = |node: SyntaxNode| {
if let Some(NodeOrToken::Node(n)) =
syntax::algo::non_trivia_sibling(node.into(), syntax::Direction::Prev)
{
if let Some(item) = ast::Item::cast(n) {
match item {
ast::Item::Const(it) => it.body().is_none(),
ast::Item::Enum(it) => it.variant_list().is_none(),
ast::Item::ExternBlock(it) => it.extern_item_list().is_none(),
ast::Item::Fn(it) => it.body().is_none(),
ast::Item::Impl(it) => it.assoc_item_list().is_none(),
ast::Item::Module(it) => it.item_list().is_none(),
ast::Item::Static(it) => it.body().is_none(),
ast::Item::Struct(it) => it.field_list().is_none(),
ast::Item::Trait(it) => it.assoc_item_list().is_none(),
ast::Item::TypeAlias(it) => it.ty().is_none(),
ast::Item::Union(it) => it.record_field_list().is_none(),
_ => false,
}
} else {
false
}
} else {
false
}
};
let kind = path.syntax().ancestors().find_map(|it| {
// using Option<Option<PathKind>> as extra controlflow
let kind = match_ast! {
match it {
ast::PathType(_) => Some(PathKind::Type),
ast::PathExpr(it) => {
if let Some(p) = it.syntax().parent() {
if ast::ExprStmt::can_cast(p.kind()) {
if inbetween_body_and_decl_check(p) {
return Some(None);
}
}
}
fill_record_expr(it.syntax());
path_ctx.has_call_parens = it.syntax().parent().map_or(false, |it| ast::CallExpr::can_cast(it.kind()));
@ -1173,6 +1267,10 @@ impl<'a> CompletionContext<'a> {
Some(PathKind::Pat)
},
ast::MacroCall(it) => {
if inbetween_body_and_decl_check(it.syntax().clone()) {
return Some(None);
}
path_ctx.has_macro_bang = it.excl_token().is_some();
let parent = it.syntax().parent();
match parent.as_ref().map(|it| it.kind()) {

View File

@ -143,7 +143,7 @@ pub fn completions(
db: &RootDatabase,
config: &CompletionConfig,
position: FilePosition,
trigger_character: Option<&str>,
trigger_character: Option<char>,
) -> Option<Completions> {
let ctx = &CompletionContext::new(db, position, config)?;
let mut acc = Completions::default();
@ -151,7 +151,7 @@ pub fn completions(
{
let acc = &mut acc;
// prevent `(` from triggering unwanted completion noise
if trigger_character != Some("(") {
if trigger_character != Some('(') {
completions::attribute::complete_attribute(acc, ctx);
completions::attribute::complete_derive(acc, ctx);
completions::attribute::complete_known_attribute_input(acc, ctx);

View File

@ -88,7 +88,7 @@ pub(crate) fn completion_list_no_kw(ra_fixture: &str) -> String {
pub(crate) fn completion_list_with_trigger_character(
ra_fixture: &str,
trigger_character: Option<&str>,
trigger_character: Option<char>,
) -> String {
completion_list_with_config(TEST_CONFIG, ra_fixture, true, trigger_character)
}
@ -97,7 +97,7 @@ fn completion_list_with_config(
config: CompletionConfig,
ra_fixture: &str,
include_keywords: bool,
trigger_character: Option<&str>,
trigger_character: Option<char>,
) -> String {
// filter out all but one builtintype completion for smaller test outputs
let items = get_all_items(config, ra_fixture, trigger_character);
@ -225,7 +225,7 @@ pub(crate) fn check_pattern_is_applicable(code: &str, check: impl FnOnce(SyntaxE
pub(crate) fn get_all_items(
config: CompletionConfig,
code: &str,
trigger_character: Option<&str>,
trigger_character: Option<char>,
) -> Vec<CompletionItem> {
let (db, position) = position(code);
let res = crate::completions(&db, &config, position, trigger_character)

View File

@ -7,8 +7,8 @@ fn check(ra_fixture: &str, expect: Expect) {
expect.assert_eq(&actual);
}
fn check_with_trigger_character(ra_fixture: &str, trigger_character: Option<&str>, expect: Expect) {
let actual = completion_list_with_trigger_character(ra_fixture, trigger_character);
fn check_with_trigger_character(ra_fixture: &str, trigger_character: char, expect: Expect) {
let actual = completion_list_with_trigger_character(ra_fixture, Some(trigger_character));
expect.assert_eq(&actual)
}
@ -124,7 +124,7 @@ fn trigger_by_l_paren() {
r#"
fn foo($0)
"#,
Some("("),
'(',
expect![[]],
)
}

View File

@ -92,10 +92,7 @@ fn after_struct_name() {
check(
r"struct Struct $0",
expect![[r#"
ma makro!() macro_rules! makro
md module
kw const
kw crate::
kw enum
kw extern
kw fn
@ -104,18 +101,13 @@ fn after_struct_name() {
kw pub
kw pub(crate)
kw pub(super)
kw self::
kw static
kw struct
kw super::
kw trait
kw type
kw union
kw unsafe
kw use
sn macro_rules
sn tfn (Test function)
sn tmod (Test module)
"#]],
);
}
@ -126,10 +118,7 @@ fn after_fn_name() {
check(
r"fn func() $0",
expect![[r#"
ma makro!() macro_rules! makro
md module
kw const
kw crate::
kw enum
kw extern
kw fn
@ -138,18 +127,13 @@ fn after_fn_name() {
kw pub
kw pub(crate)
kw pub(super)
kw self::
kw static
kw struct
kw super::
kw trait
kw type
kw union
kw unsafe
kw use
sn macro_rules
sn tfn (Test function)
sn tmod (Test module)
"#]],
);
}

View File

@ -8,8 +8,8 @@ fn check(ra_fixture: &str, expect: Expect) {
expect.assert_eq(&actual)
}
fn check_with_trigger_character(ra_fixture: &str, trigger_character: Option<&str>, expect: Expect) {
let actual = completion_list_with_trigger_character(ra_fixture, trigger_character);
fn check_with_trigger_character(ra_fixture: &str, trigger_character: char, expect: Expect) {
let actual = completion_list_with_trigger_character(ra_fixture, Some(trigger_character));
expect.assert_eq(&actual)
}
@ -20,7 +20,7 @@ fn empty_pub() {
r#"
pub($0)
"#,
Some("("),
'(',
expect![[r#"
kw crate
kw in

View File

@ -548,7 +548,7 @@ impl Analysis {
&self,
config: &CompletionConfig,
position: FilePosition,
trigger_character: Option<&str>,
trigger_character: Option<char>,
) -> Cancellable<Option<Vec<CompletionItem>>> {
self.with_db(|db| {
ide_completion::completions(db, config, position, trigger_character).map(Into::into)

View File

@ -796,9 +796,10 @@ pub(crate) fn handle_completion(
let _p = profile::span("handle_completion");
let text_document_position = params.text_document_position.clone();
let position = from_proto::file_position(&snap, params.text_document_position)?;
let completion_trigger_character = params.context.and_then(|ctx| ctx.trigger_character);
let completion_trigger_character =
params.context.and_then(|ctx| ctx.trigger_character).and_then(|s| s.chars().next());
if Some(":") == completion_trigger_character.as_deref() {
if Some(':') == completion_trigger_character {
let source_file = snap.analysis.parse(position.file_id)?;
let left_token = source_file.syntax().token_at_offset(position.offset).left_biased();
let completion_triggered_after_single_colon = match left_token {
@ -814,7 +815,7 @@ pub(crate) fn handle_completion(
let items = match snap.analysis.completions(
completion_config,
position,
completion_trigger_character.as_deref(),
completion_trigger_character,
)? {
None => return Ok(None),
Some(items) => items,