Merge #7281

7281: Insert `;` when completing keywords in let r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

2 files changed, 89 insertions, 35 deletions

diff --git a/crates/completion/src/completions/keyword.rs b/crates/completion/src/completions/keyword.rs
index 425a688ff..c1af348dc 100644
--- a/crates/completion/src/completions/keyword.rs
+++ b/crates/completion/src/completions/keyword.rs
@@ -1,6 +1,6 @@
 //! Completes keywords.
 
-use syntax::{ast, SyntaxKind};
+use syntax::SyntaxKind;
 use test_utils::mark;
 
 use crate::{CompletionContext, CompletionItem, CompletionItemKind, CompletionKind, Completions};
@@ -86,8 +86,8 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         add_keyword(ctx, acc, "match", "match $0 {}");
         add_keyword(ctx, acc, "while", "while $0 {}");
         add_keyword(ctx, acc, "loop", "loop {$0}");
-        add_keyword(ctx, acc, "if", "if ");
-        add_keyword(ctx, acc, "if let", "if let ");
+        add_keyword(ctx, acc, "if", "if $0 {}");
+        add_keyword(ctx, acc, "if let", "if let $1 = $0 {}");
     }
 
     if ctx.if_is_prev || ctx.block_expr_parent {
@@ -143,47 +143,49 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         Some(it) => it,
         None => return,
     };
-    acc.add_all(complete_return(ctx, &fn_def, ctx.can_be_stmt));
-}
-
-fn keyword(ctx: &CompletionContext, kw: &str, snippet: &str) -> CompletionItem {
-    let res = CompletionItem::new(CompletionKind::Keyword, ctx.source_range(), kw)
-        .kind(CompletionItemKind::Keyword);
 
-    match ctx.config.snippet_cap {
-        Some(cap) => res.insert_snippet(cap, snippet),
-        _ => res.insert_text(if snippet.contains('$') { kw } else { snippet }),
-    }
-    .build()
+    add_keyword(
+        ctx,
+        acc,
+        "return",
+        match (ctx.can_be_stmt, fn_def.ret_type().is_some()) {
+            (true, true) => "return $0;",
+            (true, false) => "return;",
+            (false, true) => "return $0",
+            (false, false) => "return",
+        },
+    )
 }
 
 fn add_keyword(ctx: &CompletionContext, acc: &mut Completions, kw: &str, snippet: &str) {
-    acc.add(keyword(ctx, kw, snippet));
-}
-
-fn complete_return(
-    ctx: &CompletionContext,
-    fn_def: &ast::Fn,
-    can_be_stmt: bool,
-) -> Option<CompletionItem> {
-    let snip = match (can_be_stmt, fn_def.ret_type().is_some()) {
-        (true, true) => "return $0;",
-        (true, false) => "return;",
-        (false, true) => "return $0",
-        (false, false) => "return",
+    let builder = CompletionItem::new(CompletionKind::Keyword, ctx.source_range(), kw)
+        .kind(CompletionItemKind::Keyword);
+    let builder = match ctx.config.snippet_cap {
+        Some(cap) => {
+            let tmp;
+            let snippet = if snippet.ends_with('}') && ctx.incomplete_let {
+                mark::hit!(let_semi);
+                tmp = format!("{};", snippet);
+                &tmp
+            } else {
+                snippet
+            };
+            builder.insert_snippet(cap, snippet)
+        }
+        None => builder.insert_text(if snippet.contains('$') { kw } else { snippet }),
     };
-    Some(keyword(ctx, "return", snip))
+    acc.add(builder.build());
 }
 
 #[cfg(test)]
 mod tests {
     use expect_test::{expect, Expect};
+    use test_utils::mark;
 
     use crate::{
         test_utils::{check_edit, completion_list},
         CompletionKind,
     };
-    use test_utils::mark;
 
     fn check(ra_fixture: &str, expect: Expect) {
         let actual = completion_list(ra_fixture, CompletionKind::Keyword);
@@ -609,4 +611,50 @@ fn foo() {
             "#]],
         );
     }
+
+    #[test]
+    fn let_semi() {
+        mark::check!(let_semi);
+        check_edit(
+            "match",
+            r#"
+fn main() { let x = $0 }
+"#,
+            r#"
+fn main() { let x = match $0 {}; }
+"#,
+        );
+
+        check_edit(
+            "if",
+            r#"
+fn main() {
+    let x = $0
+    let y = 92;
+}
+"#,
+            r#"
+fn main() {
+    let x = if $0 {};
+    let y = 92;
+}
+"#,
+        );
+
+        check_edit(
+            "loop",
+            r#"
+fn main() {
+    let x = $0
+    bar();
+}
+"#,
+            r#"
+fn main() {
+    let x = loop {$0};
+    bar();
+}
+"#,
+        );
+    }
 }
diff --git a/crates/completion/src/context.rs b/crates/completion/src/context.rs
index ebf28e887..d809460e2 100644
--- a/crates/completion/src/context.rs
+++ b/crates/completion/src/context.rs
@@ -92,6 +92,7 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) has_item_list_or_source_file_parent: bool,
     pub(super) for_is_prev2: bool,
     pub(super) fn_is_prev: bool,
+    pub(super) incomplete_let: bool,
     pub(super) locals: Vec<(String, Local)>,
 }
 
@@ -132,9 +133,9 @@ impl<'a> CompletionContext<'a> {
             scope,
             db,
             config,
+            position,
             original_token,
             token,
-            position,
             krate,
             expected_type: None,
             name_ref_syntax: None,
@@ -155,30 +156,31 @@ impl<'a> CompletionContext<'a> {
             is_expr: false,
             is_new_item: false,
             dot_receiver: None,
+            dot_receiver_is_ambiguous_float_literal: false,
             is_call: false,
             is_pattern_call: false,
             is_macro_call: false,
             is_path_type: false,
             has_type_args: false,
-            dot_receiver_is_ambiguous_float_literal: false,
             attribute_under_caret: None,
             mod_declaration_under_caret: None,
             unsafe_is_prev: false,
-            in_loop_body: false,
-            ref_pat_parent: false,
-            bind_pat_parent: false,
+            if_is_prev: false,
             block_expr_parent: false,
+            bind_pat_parent: false,
+            ref_pat_parent: false,
+            in_loop_body: false,
             has_trait_parent: false,
             has_impl_parent: false,
             inside_impl_trait_block: false,
             has_field_list_parent: false,
             trait_as_prev_sibling: false,
             impl_as_prev_sibling: false,
-            if_is_prev: false,
             is_match_arm: false,
             has_item_list_or_source_file_parent: false,
             for_is_prev2: false,
             fn_is_prev: false,
+            incomplete_let: false,
             locals,
         };
 
@@ -270,6 +272,10 @@ impl<'a> CompletionContext<'a> {
                 .filter(|module| module.item_list().is_none());
         self.for_is_prev2 = for_is_prev2(syntax_element.clone());
         self.fn_is_prev = fn_is_prev(syntax_element.clone());
+        self.incomplete_let =
+            syntax_element.ancestors().take(6).find_map(ast::LetStmt::cast).map_or(false, |it| {
+                it.syntax().text_range().end() == syntax_element.text_range().end()
+            });
     }
 
     fn fill(