1 files changed, 147 insertions, 2 deletions

diff --git a/crates/ra_ide_api_light/src/join_lines.rs b/crates/ra_ide_api_light/src/join_lines.rs
index 949ee1544..9f59fbd43 100644
--- a/crates/ra_ide_api_light/src/join_lines.rs
+++ b/crates/ra_ide_api_light/src/join_lines.rs
@@ -120,11 +120,47 @@ fn remove_newline(
     }
 }
 
+/// fixes a comma after the given expression and optionally inserts a new trailing comma
+/// if no comma was found and `comma_offset` is provided
+fn fix_comma_after(edit: &mut TextEditBuilder, node: &SyntaxNode, comma_offset: Option<TextUnit>) {
+    let next = node.next_sibling();
+    let nnext = node.next_sibling().and_then(|n| n.next_sibling());
+
+    match (next, nnext) {
+        // Whitespace followed by a comma
+        // remove the whitespace
+        (Some(ws), Some(comma)) if ws.kind() == WHITESPACE && comma.kind() == COMMA => {
+            edit.delete(ws.range());
+        }
+
+        // if we are not a comma and if comma_offset was provided,
+        // insert trailing comma after the block
+        (Some(n), _) if n.kind() != COMMA => {
+            if let Some(comma_offset) = comma_offset {
+                edit.insert(comma_offset, ",".to_owned());
+            }
+        }
+
+        _ => {}
+    }
+}
+
 fn join_single_expr_block(edit: &mut TextEditBuilder, node: &SyntaxNode) -> Option<()> {
     let block = ast::Block::cast(node.parent()?)?;
     let block_expr = ast::BlockExpr::cast(block.syntax().parent()?)?;
     let expr = extract_trivial_expression(block)?;
-    edit.replace(block_expr.syntax().range(), expr.syntax().text().to_string());
+
+    let block_range = block_expr.syntax().range();
+    edit.replace(block_range, expr.syntax().text().to_string());
+
+    // Match block needs to have a comma after the block
+    // otherwise we'll maintain a comma after the block if such existed
+    // but we remove excess whitespace between the expression and the comma.
+    if let Some(match_arm) = block_expr.syntax().parent().and_then(ast::MatchArm::cast) {
+        fix_comma_after(edit, match_arm.syntax(), Some(block_range.end()));
+    } else {
+        fix_comma_after(edit, block_expr.syntax(), None);
+    }
     Some(())
 }
 
@@ -208,7 +244,6 @@ fn foo() {
     }
 
     #[test]
-    #[ignore] // FIXME: https://github.com/rust-analyzer/rust-analyzer/issues/868
     fn join_lines_adds_comma_for_block_in_match_arm() {
         check_join_lines(
             r"
@@ -231,6 +266,116 @@ fn foo(e: Result<U, V>) {
     }
 
     #[test]
+    fn join_lines_keeps_comma_for_block_in_match_arm() {
+        // We already have a comma
+        check_join_lines(
+            r"
+fn foo(e: Result<U, V>) {
+    match e {
+        Ok(u) => <|>{
+            u.foo()
+        },
+        Err(v) => v,
+    }
+}",
+            r"
+fn foo(e: Result<U, V>) {
+    match e {
+        Ok(u) => <|>u.foo(),
+        Err(v) => v,
+    }
+}",
+        );
+
+        // comma with whitespace between brace and ,
+        check_join_lines(
+            r"
+fn foo(e: Result<U, V>) {
+    match e {
+        Ok(u) => <|>{
+            u.foo()
+        }    ,
+        Err(v) => v,
+    }
+}",
+            r"
+fn foo(e: Result<U, V>) {
+    match e {
+        Ok(u) => <|>u.foo(),
+        Err(v) => v,
+    }
+}",
+        );
+
+        // comma with newline between brace and ,
+        check_join_lines(
+            r"
+fn foo(e: Result<U, V>) {
+    match e {
+        Ok(u) => <|>{
+            u.foo()
+        }
+        ,
+        Err(v) => v,
+    }
+}",
+            r"
+fn foo(e: Result<U, V>) {
+    match e {
+        Ok(u) => <|>u.foo(),
+        Err(v) => v,
+    }
+}",
+        );
+    }
+
+    #[test]
+    fn join_lines_keeps_comma_with_single_arg_tuple() {
+        // A single arg tuple
+        check_join_lines(
+            r"
+fn foo() {
+    let x = (<|>{
+       4
+    },);
+}",
+            r"
+fn foo() {
+    let x = (<|>4,);
+}",
+        );
+
+        // single arg tuple with whitespace between brace and comma
+        check_join_lines(
+            r"
+fn foo() {
+    let x = (<|>{
+       4
+    }   ,);
+}",
+            r"
+fn foo() {
+    let x = (<|>4,);
+}",
+        );
+
+        // single arg tuple with newline between brace and comma
+        check_join_lines(
+            r"
+fn foo() {
+    let x = (<|>{
+       4
+    }
+    ,);
+}",
+            r"
+fn foo() {
+    let x = (<|>4,);
+}",
+        );
+    }
+
+    #[test]
     fn test_join_lines_use_items_left() {
         // No space after the '{'
         check_join_lines(