6 files changed, 270 insertions, 35 deletions

diff --git a/crates/ra_assists/src/ast_transform.rs b/crates/ra_assists/src/ast_transform.rs
index 9ac65ab39..3079a02a2 100644
--- a/crates/ra_assists/src/ast_transform.rs
+++ b/crates/ra_assists/src/ast_transform.rs
@@ -1,7 +1,7 @@
 //! `AstTransformer`s are functions that replace nodes in an AST and can be easily combined.
 use rustc_hash::FxHashMap;
 
-use hir::{PathResolution, SemanticsScope};
+use hir::{HirDisplay, PathResolution, SemanticsScope};
 use ra_ide_db::RootDatabase;
 use ra_syntax::{
     algo::SyntaxRewriter,
@@ -51,7 +51,27 @@ impl<'a> SubstituteTypeParams<'a> {
             .into_iter()
             // this is a trait impl, so we need to skip the first type parameter -- this is a bit hacky
             .skip(1)
-            .zip(substs.into_iter())
+            // The actual list of trait type parameters may be longer than the one
+            // used in the `impl` block due to trailing default type parametrs.
+            // For that case we extend the `substs` with an empty iterator so we
+            // can still hit those trailing values and check if they actually have
+            // a default type. If they do, go for that type from `hir` to `ast` so
+            // the resulting change can be applied correctly.
+            .zip(substs.into_iter().map(Some).chain(std::iter::repeat(None)))
+            .filter_map(|(k, v)| match v {
+                Some(v) => Some((k, v)),
+                None => {
+                    let default = k.default(source_scope.db)?;
+                    Some((
+                        k,
+                        ast::make::type_ref(
+                            &default
+                                .display_source_code(source_scope.db, source_scope.module()?.into())
+                                .ok()?,
+                        ),
+                    ))
+                }
+            })
             .collect();
         return SubstituteTypeParams {
             source_scope,
diff --git a/crates/ra_assists/src/handlers/add_custom_impl.rs b/crates/ra_assists/src/handlers/add_custom_impl.rs
index 795a225a4..2baeb8607 100644
--- a/crates/ra_assists/src/handlers/add_custom_impl.rs
+++ b/crates/ra_assists/src/handlers/add_custom_impl.rs
@@ -49,7 +49,7 @@ pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<
     let start_offset = annotated.syntax().parent()?.text_range().end();
 
     let label =
-        format!("Add custom impl '{}' for '{}'", trait_token.text().as_str(), annotated_name);
+        format!("Add custom impl `{}` for `{}`", trait_token.text().as_str(), annotated_name);
 
     let target = attr.syntax().text_range();
     acc.add(AssistId("add_custom_impl"), label, target, |edit| {
diff --git a/crates/ra_assists/src/handlers/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs
index 146cc75df..0c7d5e355 100644
--- a/crates/ra_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ra_assists/src/handlers/add_explicit_type.rs
@@ -61,7 +61,7 @@ pub(crate) fn add_explicit_type(acc: &mut Assists, ctx: &AssistContext) -> Optio
     let inferred_type = ty.display_source_code(ctx.db, module.into()).ok()?;
     acc.add(
         AssistId("add_explicit_type"),
-        format!("Insert explicit type '{}'", inferred_type),
+        format!("Insert explicit type `{}`", inferred_type),
         pat_range,
         |builder| match ascribed_ty {
             Some(ascribed_ty) => {
@@ -209,7 +209,7 @@ struct Test<K, T = u8> {
 }
 
 fn main() {
-    let test<|>: Test<i32, u8> = Test { t: 23, k: 33 };
+    let test<|>: Test<i32> = Test { t: 23, k: 33 };
 }"#,
         );
     }
diff --git a/crates/ra_assists/src/handlers/add_missing_impl_members.rs b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
index c1ce87914..22e1156d2 100644
--- a/crates/ra_assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/ra_assists/src/handlers/add_missing_impl_members.rs
@@ -618,4 +618,54 @@ impl Foo for S {
 }"#,
         )
     }
+
+    #[test]
+    fn test_generic_single_default_parameter() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo<T = Self> {
+    fn bar(&self, other: &T);
+}
+
+struct S;
+impl Foo for S { <|> }"#,
+            r#"
+trait Foo<T = Self> {
+    fn bar(&self, other: &T);
+}
+
+struct S;
+impl Foo for S {
+    <|>fn bar(&self, other: &Self) {
+        todo!()
+    }
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_generic_default_parameter_is_second() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Foo<T1, T2 = Self> {
+    fn bar(&self, this: &T1, that: &T2);
+}
+
+struct S<T>;
+impl Foo<T> for S<T> { <|> }"#,
+            r#"
+trait Foo<T1, T2 = Self> {
+    fn bar(&self, this: &T1, that: &T2);
+}
+
+struct S<T>;
+impl Foo<T> for S<T> {
+    <|>fn bar(&self, this: &T, that: &Self) {
+        todo!()
+    }
+}"#,
+        )
+    }
 }
diff --git a/crates/ra_assists/src/handlers/unwrap_block.rs b/crates/ra_assists/src/handlers/unwrap_block.rs
index eba0631a4..e52ec557e 100644
--- a/crates/ra_assists/src/handlers/unwrap_block.rs
+++ b/crates/ra_assists/src/handlers/unwrap_block.rs
@@ -1,6 +1,6 @@
 use crate::{AssistContext, AssistId, Assists};
 
-use ast::LoopBodyOwner;
+use ast::{ElseBranch, Expr, LoopBodyOwner};
 use ra_fmt::unwrap_trivial_block;
 use ra_syntax::{ast, match_ast, AstNode, TextRange, T};
 
@@ -25,19 +25,11 @@ pub(crate) fn unwrap_block(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
     let l_curly_token = ctx.find_token_at_offset(T!['{'])?;
     let block = ast::BlockExpr::cast(l_curly_token.parent())?;
     let parent = block.syntax().parent()?;
+    let assist_id = AssistId("unwrap_block");
+    let assist_label = "Unwrap block";
+
     let (expr, expr_to_unwrap) = match_ast! {
         match parent {
-            ast::IfExpr(if_expr) => {
-                let expr_to_unwrap = if_expr.blocks().find_map(|expr| extract_expr(ctx.frange.range, expr));
-                let expr_to_unwrap = expr_to_unwrap?;
-                // Find if we are in a else if block
-                let ancestor = if_expr.syntax().parent().and_then(ast::IfExpr::cast);
-
-                match ancestor {
-                    None => (ast::Expr::IfExpr(if_expr), expr_to_unwrap),
-                    Some(ancestor) => (ast::Expr::IfExpr(ancestor), expr_to_unwrap),
-                }
-            },
             ast::ForExpr(for_expr) => {
                 let block_expr = for_expr.loop_body()?;
                 let expr_to_unwrap = extract_expr(ctx.frange.range, block_expr)?;
@@ -53,27 +45,62 @@ pub(crate) fn unwrap_block(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
                 let expr_to_unwrap = extract_expr(ctx.frange.range, block_expr)?;
                 (ast::Expr::LoopExpr(loop_expr), expr_to_unwrap)
             },
+            ast::IfExpr(if_expr) => {
+                let mut resp = None;
+
+                let then_branch = if_expr.then_branch()?;
+                if then_branch.l_curly_token()?.text_range().contains_range(ctx.frange.range) {
+                    if let Some(ancestor) = if_expr.syntax().parent().and_then(ast::IfExpr::cast) {
+                        // For `else if` blocks
+                        let ancestor_then_branch = ancestor.then_branch()?;
+                        let l_curly_token = then_branch.l_curly_token()?;
+
+                        let target = then_branch.syntax().text_range();
+                        return acc.add(assist_id, assist_label, target, |edit| {
+                            let range_to_del_else_if = TextRange::new(ancestor_then_branch.syntax().text_range().end(), l_curly_token.text_range().start());
+                            let range_to_del_rest = TextRange::new(then_branch.syntax().text_range().end(), if_expr.syntax().text_range().end());
+
+                            edit.set_cursor(ancestor_then_branch.syntax().text_range().end());
+                            edit.delete(range_to_del_rest);
+                            edit.delete(range_to_del_else_if);
+                            edit.replace(target, update_expr_string(then_branch.to_string(), &[' ', '{']));
+                        });
+                    } else {
+                        resp = Some((ast::Expr::IfExpr(if_expr.clone()), Expr::BlockExpr(then_branch)));
+                    }
+                } else if let Some(else_branch) = if_expr.else_branch() {
+                    match else_branch {
+                        ElseBranch::Block(else_block) => {
+                            let l_curly_token = else_block.l_curly_token()?;
+                            if l_curly_token.text_range().contains_range(ctx.frange.range) {
+                                let target = else_block.syntax().text_range();
+                                return acc.add(assist_id, assist_label, target, |edit| {
+                                    let range_to_del = TextRange::new(then_branch.syntax().text_range().end(), l_curly_token.text_range().start());
+
+                                    edit.set_cursor(then_branch.syntax().text_range().end());
+                                    edit.delete(range_to_del);
+                                    edit.replace(target, update_expr_string(else_block.to_string(), &[' ', '{']));
+                                });
+                            }
+                        },
+                        ElseBranch::IfExpr(_) => {},
+                    }
+                }
+
+                resp?
+            },
             _ => return None,
         }
     };
 
     let target = expr_to_unwrap.syntax().text_range();
-    acc.add(AssistId("unwrap_block"), "Unwrap block", target, |edit| {
+    acc.add(assist_id, assist_label, target, |edit| {
         edit.set_cursor(expr.syntax().text_range().start());
 
-        let pat_start: &[_] = &[' ', '{', '\n'];
-        let expr_to_unwrap = expr_to_unwrap.to_string();
-        let expr_string = expr_to_unwrap.trim_start_matches(pat_start);
-        let mut expr_string_lines: Vec<&str> = expr_string.lines().collect();
-        expr_string_lines.pop(); // Delete last line
-
-        let expr_string = expr_string_lines
-            .into_iter()
-            .map(|line| line.replacen("    ", "", 1)) // Delete indentation
-            .collect::<Vec<String>>()
-            .join("\n");
-
-        edit.replace(expr.syntax().text_range(), expr_string);
+        edit.replace(
+            expr.syntax().text_range(),
+            update_expr_string(expr_to_unwrap.to_string(), &[' ', '{', '\n']),
+        );
     })
 }
 
@@ -87,6 +114,18 @@ fn extract_expr(cursor_range: TextRange, block: ast::BlockExpr) -> Option<ast::E
     }
 }
 
+fn update_expr_string(expr_str: String, trim_start_pat: &[char]) -> String {
+    let expr_string = expr_str.trim_start_matches(trim_start_pat);
+    let mut expr_string_lines: Vec<&str> = expr_string.lines().collect();
+    expr_string_lines.pop(); // Delete last line
+
+    expr_string_lines
+        .into_iter()
+        .map(|line| line.replacen("    ", "", 1)) // Delete indentation
+        .collect::<Vec<String>>()
+        .join("\n")
+}
+
 #[cfg(test)]
 mod tests {
     use crate::tests::{check_assist, check_assist_not_applicable};
@@ -142,7 +181,13 @@ mod tests {
             r#"
             fn main() {
                 bar();
-                <|>println!("bar");
+                if true {
+                    foo();
+
+                    //comment
+                    bar();
+                }<|>
+                println!("bar");
             }
             "#,
         );
@@ -170,7 +215,127 @@ mod tests {
             r#"
             fn main() {
                 //bar();
-                <|>println!("bar");
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                }<|>
+                println!("bar");
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn simple_if_else_if_nested() {
+        check_assist(
+            unwrap_block,
+            r#"
+            fn main() {
+                //bar();
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                } else if false {
+                    println!("bar");
+                } else if true {<|>
+                    println!("foo");
+                }
+            }
+            "#,
+            r#"
+            fn main() {
+                //bar();
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                } else if false {
+                    println!("bar");
+                }<|>
+                println!("foo");
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn simple_if_else_if_nested_else() {
+        check_assist(
+            unwrap_block,
+            r#"
+            fn main() {
+                //bar();
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                } else if false {
+                    println!("bar");
+                } else if true {
+                    println!("foo");
+                } else {<|>
+                    println!("else");
+                }
+            }
+            "#,
+            r#"
+            fn main() {
+                //bar();
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                } else if false {
+                    println!("bar");
+                } else if true {
+                    println!("foo");
+                }<|>
+                println!("else");
+            }
+            "#,
+        );
+    }
+
+    #[test]
+    fn simple_if_else_if_nested_middle() {
+        check_assist(
+            unwrap_block,
+            r#"
+            fn main() {
+                //bar();
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                } else if false {
+                    println!("bar");
+                } else if true {<|>
+                    println!("foo");
+                } else {
+                    println!("else");
+                }
+            }
+            "#,
+            r#"
+            fn main() {
+                //bar();
+                if true {
+                    println!("true");
+
+                    //comment
+                    //bar();
+                } else if false {
+                    println!("bar");
+                }<|>
+                println!("foo");
             }
             "#,
         );
diff --git a/crates/ra_assists/src/utils.rs b/crates/ra_assists/src/utils.rs
index 2f15a3f15..f3fc92ebf 100644
--- a/crates/ra_assists/src/utils.rs
+++ b/crates/ra_assists/src/utils.rs
@@ -103,7 +103,7 @@ fn invert_special_case(expr: &ast::Expr) -> Option<ast::Expr> {
 }
 
 #[derive(Clone, Copy)]
-pub(crate) enum TryEnum {
+pub enum TryEnum {
     Result,
     Option,
 }
@@ -111,7 +111,7 @@ pub(crate) enum TryEnum {
 impl TryEnum {
     const ALL: [TryEnum; 2] = [TryEnum::Option, TryEnum::Result];
 
-    pub(crate) fn from_ty(sema: &Semantics<RootDatabase>, ty: &Type) -> Option<TryEnum> {
+    pub fn from_ty(sema: &Semantics<RootDatabase>, ty: &Type) -> Option<TryEnum> {
         let enum_ = match ty.as_adt() {
             Some(Adt::Enum(it)) => it,
             _ => return None,