Repository: rust-lang/rust-analyzer
Pull request #12418 (2022-05-30 14:03:08)

internal: More precise completion filtering with existing item qualifiers
Now we are unfortunately approaching the more complex cases for filtering completions where we have to deal with parser recovered nodes properly ...


Patch:
diff --git a/crates/ide-completion/src/completions/item_list.rs b/crates/ide-completion/src/completions/item_list.rs
index ebbc33c2da..edff146d8d 100644
--- a/crates/ide-completion/src/completions/item_list.rs
+++ b/crates/ide-completion/src/completions/item_list.rs
@@ -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 => {}
     }
 }
diff --git a/crates/ide-completion/src/completions/keyword.rs b/crates/ide-completion/src/completions/keyword.rs
index 5917da9b7f..281e6e9783 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 2bae19c84f..ebc3bb5a6f 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 5e2118ae14..b70d6f9e8d 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<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()) {
diff --git a/crates/ide-completion/src/lib.rs b/crates/ide-completion/src/lib.rs
index 2bbdc48e63..9659efad61 100644
--- a/crates/ide-completion/src/lib.rs
+++ b/crates/ide-completion/src/lib.rs
@@ -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);
diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs
index 116c1c480a..910aee7b05 100644
--- a/crates/ide/src/inlay_hints.rs
+++ b/crates/ide/src/inlay_hints.rs
@@ -117,32 +117,33 @@ pub(crate) fn inlay_hints(
 
     let mut acc = Vec::new();
 
-    if let Some(scope) = sema.scope(&file) {
-        let famous_defs = FamousDefs(&sema, scope.krate());
-
-        let hints = |node| hints(&mut acc, &famous_defs, config, file_id, node);
-        match range_limit {
-            Some(FileRange { range, .. }) => match file.covering_element(range) {
-                NodeOrToken::Token(_) => return acc,
-                NodeOrToken::Node(n) => n
-                    .descendants()
-                    .filter(|descendant| range.intersect(descendant.text_range()).is_some())
-                    .for_each(hints),
-            },
-            None => file.descendants().for_each(hints),
-        };
-    }
+    let hints = |node| hints(&mut acc, &sema, config, file_id, node);
+    match range_limit {
+        Some(FileRange { range, .. }) => match file.covering_element(range) {
+            NodeOrToken::Token(_) => return acc,
+            NodeOrToken::Node(n) => n
+                .descendants()
+                .filter(|descendant| range.intersect(descendant.text_range()).is_some())
+                .for_each(hints),
+        },
+        None => file.descendants().for_each(hints),
+    };
 
     acc
 }
 
 fn hints(
     hints: &mut Vec<InlayHint>,
-    famous_defs @ FamousDefs(sema, _): &FamousDefs,
+    sema: &Semantics<RootDatabase>,
     config: &InlayHintsConfig,
     file_id: FileId,
     node: SyntaxNode,
 ) {
+    let famous_defs = match sema.scope(&node) {
+        Some(it) => FamousDefs(sema, it.krate()),
+        None => return,
+    };
+
     closing_brace_hints(hints, sema, config, file_id, node.clone());
     match_ast! {
         match node {
@@ -167,18 +168,8 @@ fn hints(
                 }
                 Some(())
             },
-            ast::Item(it) => match it {
-                // FIXME: record impl lifetimes so they aren't being reused in assoc item lifetime inlay hints
-                ast::Item::Impl(_) => None,
-                ast::Item::Fn(it) => fn_lifetime_fn_hints(hints, config, it),
-                // static type elisions
-                ast::Item::Static(it) => implicit_static_hints(hints, config, Either::Left(it)),
-                ast::Item::Const(it) => implicit_static_hints(hints, config, Either::Right(it)),
-                _ => None,
-            },
-            // FIXME: fn-ptr type, dyn fn type, and trait object type elisions
-            ast::Type(_) => None,
-            _ => None,
+            ast::Fn(it) => lifetime_fn_hints(hints, config, it),
+            _ => Some(()),
         }
     };
 }
@@ -288,39 +279,7 @@ fn closing_brace_hints(
     None
 }
 
-fn implicit_static_hints(
-    acc: &mut Vec<InlayHint>,
-    config: &InlayHintsConfig,
-    statik_or_const: Either<ast::Static, ast::Const>,
-) -> Option<()> {
-    if config.lifetime_elision_hints != LifetimeElisionHints::Always {
-        return None;
-    }
-
-    if let Either::Right(it) = &statik_or_const {
-        if ast::AssocItemList::can_cast(
-            it.syntax().parent().map_or(SyntaxKind::EOF, |it| it.kind()),
-        ) {
-            return None;
-        }
-    }
-
-    if let Some(ast::Type::RefType(ty)) = statik_or_const.either(|it| it.ty(), |it| it.ty()) {
-        if ty.lifetime().is_none() {
-            let t = ty.amp_token()?;
-            acc.push(InlayHint {
-                range: t.text_range(),
-                kind: InlayKind::LifetimeHint,
-                label: "'static".to_owned(),
-                tooltip: Some(InlayTooltip::String("Elided static lifetime".into())),
-            });
-        }
-    }
-
-    Some(())
-}
-
-fn fn_lifetime_fn_hints(
+fn lifetime_fn_hints(
     acc: &mut Vec<InlayHint>,
     config: &InlayHintsConfig,
     func: ast::Fn,
@@ -2634,30 +2593,6 @@ impl () {
         );
     }
 
-    #[test]
-    fn hints_lifetimes_static() {
-        check_with_config(
-            InlayHintsConfig {
-                lifetime_elision_hints: LifetimeElisionHints::Always,
-                ..TEST_CONFIG
-            },
-            r#"
-trait Trait {}
-static S: &str = "";
-//        ^'static
-const C: &str = "";
-//       ^'static
-const C: &dyn Trait = panic!();
-//       ^'static
-
-impl () {
-    const C: &str = "";
-    const C: &dyn Trait = panic!();
-}
-"#,
-        );
-    }
-
     #[test]
     fn hints_implicit_reborrow() {
         check_with_config(
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index 29bd86430d..aab5ceda36 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -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)
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index a8e3e93b8f..ff61081aa8 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -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,


Test patch:
diff --git a/crates/ide-completion/src/tests.rs b/crates/ide-completion/src/tests.rs
index 7625018058..d30ff77bab 100644
--- a/crates/ide-completion/src/tests.rs
+++ b/crates/ide-completion/src/tests.rs
@@ -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)
diff --git a/crates/ide-completion/src/tests/fn_param.rs b/crates/ide-completion/src/tests/fn_param.rs
index a08f3f6e88..f5a5b5bae6 100644
--- a/crates/ide-completion/src/tests/fn_param.rs
+++ b/crates/ide-completion/src/tests/fn_param.rs
@@ -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![[]],
     )
 }
diff --git a/crates/ide-completion/src/tests/item.rs b/crates/ide-completion/src/tests/item.rs
index e4b70f97e3..537c9a7fa2 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)
         "#]],
     );
 }
diff --git a/crates/ide-completion/src/tests/visibility.rs b/crates/ide-completion/src/tests/visibility.rs
index 64bbb444bf..151dd6a7e8 100644
--- a/crates/ide-completion/src/tests/visibility.rs
+++ b/crates/ide-completion/src/tests/visibility.rs
@@ -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