Repository: rust-analyzer/rust-analyzer
Pull request #11685 (2022-03-12 01:15:06)

internal: Simplify CompletionContext
bors r+


Patch:
diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs
index 4704e842e6..ac782352fc 100644
--- a/crates/ide_completion/src/completions/keyword.rs
+++ b/crates/ide_completion/src/completions/keyword.rs
@@ -163,6 +163,7 @@ fn add_keyword(acc: &mut Completions, ctx: &CompletionContext, kw: &str, snippet
     match ctx.config.snippet_cap {
         Some(cap) => {
             if snippet.ends_with('}') && ctx.incomplete_let {
+                // complete block expression snippets with a trailing semicolon, if inside an incomplete let
                 cov_mark::hit!(let_semi);
                 item.insert_snippet(cap, format!("{};", snippet));
             } else {
diff --git a/crates/ide_completion/src/completions/lifetime.rs b/crates/ide_completion/src/completions/lifetime.rs
index 878d72ea0f..b8a50442bc 100644
--- a/crates/ide_completion/src/completions/lifetime.rs
+++ b/crates/ide_completion/src/completions/lifetime.rs
@@ -8,7 +8,7 @@
 //! show up for normal completions, or they won't show completions other than lifetimes depending
 //! on the fixture input.
 use hir::{known, ScopeDef};
-use syntax::ast;
+use syntax::{ast, TokenText};
 
 use crate::{
     completions::Completions,
@@ -19,24 +19,24 @@ use crate::{
 pub(crate) fn complete_lifetime(acc: &mut Completions, ctx: &CompletionContext) {
     let lp = match &ctx.lifetime_ctx {
         Some(LifetimeContext::Lifetime) => None,
-        Some(LifetimeContext::LifetimeParam(param)) => param.as_ref(),
+        Some(LifetimeContext::LifetimeParam { is_decl: false, param }) => Some(param),
         _ => return,
     };
-    let lp_string;
     let param_lifetime = match (&ctx.name_syntax, lp.and_then(|lp| lp.lifetime())) {
         (Some(ast::NameLike::Lifetime(lt)), Some(lp)) if lp == lt.clone() => return,
-        (Some(_), Some(lp)) => {
-            lp_string = lp.to_string();
-            Some(&*lp_string)
-        }
+        (Some(_), Some(lp)) => Some(lp),
         _ => None,
     };
+    let param_lifetime = param_lifetime.as_ref().map(ast::Lifetime::text);
+    let param_lifetime = param_lifetime.as_ref().map(TokenText::as_str);
 
     ctx.scope.process_all_names(&mut |name, res| {
-        if let ScopeDef::GenericParam(hir::GenericParam::LifetimeParam(_)) = res {
-            if param_lifetime != Some(&*name.to_smol_str()) {
-                acc.add_lifetime(ctx, name);
-            }
+        if matches!(
+            res,
+            ScopeDef::GenericParam(hir::GenericParam::LifetimeParam(_))
+                 if param_lifetime != Some(&*name.to_smol_str())
+        ) {
+            acc.add_lifetime(ctx, name);
         }
     });
     if param_lifetime.is_none() {
@@ -195,6 +195,12 @@ fn foo2<'lifetime, T>() where T: Trait<Item = 'a$0> {}
     fn complete_lifetime_in_param_list() {
         check(
             r#"
+fn foo<'$0>() {}
+"#,
+            expect![[r#""#]],
+        );
+        check(
+            r#"
 fn foo<'a$0>() {}
 "#,
             expect![[r#""#]],
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index 51bbd66ff3..bf841ee2b7 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -24,8 +24,8 @@ use text_edit::Indel;
 
 use crate::{
     patterns::{
-        determine_location, determine_prev_sibling, for_is_prev2, inside_impl_trait_block,
-        is_in_loop_body, previous_token, ImmediateLocation, ImmediatePrevSibling,
+        determine_location, determine_prev_sibling, for_is_prev2, is_in_loop_body, previous_token,
+        ImmediateLocation, ImmediatePrevSibling,
     },
     CompletionConfig,
 };
@@ -94,7 +94,7 @@ pub(super) struct PatternContext {
 
 #[derive(Debug)]
 pub(super) enum LifetimeContext {
-    LifetimeParam(Option<ast::LifetimeParam>),
+    LifetimeParam { is_decl: bool, param: ast::LifetimeParam },
     Lifetime,
     LabelRef,
     LabelDef,
@@ -115,6 +115,7 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) db: &'a RootDatabase,
     pub(super) config: &'a CompletionConfig,
     pub(super) position: FilePosition,
+
     /// The token before the cursor, in the original file.
     pub(super) original_token: SyntaxToken,
     /// The token before the cursor, in the macro-expanded file.
@@ -146,32 +147,22 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) existing_derives: FxHashSet<hir::Macro>,
 
     pub(super) locals: Vec<(Name, Local)>,
-
-    no_completion_required: bool,
 }
 
 impl<'a> CompletionContext<'a> {
-    /// Checks whether completions in that particular case don't make much sense.
-    /// Examples:
-    /// - `fn $0` -- we expect function name, it's unlikely that "hint" will be helpful.
-    ///   Exception for this case is `impl Trait for Foo`, where we would like to hint trait method names.
-    /// - `for _ i$0` -- obviously, it'll be "in" keyword.
-    pub(crate) fn no_completion_required(&self) -> bool {
-        self.no_completion_required
-    }
-
     /// The range of the identifier that is being completed.
     pub(crate) fn source_range(&self) -> TextRange {
         // check kind of macro-expanded token, but use range of original token
         let kind = self.token.kind();
-        if kind == IDENT || kind == LIFETIME_IDENT || kind == UNDERSCORE || kind.is_keyword() {
-            self.original_token.text_range()
-        } else if kind == CHAR {
-            // assume we are completing a lifetime but the user has only typed the '
-            cov_mark::hit!(completes_if_lifetime_without_idents);
-            TextRange::at(self.original_token.text_range().start(), TextSize::from(1))
-        } else {
-            TextRange::empty(self.position.offset)
+        match kind {
+            CHAR => {
+                // assume we are completing a lifetime but the user has only typed the '
+                cov_mark::hit!(completes_if_lifetime_without_idents);
+                TextRange::at(self.original_token.text_range().start(), TextSize::from(1))
+            }
+            IDENT | LIFETIME_IDENT | UNDERSCORE => self.original_token.text_range(),
+            _ if kind.is_keyword() => self.original_token.text_range(),
+            _ => TextRange::empty(self.position.offset),
         }
     }
 
@@ -453,7 +444,6 @@ impl<'a> CompletionContext<'a> {
             path_context: None,
             locals,
             incomplete_let: false,
-            no_completion_required: false,
             existing_derives: Default::default(),
         };
         ctx.expand_and_fill(
@@ -740,14 +730,16 @@ impl<'a> CompletionContext<'a> {
     ) {
         let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap();
         let syntax_element = NodeOrToken::Token(fake_ident_token);
-        self.previous_token = previous_token(syntax_element.clone());
-        self.no_completion_required = {
-            let inside_impl_trait_block = inside_impl_trait_block(syntax_element.clone());
-            let fn_is_prev = self.previous_token_is(T![fn]);
-            let for_is_prev2 = for_is_prev2(syntax_element.clone());
-            (fn_is_prev && !inside_impl_trait_block) || for_is_prev2
-        };
+        if for_is_prev2(syntax_element.clone()) {
+            // for pat $0
+            // there is nothing to complete here except `in` keyword
+            // don't bother populating the context
+            // FIXME: the completion calculations should end up good enough
+            // such that this special case becomes unnecessary
+            return;
+        }
 
+        self.previous_token = previous_token(syntax_element.clone());
         self.fake_attribute_under_caret = syntax_element.ancestors().find_map(ast::Attr::cast);
 
         self.incomplete_let =
@@ -755,9 +747,7 @@ impl<'a> CompletionContext<'a> {
                 it.syntax().text_range().end() == syntax_element.text_range().end()
             });
 
-        let (expected_type, expected_name) = self.expected_type_and_name();
-        self.expected_type = expected_type;
-        self.expected_name = expected_name;
+        (self.expected_type, self.expected_name) = self.expected_type_and_name();
 
         // Overwrite the path kind for derives
         if let Some((original_file, file_with_fake_ident, offset)) = derive_ctx {
@@ -808,8 +798,7 @@ impl<'a> CompletionContext<'a> {
 
         match name_like {
             ast::NameLike::Lifetime(lifetime) => {
-                self.lifetime_ctx =
-                    Self::classify_lifetime(&self.sema, original_file, lifetime, offset);
+                self.lifetime_ctx = Self::classify_lifetime(&self.sema, original_file, lifetime);
             }
             ast::NameLike::NameRef(name_ref) => {
                 if let Some((path_ctx, pat_ctx)) =
@@ -826,10 +815,9 @@ impl<'a> CompletionContext<'a> {
     }
 
     fn classify_lifetime(
-        sema: &Semantics<RootDatabase>,
-        original_file: &SyntaxNode,
+        _sema: &Semantics<RootDatabase>,
+        _original_file: &SyntaxNode,
         lifetime: ast::Lifetime,
-        offset: TextSize,
     ) -> Option<LifetimeContext> {
         let parent = lifetime.syntax().parent()?;
         if parent.kind() == ERROR {
@@ -838,7 +826,10 @@ impl<'a> CompletionContext<'a> {
 
         Some(match_ast! {
             match parent {
-                ast::LifetimeParam(_) => LifetimeContext::LifetimeParam(sema.find_node_at_offset_with_macros(original_file, offset)),
+                ast::LifetimeParam(param) => LifetimeContext::LifetimeParam {
+                    is_decl: param.lifetime().as_ref() == Some(&lifetime),
+                    param
+                },
                 ast::BreakExpr(_) => LifetimeContext::LabelRef,
                 ast::ContinueExpr(_) => LifetimeContext::LabelRef,
                 ast::Label(_) => LifetimeContext::LabelDef,
diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs
index 86a6947b1d..422bda6460 100644
--- a/crates/ide_completion/src/lib.rs
+++ b/crates/ide_completion/src/lib.rs
@@ -144,12 +144,6 @@ pub fn completions(
 ) -> Option<Completions> {
     let ctx = CompletionContext::new(db, position, config)?;
 
-    if ctx.no_completion_required() {
-        cov_mark::hit!(no_completion_required);
-        // No work required here.
-        return None;
-    }
-
     let mut acc = Completions::default();
     completions::attribute::complete_attribute(&mut acc, &ctx);
     completions::attribute::complete_derive(&mut acc, &ctx);
diff --git a/crates/ide_completion/src/patterns.rs b/crates/ide_completion/src/patterns.rs
index 41e2423f46..8a53d6e4d4 100644
--- a/crates/ide_completion/src/patterns.rs
+++ b/crates/ide_completion/src/patterns.rs
@@ -15,7 +15,7 @@ use syntax::{
 };
 
 #[cfg(test)]
-use crate::tests::{check_pattern_is_applicable, check_pattern_is_not_applicable};
+use crate::tests::check_pattern_is_applicable;
 
 /// Immediate previous node to what we are completing.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -291,24 +291,6 @@ fn find_node_with_range<N: AstNode>(syntax: &SyntaxNode, range: TextRange) -> Op
     syntax.covering_element(range).ancestors().find_map(N::cast)
 }
 
-pub(crate) fn inside_impl_trait_block(element: SyntaxElement) -> bool {
-    // Here we search `impl` keyword up through the all ancestors, unlike in `has_impl_parent`,
-    // where we only check the first parent with different text range.
-    element
-        .ancestors()
-        .find(|it| it.kind() == IMPL)
-        .map(|it| ast::Impl::cast(it).unwrap())
-        .map(|it| it.trait_().is_some())
-        .unwrap_or(false)
-}
-#[test]
-fn test_inside_impl_trait_block() {
-    check_pattern_is_applicable(r"impl Foo for Bar { f$0 }", inside_impl_trait_block);
-    check_pattern_is_applicable(r"impl Foo for Bar { fn f$0 }", inside_impl_trait_block);
-    check_pattern_is_not_applicable(r"impl A { f$0 }", inside_impl_trait_block);
-    check_pattern_is_not_applicable(r"impl A { fn f$0 }", inside_impl_trait_block);
-}
-
 pub(crate) fn previous_token(element: SyntaxElement) -> Option<SyntaxToken> {
     element.into_token().and_then(previous_non_trivia_token)
 }
diff --git a/crates/parser/src/grammar/generic_args.rs b/crates/parser/src/grammar/generic_args.rs
index 948873ae2d..1148c6c35c 100644
--- a/crates/parser/src/grammar/generic_args.rs
+++ b/crates/parser/src/grammar/generic_args.rs
@@ -72,24 +72,28 @@ fn lifetime_arg(p: &mut Parser) {
     m.complete(p, LIFETIME_ARG);
 }
 
-pub(super) fn const_arg_expr(p: &mut Parser) {
-    // The tests in here are really for `const_arg`, which wraps the content
-    // CONST_ARG.
+// test const_arg
+// type T = S<92>;
+pub(super) fn const_arg(p: &mut Parser) {
+    let m = p.start();
     match p.current() {
         // test const_arg_block
         // type T = S<{90 + 2}>;
         T!['{'] => {
             expressions::block_expr(p);
+            m.complete(p, CONST_ARG);
         }
         // test const_arg_literal
         // type T = S<"hello", 0xdeadbeef>;
         k if k.is_literal() => {
             expressions::literal(p);
+            m.complete(p, CONST_ARG);
         }
         // test const_arg_bool_literal
         // type T = S<true>;
         T![true] | T![false] => {
             expressions::literal(p);
+            m.complete(p, CONST_ARG);
         }
         // test const_arg_negative_number
         // type T = S<-92>;
@@ -98,24 +102,19 @@ pub(super) fn const_arg_expr(p: &mut Parser) {
             p.bump(T![-]);
             expressions::literal(p);
             lm.complete(p, PREFIX_EXPR);
+            m.complete(p, CONST_ARG);
         }
+        // test const_arg_path
+        // struct S<const N: u32 = u32::MAX>;
         _ => {
-            // This shouldn't be hit by `const_arg`
             let lm = p.start();
             paths::use_path(p);
             lm.complete(p, PATH_EXPR);
+            m.complete(p, CONST_ARG);
         }
     }
 }
 
-// test const_arg
-// type T = S<92>;
-pub(super) fn const_arg(p: &mut Parser) {
-    let m = p.start();
-    const_arg_expr(p);
-    m.complete(p, CONST_ARG);
-}
-
 fn type_arg(p: &mut Parser) {
     let m = p.start();
     types::type_(p);
diff --git a/crates/parser/src/grammar/generic_params.rs b/crates/parser/src/grammar/generic_params.rs
index fbd514078d..1009c67ffa 100644
--- a/crates/parser/src/grammar/generic_params.rs
+++ b/crates/parser/src/grammar/generic_params.rs
@@ -79,16 +79,10 @@ fn const_param(p: &mut Parser, m: Marker) {
     }
 
     if p.at(T![=]) {
-        // test const_param_default_literal
+        // test const_param_defaults
         // struct A<const N: i32 = -1>;
         p.bump(T![=]);
-
-        // test const_param_default_expression
-        // struct A<const N: i32 = { 1 }>;
-
-        // test const_param_default_path
-        // struct A<const N: i32 = i32::MAX>;
-        generic_args::const_arg_expr(p);
+        generic_args::const_arg(p);
     }
 
     m.complete(p, CONST_PARAM);


Test patch:
diff --git a/crates/ide_completion/src/tests.rs b/crates/ide_completion/src/tests.rs
index eedc37e2a6..f505e82d22 100644
--- a/crates/ide_completion/src/tests.rs
+++ b/crates/ide_completion/src/tests.rs
@@ -154,10 +154,12 @@ fn render_completion_list(completions: Vec<CompletionItem>) -> String {
         .collect()
 }
 
+#[track_caller]
 pub(crate) fn check_edit(what: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
     check_edit_with_config(TEST_CONFIG, what, ra_fixture_before, ra_fixture_after)
 }
 
+#[track_caller]
 pub(crate) fn check_edit_with_config(
     config: CompletionConfig,
     what: &str,
@@ -199,32 +201,14 @@ pub(crate) fn check_pattern_is_applicable(code: &str, check: impl FnOnce(SyntaxE
     assert!(check(NodeOrToken::Token(token)));
 }
 
-pub(crate) fn check_pattern_is_not_applicable(code: &str, check: fn(SyntaxElement) -> bool) {
-    let (db, pos) = position(code);
-    let sema = Semantics::new(&db);
-    let original_file = sema.parse(pos.file_id);
-    let token = original_file.syntax().token_at_offset(pos.offset).left_biased().unwrap();
-    assert!(!check(NodeOrToken::Token(token)));
-}
-
 pub(crate) fn get_all_items(config: CompletionConfig, code: &str) -> Vec<CompletionItem> {
     let (db, position) = position(code);
     crate::completions(&db, &config, position).map_or_else(Vec::default, Into::into)
 }
 
-fn check_no_completion(ra_fixture: &str) {
-    let (db, position) = position(ra_fixture);
-
-    assert!(
-        crate::completions(&db, &TEST_CONFIG, position).is_none(),
-        "Completions were generated, but weren't expected"
-    );
-}
-
 #[test]
 fn test_no_completions_required() {
-    cov_mark::check!(no_completion_required);
-    check_no_completion(r#"fn foo() { for i i$0 }"#);
+    assert_eq!(completion_list(r#"fn foo() { for i i$0 }"#), String::new());
 }
 
 #[test]
diff --git a/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast b/crates/parser/test_data/parser/inline/ok/0165_const_param_defaults.rast
similarity index 81%
rename from crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast
rename to crates/parser/test_data/parser/inline/ok/0165_const_param_defaults.rast
index 8e52313651..6de10353bf 100644
--- a/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rast
+++ b/crates/parser/test_data/parser/inline/ok/0165_const_param_defaults.rast
@@ -21,10 +21,11 @@ SOURCE_FILE
         WHITESPACE " "
         EQ "="
         WHITESPACE " "
-        PREFIX_EXPR
-          MINUS "-"
-          LITERAL
-            INT_NUMBER "1"
+        CONST_ARG
+          PREFIX_EXPR
+            MINUS "-"
+            LITERAL
+              INT_NUMBER "1"
       R_ANGLE ">"
     SEMICOLON ";"
   WHITESPACE "\n"
diff --git a/crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rs b/crates/parser/test_data/parser/inline/ok/0165_const_param_defaults.rs
similarity index 100%
rename from crates/parser/test_data/parser/inline/ok/0200_const_param_default_literal.rs
rename to crates/parser/test_data/parser/inline/ok/0165_const_param_defaults.rs
diff --git a/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast b/crates/parser/test_data/parser/inline/ok/0188_const_arg_path.rast
similarity index 67%
rename from crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast
rename to crates/parser/test_data/parser/inline/ok/0188_const_arg_path.rast
index 11002bf98d..440dfb06a3 100644
--- a/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rast
+++ b/crates/parser/test_data/parser/inline/ok/0188_const_arg_path.rast
@@ -3,7 +3,7 @@ SOURCE_FILE
     STRUCT_KW "struct"
     WHITESPACE " "
     NAME
-      IDENT "A"
+      IDENT "S"
     GENERIC_PARAM_LIST
       L_ANGLE "<"
       CONST_PARAM
@@ -17,20 +17,21 @@ SOURCE_FILE
           PATH
             PATH_SEGMENT
               NAME_REF
-                IDENT "i32"
+                IDENT "u32"
         WHITESPACE " "
         EQ "="
         WHITESPACE " "
-        PATH_EXPR
-          PATH
+        CONST_ARG
+          PATH_EXPR
             PATH
+              PATH
+                PATH_SEGMENT
+                  NAME_REF
+                    IDENT "u32"
+              COLON2 "::"
               PATH_SEGMENT
                 NAME_REF
-                  IDENT "i32"
-            COLON2 "::"
-            PATH_SEGMENT
-              NAME_REF
-                IDENT "MAX"
+                  IDENT "MAX"
       R_ANGLE ">"
     SEMICOLON ";"
   WHITESPACE "\n"
diff --git a/crates/parser/test_data/parser/inline/ok/0188_const_arg_path.rs b/crates/parser/test_data/parser/inline/ok/0188_const_arg_path.rs
new file mode 100644
index 0000000000..ee075f3e50
--- /dev/null
+++ b/crates/parser/test_data/parser/inline/ok/0188_const_arg_path.rs
@@ -0,0 +1,1 @@
+struct S<const N: u32 = u32::MAX>;
diff --git a/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rs b/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rs
deleted file mode 100644
index f3da43ca06..0000000000
--- a/crates/parser/test_data/parser/inline/ok/0188_const_param_default_path.rs
+++ /dev/null
@@ -1,1 +0,0 @@
-struct A<const N: i32 = i32::MAX>;
diff --git a/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast b/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast
deleted file mode 100644
index 0607ff54fb..0000000000
--- a/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rast
+++ /dev/null
@@ -1,34 +0,0 @@
-SOURCE_FILE
-  STRUCT
-    STRUCT_KW "struct"
-    WHITESPACE " "
-    NAME
-      IDENT "A"
-    GENERIC_PARAM_LIST
-      L_ANGLE "<"
-      CONST_PARAM
-        CONST_KW "const"
-        WHITESPACE " "
-        NAME
-          IDENT "N"
-        COLON ":"
-        WHITESPACE " "
-        PATH_TYPE
-          PATH
-            PATH_SEGMENT
-              NAME_REF
-                IDENT "i32"
-        WHITESPACE " "
-        EQ "="
-        WHITESPACE " "
-        BLOCK_EXPR
-          STMT_LIST
-            L_CURLY "{"
-            WHITESPACE " "
-            LITERAL
-              INT_NUMBER "1"
-            WHITESPACE " "
-            R_CURLY "}"
-      R_ANGLE ">"
-    SEMICOLON ";"
-  WHITESPACE "\n"
diff --git a/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rs b/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rs
deleted file mode 100644
index 551bde0b00..0000000000
--- a/crates/parser/test_data/parser/inline/ok/0199_const_param_default_expression.rs
+++ /dev/null
@@ -1,1 +0,0 @@
-struct A<const N: i32 = { 1 }>;