From 0327df224bf67ef2c3ceb5a69729cf36a6a96ba1 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 30 May 2022 16:01:17 +0200 Subject: [PATCH] More precise completion filtering --- .../src/completions/item_list.rs | 14 +-- .../ide-completion/src/completions/keyword.rs | 4 +- .../ide-completion/src/completions/snippet.rs | 5 +- crates/ide-completion/src/context.rs | 112 ++++++++++++++++-- crates/ide-completion/src/tests/item.rs | 16 --- 5 files changed, 115 insertions(+), 36 deletions(-) diff --git a/crates/ide-completion/src/completions/item_list.rs b/crates/ide-completion/src/completions/item_list.rs index ebbc33c2da0..edff146d8d7 100644 --- a/crates/ide-completion/src/completions/item_list.rs +++ b/crates/ide-completion/src/completions/item_list.rs @@ -8,21 +8,18 @@ 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 => {} } } diff --git a/crates/ide-completion/src/completions/keyword.rs b/crates/ide-completion/src/completions/keyword.rs index 5917da9b7f3..281e6e9783c 100644 --- a/crates/ide-completion/src/completions/keyword.rs +++ b/crates/ide-completion/src/completions/keyword.rs @@ -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"); } diff --git a/crates/ide-completion/src/completions/snippet.rs b/crates/ide-completion/src/completions/snippet.rs index 2bae19c84fd..ebc3bb5a6f9 100644 --- a/crates/ide-completion/src/completions/snippet.rs +++ b/crates/ide-completion/src/completions/snippet.rs @@ -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. } diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs index 5e2118ae14b..b70d6f9e8dc 100644 --- a/crates/ide-completion/src/context.rs +++ b/crates/ide-completion/src/context.rs @@ -75,6 +75,18 @@ pub(super) enum ItemListKind { ExternBlock, } +#[derive(Debug, Default)] +pub(super) struct QualifierCtx { + pub(super) unsafe_tok: Option, + pub(super) vis_node: Option, +} + +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, + pub(super) qualifier_ctx: QualifierCtx, pub(super) existing_derives: FxHashSet, @@ -363,17 +376,13 @@ pub(crate) fn has_impl_prev_sibling(&self) -> bool { 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 @@ pub(super) fn new( // 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 @@ fn fill( 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 @@ fn fill( 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 @@ fn classify_name_ref( } }; + // 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> 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 @@ fn classify_name_ref( 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()) { diff --git a/crates/ide-completion/src/tests/item.rs b/crates/ide-completion/src/tests/item.rs index e4b70f97e31..537c9a7fa24 100644 --- a/crates/ide-completion/src/tests/item.rs +++ b/crates/ide-completion/src/tests/item.rs @@ -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) "#]], ); } -- 2.44.0