Merge #367

367: add `;` to last return in block r=matklad a=matklad



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

2 files changed, 46 insertions, 15 deletions

diff --git a/crates/ra_analysis/src/completion/complete_keyword.rs b/crates/ra_analysis/src/completion/complete_keyword.rs
index d1e0a20a8..2869e67e0 100644
--- a/crates/ra_analysis/src/completion/complete_keyword.rs
+++ b/crates/ra_analysis/src/completion/complete_keyword.rs
@@ -35,7 +35,7 @@ pub(super) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         acc.add(keyword("continue", "continue"));
         acc.add(keyword("break", "break"));
     }
-    acc.add_all(complete_return(fn_def, ctx.is_stmt));
+    acc.add_all(complete_return(fn_def, ctx.can_be_stmt));
 }
 
 fn is_in_loop_body(leaf: SyntaxNodeRef) -> bool {
@@ -57,8 +57,8 @@ fn is_in_loop_body(leaf: SyntaxNodeRef) -> bool {
     false
 }
 
-fn complete_return(fn_def: ast::FnDef, is_stmt: bool) -> Option<CompletionItem> {
-    let snip = match (is_stmt, fn_def.ret_type().is_some()) {
+fn complete_return(fn_def: ast::FnDef, 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",
@@ -75,7 +75,7 @@ mod tests {
     }
 
     #[test]
-    fn test_completion_kewords() {
+    fn completes_various_keywords_in_function() {
         check_keyword_completion(
             r"
             fn quux() {
@@ -87,13 +87,13 @@ mod tests {
             match "match $0 {}"
             while "while $0 {}"
             loop "loop {$0}"
-            return "return"
+            return "return;"
             "#,
         );
     }
 
     #[test]
-    fn test_completion_else() {
+    fn completes_else_after_if() {
         check_keyword_completion(
             r"
             fn quux() {
@@ -109,7 +109,7 @@ mod tests {
             loop "loop {$0}"
             else "else {$0}"
             else if "else if $0 {}"
-            return "return"
+            return "return;"
             "#,
         );
     }
@@ -149,7 +149,7 @@ mod tests {
     }
 
     #[test]
-    fn test_completion_return_no_stmt() {
+    fn dont_add_semi_after_return_if_not_a_statement() {
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -169,7 +169,27 @@ mod tests {
     }
 
     #[test]
-    fn test_continue_break_completion() {
+    fn last_return_in_block_has_semi() {
+        check_keyword_completion(
+            r"
+            fn quux() -> i32 {
+                if condition {
+                    <|>
+                }
+            }
+            ",
+            r#"
+            if "if $0 {}"
+            match "match $0 {}"
+            while "while $0 {}"
+            loop "loop {$0}"
+            return "return $0;"
+            "#,
+        );
+    }
+
+    #[test]
+    fn completes_break_and_continue_in_loops() {
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -183,9 +203,10 @@ mod tests {
             loop "loop {$0}"
             continue "continue"
             break "break"
-            return "return $0"
+            return "return $0;"
             "#,
         );
+        // No completion: lambda isolates control flow
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -197,7 +218,7 @@ mod tests {
             match "match $0 {}"
             while "while $0 {}"
             loop "loop {$0}"
-            return "return $0"
+            return "return $0;"
             "#,
         );
     }
diff --git a/crates/ra_analysis/src/completion/completion_context.rs b/crates/ra_analysis/src/completion/completion_context.rs
index 949b8135e..4685c9328 100644
--- a/crates/ra_analysis/src/completion/completion_context.rs
+++ b/crates/ra_analysis/src/completion/completion_context.rs
@@ -31,7 +31,8 @@ pub(super) struct CompletionContext<'a> {
     /// If not a trivial, path, the prefix (qualifier).
     pub(super) path_prefix: Option<hir::Path>,
     pub(super) after_if: bool,
-    pub(super) is_stmt: bool,
+    /// `true` if we are a statement or a last expr in the block.
+    pub(super) can_be_stmt: bool,
     /// Something is typed at the "top" level, in module or impl/trait.
     pub(super) is_new_item: bool,
     /// The receiver if this is a field or method access, i.e. writing something.<|>
@@ -61,7 +62,7 @@ impl<'a> CompletionContext<'a> {
             is_trivial_path: false,
             path_prefix: None,
             after_if: false,
-            is_stmt: false,
+            can_be_stmt: false,
             is_new_item: false,
             dot_receiver: None,
             is_method_call: false,
@@ -147,13 +148,22 @@ impl<'a> CompletionContext<'a> {
             if path.qualifier().is_none() {
                 self.is_trivial_path = true;
 
-                self.is_stmt = match name_ref
+                self.can_be_stmt = match name_ref
                     .syntax()
                     .ancestors()
                     .filter_map(ast::ExprStmt::cast)
                     .next()
                 {
-                    None => false,
+                    None => {
+                        name_ref
+                            .syntax()
+                            .ancestors()
+                            .filter_map(ast::Block::cast)
+                            .next()
+                            .and_then(|block| block.expr())
+                            .map(|e| e.syntax().range())
+                            == Some(name_ref.syntax().range())
+                    }
                     Some(expr_stmt) => expr_stmt.syntax().range() == name_ref.syntax().range(),
                 };