Skip to content

Commit a5d7ab5

Browse files
borsbors
committed
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
2 parents c551d1a + 0327df2 commit a5d7ab5

File tree

11 files changed

+131
-51
lines changed

11 files changed

+131
-51
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,18 @@ use crate::{
88

99
pub(crate) fn complete_item_list(acc: &mut Completions, ctx: &CompletionContext) {
1010
let _p = profile::span("complete_item_list");
11-
if ctx.is_path_disallowed() || ctx.has_unfinished_impl_or_trait_prev_sibling() {
12-
return;
13-
}
1411

15-
let (&is_absolute_path, qualifier) = match ctx.path_context() {
12+
let (&is_absolute_path, path_qualifier, _kind) = match ctx.path_context() {
1613
Some(PathCompletionCtx {
17-
kind: PathKind::Item { .. },
14+
kind: PathKind::Item { kind },
1815
is_absolute_path,
1916
qualifier,
2017
..
21-
}) => (is_absolute_path, qualifier),
18+
}) => (is_absolute_path, qualifier, kind),
2219
_ => return,
2320
};
2421

25-
match qualifier {
22+
match path_qualifier {
2623
Some(PathQualifierCtx { resolution, is_super_chain, .. }) => {
2724
if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) = resolution {
2825
for (name, def) in module.scope(ctx.db, Some(ctx.module)) {
@@ -39,13 +36,14 @@ pub(crate) fn complete_item_list(acc: &mut Completions, ctx: &CompletionContext)
3936
None if is_absolute_path => {
4037
acc.add_crate_roots(ctx);
4138
}
42-
None => {
39+
None if ctx.qualifier_ctx.none() => {
4340
ctx.process_all_names(&mut |name, def| {
4441
if let Some(def) = module_or_fn_macro(ctx.db, def) {
4542
acc.add_resolution(ctx, name, def);
4643
}
4744
});
4845
acc.add_nameref_keywords_with_colon(ctx);
4946
}
47+
None => {}
5048
}
5149
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
5151
return;
5252
}
5353

54-
if !ctx.has_visibility_prev_sibling()
54+
if ctx.qualifier_ctx.vis_node.is_none()
5555
&& (expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_field())
5656
{
5757
add_keyword("pub(crate)", "pub(crate)");
@@ -67,7 +67,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
6767
}
6868

6969
if expects_item || has_block_expr_parent {
70-
if !ctx.has_visibility_prev_sibling() {
70+
if ctx.qualifier_ctx.vis_node.is_none() {
7171
add_keyword("impl", "impl $1 {\n $0\n}");
7272
add_keyword("extern", "extern $0");
7373
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
33
use hir::Documentation;
44
use ide_db::{imports::insert_use::ImportScope, SnippetCap};
5-
use syntax::T;
65

76
use crate::{
87
context::{ItemListKind, PathCompletionCtx, PathKind},
@@ -52,10 +51,10 @@ pub(crate) fn complete_item_snippet(acc: &mut Completions, ctx: &CompletionConte
5251
}) => kind,
5352
_ => return,
5453
};
55-
if ctx.previous_token_is(T![unsafe]) || ctx.has_unfinished_impl_or_trait_prev_sibling() {
54+
if !ctx.qualifier_ctx.none() {
5655
return;
5756
}
58-
if ctx.has_visibility_prev_sibling() {
57+
if ctx.qualifier_ctx.vis_node.is_some() {
5958
return; // technically we could do some of these snippet completions if we were to put the
6059
// attributes before the vis node.
6160
}

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

Lines changed: 105 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,18 @@ pub(super) enum ItemListKind {
7575
ExternBlock,
7676
}
7777

78+
#[derive(Debug, Default)]
79+
pub(super) struct QualifierCtx {
80+
pub(super) unsafe_tok: Option<SyntaxToken>,
81+
pub(super) vis_node: Option<ast::Visibility>,
82+
}
83+
84+
impl QualifierCtx {
85+
pub(super) fn none(&self) -> bool {
86+
self.unsafe_tok.is_none() && self.vis_node.is_none()
87+
}
88+
}
89+
7890
#[derive(Debug)]
7991
pub(crate) struct PathCompletionCtx {
8092
/// If this is a call with () already there (or {} in case of record patterns)
@@ -253,6 +265,7 @@ pub(crate) struct CompletionContext<'a> {
253265
pub(super) ident_ctx: IdentContext,
254266

255267
pub(super) pattern_ctx: Option<PatternContext>,
268+
pub(super) qualifier_ctx: QualifierCtx,
256269

257270
pub(super) existing_derives: FxHashSet<hir::Macro>,
258271

@@ -363,17 +376,13 @@ impl<'a> CompletionContext<'a> {
363376
matches!(self.prev_sibling, Some(ImmediatePrevSibling::ImplDefType))
364377
}
365378

366-
pub(crate) fn has_visibility_prev_sibling(&self) -> bool {
367-
matches!(self.prev_sibling, Some(ImmediatePrevSibling::Visibility))
368-
}
369-
370379
pub(crate) fn after_if(&self) -> bool {
371380
matches!(self.prev_sibling, Some(ImmediatePrevSibling::IfExpr))
372381
}
373382

374383
// FIXME: This shouldn't exist
375384
pub(crate) fn is_path_disallowed(&self) -> bool {
376-
self.previous_token_is(T![unsafe])
385+
!self.qualifier_ctx.none()
377386
|| matches!(self.prev_sibling, Some(ImmediatePrevSibling::Visibility))
378387
|| (matches!(self.name_ctx(), Some(NameContext { .. })) && self.pattern_ctx.is_none())
379388
|| matches!(self.pattern_ctx, Some(PatternContext { record_pat: Some(_), .. }))
@@ -555,6 +564,7 @@ impl<'a> CompletionContext<'a> {
555564
// dummy value, will be overwritten
556565
ident_ctx: IdentContext::UnexpandedAttrTT { fake_attribute_under_caret: None },
557566
pattern_ctx: None,
567+
qualifier_ctx: Default::default(),
558568
existing_derives: Default::default(),
559569
locals,
560570
};
@@ -865,7 +875,7 @@ impl<'a> CompletionContext<'a> {
865875
offset: TextSize,
866876
derive_ctx: Option<(SyntaxNode, SyntaxNode, TextSize, ast::Attr)>,
867877
) -> Option<()> {
868-
let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap();
878+
let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased()?;
869879
let syntax_element = NodeOrToken::Token(fake_ident_token);
870880
if is_in_token_of_for_loop(syntax_element.clone()) {
871881
// for pat $0
@@ -967,7 +977,49 @@ impl<'a> CompletionContext<'a> {
967977
ast::NameLike::NameRef(name_ref) => {
968978
let parent = name_ref.syntax().parent()?;
969979
let (nameref_ctx, pat_ctx) =
970-
Self::classify_name_ref(&self.sema, &original_file, name_ref, parent);
980+
Self::classify_name_ref(&self.sema, &original_file, name_ref, parent.clone());
981+
982+
// Extract qualifiers
983+
if let Some(path_ctx) = &nameref_ctx.path_ctx {
984+
if path_ctx.qualifier.is_none() {
985+
let top = match path_ctx.kind {
986+
PathKind::Expr { in_block_expr: true, .. } => parent
987+
.ancestors()
988+
.find(|it| ast::PathExpr::can_cast(it.kind()))
989+
.and_then(|p| {
990+
let parent = p.parent()?;
991+
if ast::StmtList::can_cast(parent.kind()) {
992+
Some(p)
993+
} else if ast::ExprStmt::can_cast(parent.kind()) {
994+
Some(parent)
995+
} else {
996+
None
997+
}
998+
}),
999+
PathKind::Item { .. } => {
1000+
parent.ancestors().find(|it| ast::MacroCall::can_cast(it.kind()))
1001+
}
1002+
_ => None,
1003+
};
1004+
if let Some(top) = top {
1005+
if let Some(NodeOrToken::Node(error_node)) =
1006+
syntax::algo::non_trivia_sibling(
1007+
top.into(),
1008+
syntax::Direction::Prev,
1009+
)
1010+
{
1011+
if error_node.kind() == SyntaxKind::ERROR {
1012+
self.qualifier_ctx.unsafe_tok = error_node
1013+
.children_with_tokens()
1014+
.filter_map(NodeOrToken::into_token)
1015+
.find(|it| it.kind() == T![unsafe]);
1016+
self.qualifier_ctx.vis_node =
1017+
error_node.children().find_map(ast::Visibility::cast);
1018+
}
1019+
}
1020+
}
1021+
}
1022+
}
9711023
self.ident_ctx = IdentContext::NameRef(nameref_ctx);
9721024
self.pattern_ctx = pat_ctx;
9731025
}
@@ -1145,12 +1197,54 @@ impl<'a> CompletionContext<'a> {
11451197
}
11461198
};
11471199

1200+
// We do not want to generate path completions when we are sandwiched between an item decl signature and its body.
1201+
// ex. trait Foo $0 {}
1202+
// in these cases parser recovery usually kicks in for our inserted identifier, causing it
1203+
// to either be parsed as an ExprStmt or a MacroCall, depending on whether it is in a block
1204+
// expression or an item list.
1205+
// The following code checks if the body is missing, if it is we either cut off the body
1206+
// from the item or it was missing in the first place
1207+
let inbetween_body_and_decl_check = |node: SyntaxNode| {
1208+
if let Some(NodeOrToken::Node(n)) =
1209+
syntax::algo::non_trivia_sibling(node.into(), syntax::Direction::Prev)
1210+
{
1211+
if let Some(item) = ast::Item::cast(n) {
1212+
match item {
1213+
ast::Item::Const(it) => it.body().is_none(),
1214+
ast::Item::Enum(it) => it.variant_list().is_none(),
1215+
ast::Item::ExternBlock(it) => it.extern_item_list().is_none(),
1216+
ast::Item::Fn(it) => it.body().is_none(),
1217+
ast::Item::Impl(it) => it.assoc_item_list().is_none(),
1218+
ast::Item::Module(it) => it.item_list().is_none(),
1219+
ast::Item::Static(it) => it.body().is_none(),
1220+
ast::Item::Struct(it) => it.field_list().is_none(),
1221+
ast::Item::Trait(it) => it.assoc_item_list().is_none(),
1222+
ast::Item::TypeAlias(it) => it.ty().is_none(),
1223+
ast::Item::Union(it) => it.record_field_list().is_none(),
1224+
_ => false,
1225+
}
1226+
} else {
1227+
false
1228+
}
1229+
} else {
1230+
false
1231+
}
1232+
};
1233+
11481234
let kind = path.syntax().ancestors().find_map(|it| {
11491235
// using Option<Option<PathKind>> as extra controlflow
11501236
let kind = match_ast! {
11511237
match it {
11521238
ast::PathType(_) => Some(PathKind::Type),
11531239
ast::PathExpr(it) => {
1240+
if let Some(p) = it.syntax().parent() {
1241+
if ast::ExprStmt::can_cast(p.kind()) {
1242+
if inbetween_body_and_decl_check(p) {
1243+
return Some(None);
1244+
}
1245+
}
1246+
}
1247+
11541248
fill_record_expr(it.syntax());
11551249

11561250
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> {
11731267
Some(PathKind::Pat)
11741268
},
11751269
ast::MacroCall(it) => {
1270+
if inbetween_body_and_decl_check(it.syntax().clone()) {
1271+
return Some(None);
1272+
}
1273+
11761274
path_ctx.has_macro_bang = it.excl_token().is_some();
11771275
let parent = it.syntax().parent();
11781276
match parent.as_ref().map(|it| it.kind()) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,15 @@ pub fn completions(
143143
db: &RootDatabase,
144144
config: &CompletionConfig,
145145
position: FilePosition,
146-
trigger_character: Option<&str>,
146+
trigger_character: Option<char>,
147147
) -> Option<Completions> {
148148
let ctx = &CompletionContext::new(db, position, config)?;
149149
let mut acc = Completions::default();
150150

151151
{
152152
let acc = &mut acc;
153153
// prevent `(` from triggering unwanted completion noise
154-
if trigger_character != Some("(") {
154+
if trigger_character != Some('(') {
155155
completions::attribute::complete_attribute(acc, ctx);
156156
completions::attribute::complete_derive(acc, ctx);
157157
completions::attribute::complete_known_attribute_input(acc, ctx);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub(crate) fn completion_list_no_kw(ra_fixture: &str) -> String {
8888

8989
pub(crate) fn completion_list_with_trigger_character(
9090
ra_fixture: &str,
91-
trigger_character: Option<&str>,
91+
trigger_character: Option<char>,
9292
) -> String {
9393
completion_list_with_config(TEST_CONFIG, ra_fixture, true, trigger_character)
9494
}
@@ -97,7 +97,7 @@ fn completion_list_with_config(
9797
config: CompletionConfig,
9898
ra_fixture: &str,
9999
include_keywords: bool,
100-
trigger_character: Option<&str>,
100+
trigger_character: Option<char>,
101101
) -> String {
102102
// filter out all but one builtintype completion for smaller test outputs
103103
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
225225
pub(crate) fn get_all_items(
226226
config: CompletionConfig,
227227
code: &str,
228-
trigger_character: Option<&str>,
228+
trigger_character: Option<char>,
229229
) -> Vec<CompletionItem> {
230230
let (db, position) = position(code);
231231
let res = crate::completions(&db, &config, position, trigger_character)

‎crates/ide-completion/src/tests/fn_param.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ fn check(ra_fixture: &str, expect: Expect) {
77
expect.assert_eq(&actual);
88
}
99

10-
fn check_with_trigger_character(ra_fixture: &str, trigger_character: Option<&str>, expect: Expect) {
11-
let actual = completion_list_with_trigger_character(ra_fixture, trigger_character);
10+
fn check_with_trigger_character(ra_fixture: &str, trigger_character: char, expect: Expect) {
11+
let actual = completion_list_with_trigger_character(ra_fixture, Some(trigger_character));
1212
expect.assert_eq(&actual)
1313
}
1414

@@ -124,7 +124,7 @@ fn trigger_by_l_paren() {
124124
r#"
125125
fn foo($0)
126126
"#,
127-
Some("("),
127+
'(',
128128
expect![[]],
129129
)
130130
}

‎crates/ide-completion/src/tests/item.rs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,7 @@ fn after_struct_name() {
9292
check(
9393
r"struct Struct $0",
9494
expect![[r#"
95-
ma makro!(…) macro_rules! makro
96-
md module
9795
kw const
98-
kw crate::
9996
kw enum
10097
kw extern
10198
kw fn
@@ -104,18 +101,13 @@ fn after_struct_name() {
104101
kw pub
105102
kw pub(crate)
106103
kw pub(super)
107-
kw self::
108104
kw static
109105
kw struct
110-
kw super::
111106
kw trait
112107
kw type
113108
kw union
114109
kw unsafe
115110
kw use
116-
sn macro_rules
117-
sn tfn (Test function)
118-
sn tmod (Test module)
119111
"#]],
120112
);
121113
}
@@ -126,10 +118,7 @@ fn after_fn_name() {
126118
check(
127119
r"fn func() $0",
128120
expect![[r#"
129-
ma makro!(…) macro_rules! makro
130-
md module
131121
kw const
132-
kw crate::
133122
kw enum
134123
kw extern
135124
kw fn
@@ -138,18 +127,13 @@ fn after_fn_name() {
138127
kw pub
139128
kw pub(crate)
140129
kw pub(super)
141-
kw self::
142130
kw static
143131
kw struct
144-
kw super::
145132
kw trait
146133
kw type
147134
kw union
148135
kw unsafe
149136
kw use
150-
sn macro_rules
151-
sn tfn (Test function)
152-
sn tmod (Test module)
153137
"#]],
154138
);
155139
}

0 commit comments

Comments
 (0)