10 files changed, 417 insertions, 55 deletions

diff --git a/crates/assists/src/ast_transform.rs b/crates/assists/src/ast_transform.rs
index da94e9987..4a3ed7783 100644
--- a/crates/assists/src/ast_transform.rs
+++ b/crates/assists/src/ast_transform.rs
@@ -204,7 +204,8 @@ impl<'a> AstTransform<'a> for QualifyPaths<'a> {
             }
             PathResolution::Local(_)
             | PathResolution::TypeParam(_)
-            | PathResolution::SelfType(_) => None,
+            | PathResolution::SelfType(_)
+            | PathResolution::ConstParam(_) => None,
             PathResolution::Macro(_) => None,
             PathResolution::AssocItem(_) => None,
         }
diff --git a/crates/assists/src/handlers/extract_assignment.rs b/crates/assists/src/handlers/extract_assignment.rs
new file mode 100644
index 000000000..281cf5d24
--- /dev/null
+++ b/crates/assists/src/handlers/extract_assignment.rs
@@ -0,0 +1,325 @@
+use hir::AsName;
+use syntax::{
+    ast::{self, edit::AstNodeEdit, make},
+    AstNode,
+};
+use test_utils::mark;
+
+use crate::{
+    assist_context::{AssistContext, Assists},
+    AssistId, AssistKind,
+};
+
+// Assist: extract_assignment
+//
+// Extracts variable assigment to outside an if or match statement.
+//
+// ```
+// fn main() {
+//     let mut foo = 6;
+//
+//     if true {
+//         <|>foo = 5;
+//     } else {
+//         foo = 4;
+//     }
+// }
+// ```
+// ->
+// ```
+// fn main() {
+//     let mut foo = 6;
+//
+//     foo = if true {
+//         5
+//     } else {
+//         4
+//     };
+// }
+// ```
+pub(crate) fn extract_assigment(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+    let name = ctx.find_node_at_offset::<ast::NameRef>()?.as_name();
+
+    let (old_stmt, new_stmt) = if let Some(if_expr) = ctx.find_node_at_offset::<ast::IfExpr>() {
+        (
+            ast::Expr::cast(if_expr.syntax().to_owned())?,
+            exprify_if(&if_expr, &name)?.indent(if_expr.indent_level()),
+        )
+    } else if let Some(match_expr) = ctx.find_node_at_offset::<ast::MatchExpr>() {
+        (ast::Expr::cast(match_expr.syntax().to_owned())?, exprify_match(&match_expr, &name)?)
+    } else {
+        return None;
+    };
+
+    let expr_stmt = make::expr_stmt(new_stmt);
+
+    acc.add(
+        AssistId("extract_assignment", AssistKind::RefactorExtract),
+        "Extract assignment",
+        old_stmt.syntax().text_range(),
+        move |edit| {
+            edit.replace(old_stmt.syntax().text_range(), format!("{} = {};", name, expr_stmt));
+        },
+    )
+}
+
+fn exprify_match(match_expr: &ast::MatchExpr, name: &hir::Name) -> Option<ast::Expr> {
+    let new_arm_list = match_expr
+        .match_arm_list()?
+        .arms()
+        .map(|arm| {
+            if let ast::Expr::BlockExpr(block) = arm.expr()? {
+                let new_block = exprify_block(&block, name)?.indent(block.indent_level());
+                Some(arm.replace_descendant(block, new_block))
+            } else {
+                None
+            }
+        })
+        .collect::<Option<Vec<_>>>()?;
+    let new_arm_list = match_expr
+        .match_arm_list()?
+        .replace_descendants(match_expr.match_arm_list()?.arms().zip(new_arm_list));
+    Some(make::expr_match(match_expr.expr()?, new_arm_list))
+}
+
+fn exprify_if(statement: &ast::IfExpr, name: &hir::Name) -> Option<ast::Expr> {
+    let then_branch = exprify_block(&statement.then_branch()?, name)?;
+    let else_branch = match statement.else_branch()? {
+        ast::ElseBranch::Block(ref block) => ast::ElseBranch::Block(exprify_block(block, name)?),
+        ast::ElseBranch::IfExpr(expr) => {
+            mark::hit!(test_extract_assigment_chained_if);
+            ast::ElseBranch::IfExpr(ast::IfExpr::cast(
+                exprify_if(&expr, name)?.syntax().to_owned(),
+            )?)
+        }
+    };
+    Some(make::expr_if(statement.condition()?, then_branch, Some(else_branch)))
+}
+
+fn exprify_block(block: &ast::BlockExpr, name: &hir::Name) -> Option<ast::BlockExpr> {
+    if block.expr().is_some() {
+        return None;
+    }
+
+    let mut stmts: Vec<_> = block.statements().collect();
+    let stmt = stmts.pop()?;
+
+    if let ast::Stmt::ExprStmt(stmt) = stmt {
+        if let ast::Expr::BinExpr(expr) = stmt.expr()? {
+            if expr.op_kind()? == ast::BinOp::Assignment
+                && &expr.lhs()?.name_ref()?.as_name() == name
+            {
+                // The last statement in the block is an assignment to the name we want
+                return Some(make::block_expr(stmts, Some(expr.rhs()?)));
+            }
+        }
+    }
+    None
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use crate::tests::{check_assist, check_assist_not_applicable};
+
+    #[test]
+    fn test_extract_assignment_if() {
+        check_assist(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    if true {
+        <|>a = 2;
+    } else {
+        a = 3;
+    }
+}"#,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    a = if true {
+        2
+    } else {
+        3
+    };
+}"#,
+        );
+    }
+
+    #[test]
+    fn test_extract_assignment_match() {
+        check_assist(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    match 1 {
+        1 => {
+            <|>a = 2;
+        },
+        2 => {
+            a = 3;
+        },
+        3 => {
+            a = 4;
+        }
+    }
+}"#,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    a = match 1 {
+        1 => {
+            2
+        },
+        2 => {
+            3
+        },
+        3 => {
+            4
+        }
+    };
+}"#,
+        );
+    }
+
+    #[test]
+    fn test_extract_assignment_not_last_not_applicable() {
+        check_assist_not_applicable(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    if true {
+        <|>a = 2;
+        b = a;
+    } else {
+        a = 3;
+    }
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_extract_assignment_chained_if() {
+        mark::check!(test_extract_assigment_chained_if);
+        check_assist(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    if true {
+        <|>a = 2;
+    } else if false {
+        a = 3;
+    } else {
+        a = 4;
+    }
+}"#,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    a = if true {
+        2
+    } else if false {
+        3
+    } else {
+        4
+    };
+}"#,
+        );
+    }
+
+    #[test]
+    fn test_extract_assigment_retains_stmts() {
+        check_assist(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    if true {
+        let b = 2;
+        <|>a = 2;
+    } else {
+        let b = 3;
+        a = 3;
+    }
+}"#,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    a = if true {
+        let b = 2;
+        2
+    } else {
+        let b = 3;
+        3
+    };
+}"#,
+        )
+    }
+
+    #[test]
+    fn extract_assignment_let_stmt_not_applicable() {
+        check_assist_not_applicable(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    let b = if true {
+        <|>a = 2
+    } else {
+        a = 3
+    };
+}"#,
+        )
+    }
+
+    #[test]
+    fn extract_assignment_if_missing_assigment_not_applicable() {
+        check_assist_not_applicable(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    if true {
+        <|>a = 2;
+    } else {}
+}"#,
+        )
+    }
+
+    #[test]
+    fn extract_assignment_match_missing_assigment_not_applicable() {
+        check_assist_not_applicable(
+            extract_assigment,
+            r#"
+fn foo() {
+    let mut a = 1;
+
+    match 1 {
+        1 => {
+            <|>a = 2;
+        },
+        2 => {
+            a = 3;
+        },
+        3 => {},
+    }
+}"#,
+        )
+    }
+}
diff --git a/crates/assists/src/handlers/extract_variable.rs b/crates/assists/src/handlers/extract_variable.rs
index d2ae137cd..9957012fe 100644
--- a/crates/assists/src/handlers/extract_variable.rs
+++ b/crates/assists/src/handlers/extract_variable.rs
@@ -91,7 +91,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option
             // extra newlines in the indent block
             let text = indent.text();
             if text.starts_with('\n') {
-                buf.push_str("\n");
+                buf.push('\n');
                 buf.push_str(text.trim_start_matches('\n'));
             } else {
                 buf.push_str(text);
diff --git a/crates/assists/src/handlers/fill_match_arms.rs b/crates/assists/src/handlers/fill_match_arms.rs
index cb60a3128..f9a62b9fa 100644
--- a/crates/assists/src/handlers/fill_match_arms.rs
+++ b/crates/assists/src/handlers/fill_match_arms.rs
@@ -196,7 +196,7 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: hir::Variant) -> Optio
     let path = mod_path_to_ast(&module.find_use_path(db, ModuleDef::from(var))?);
 
     // FIXME: use HIR for this; it doesn't currently expose struct vs. tuple vs. unit variants though
-    let pat: ast::Pat = match var.source(db).value.kind() {
+    let pat: ast::Pat = match var.source(db)?.value.kind() {
         ast::StructKind::Tuple(field_list) => {
             let pats = iter::repeat(make::wildcard_pat().into()).take(field_list.fields().count());
             make::tuple_struct_pat(path, pats).into()
diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs
index 8558a8ff0..de1e8f0bf 100644
--- a/crates/assists/src/handlers/fix_visibility.rs
+++ b/crates/assists/src/handlers/fix_visibility.rs
@@ -97,7 +97,8 @@ fn add_vis_to_referenced_record_field(acc: &mut Assists, ctx: &AssistContext) ->
     let parent_name = parent.name(ctx.db());
     let target_module = parent.module(ctx.db());
 
-    let in_file_source = record_field_def.source(ctx.db());
+    #[allow(deprecated)]
+    let in_file_source = record_field_def.source(ctx.db())?;
     let (offset, current_visibility, target) = match in_file_source.value {
         hir::FieldSource::Named(it) => {
             let s = it.syntax();
@@ -145,53 +146,53 @@ fn target_data_for_def(
     fn offset_target_and_file_id<S, Ast>(
         db: &dyn HirDatabase,
         x: S,
-    ) -> (TextSize, Option<ast::Visibility>, TextRange, FileId)
+    ) -> Option<(TextSize, Option<ast::Visibility>, TextRange, FileId)>
     where
         S: HasSource<Ast = Ast>,
         Ast: AstNode + ast::VisibilityOwner,
     {
-        let source = x.source(db);
+        let source = x.source(db)?;
         let in_file_syntax = source.syntax();
         let file_id = in_file_syntax.file_id;
         let syntax = in_file_syntax.value;
         let current_visibility = source.value.visibility();
-        (
+        Some((
             vis_offset(syntax),
             current_visibility,
             syntax.text_range(),
             file_id.original_file(db.upcast()),
-        )
+        ))
     }
 
     let target_name;
     let (offset, current_visibility, target, target_file) = match def {
         hir::ModuleDef::Function(f) => {
             target_name = Some(f.name(db));
-            offset_target_and_file_id(db, f)
+            offset_target_and_file_id(db, f)?
         }
         hir::ModuleDef::Adt(adt) => {
             target_name = Some(adt.name(db));
             match adt {
-                hir::Adt::Struct(s) => offset_target_and_file_id(db, s),
-                hir::Adt::Union(u) => offset_target_and_file_id(db, u),
-                hir::Adt::Enum(e) => offset_target_and_file_id(db, e),
+                hir::Adt::Struct(s) => offset_target_and_file_id(db, s)?,
+                hir::Adt::Union(u) => offset_target_and_file_id(db, u)?,
+                hir::Adt::Enum(e) => offset_target_and_file_id(db, e)?,
             }
         }
         hir::ModuleDef::Const(c) => {
             target_name = c.name(db);
-            offset_target_and_file_id(db, c)
+            offset_target_and_file_id(db, c)?
         }
         hir::ModuleDef::Static(s) => {
             target_name = s.name(db);
-            offset_target_and_file_id(db, s)
+            offset_target_and_file_id(db, s)?
         }
         hir::ModuleDef::Trait(t) => {
             target_name = Some(t.name(db));
-            offset_target_and_file_id(db, t)
+            offset_target_and_file_id(db, t)?
         }
         hir::ModuleDef::TypeAlias(t) => {
             target_name = Some(t.name(db));
-            offset_target_and_file_id(db, t)
+            offset_target_and_file_id(db, t)?
         }
         hir::ModuleDef::Module(m) => {
             target_name = m.name(db);
diff --git a/crates/assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs
index 4d6a1956b..cb7a5c104 100644
--- a/crates/assists/src/handlers/replace_derive_with_manual_impl.rs
+++ b/crates/assists/src/handlers/replace_derive_with_manual_impl.rs
@@ -62,21 +62,22 @@ pub(crate) fn replace_derive_with_manual_impl(
     let current_module = ctx.sema.scope(annotated_name.syntax()).module()?;
     let current_crate = current_module.krate();
 
-    let found_traits =
-        imports_locator::find_exact_imports(&ctx.sema, current_crate, trait_token.text())
-            .filter_map(
-                |candidate: either::Either<hir::ModuleDef, hir::MacroDef>| match candidate {
-                    either::Either::Left(hir::ModuleDef::Trait(trait_)) => Some(trait_),
-                    _ => None,
-                },
-            )
-            .flat_map(|trait_| {
-                current_module
-                    .find_use_path(ctx.sema.db, hir::ModuleDef::Trait(trait_))
-                    .as_ref()
-                    .map(mod_path_to_ast)
-                    .zip(Some(trait_))
-            });
+    let found_traits = imports_locator::find_exact_imports(
+        &ctx.sema,
+        current_crate,
+        trait_token.text().to_string(),
+    )
+    .filter_map(|candidate: either::Either<hir::ModuleDef, hir::MacroDef>| match candidate {
+        either::Either::Left(hir::ModuleDef::Trait(trait_)) => Some(trait_),
+        _ => None,
+    })
+    .flat_map(|trait_| {
+        current_module
+            .find_use_path(ctx.sema.db, hir::ModuleDef::Trait(trait_))
+            .as_ref()
+            .map(mod_path_to_ast)
+            .zip(Some(trait_))
+    });
 
     let mut no_traits_found = true;
     for (trait_path, trait_) in found_traits.inspect(|_| no_traits_found = false) {
diff --git a/crates/assists/src/lib.rs b/crates/assists/src/lib.rs
index fdec886e9..212464f85 100644
--- a/crates/assists/src/lib.rs
+++ b/crates/assists/src/lib.rs
@@ -116,6 +116,7 @@ mod handlers {
     mod convert_integer_literal;
     mod early_return;
     mod expand_glob_import;
+    mod extract_assignment;
     mod extract_module_to_file;
     mod extract_struct_from_enum_variant;
     mod extract_variable;
@@ -167,6 +168,7 @@ mod handlers {
             convert_integer_literal::convert_integer_literal,
             early_return::convert_to_guarded_return,
             expand_glob_import::expand_glob_import,
+            extract_assignment::extract_assigment,
             extract_module_to_file::extract_module_to_file,
             extract_struct_from_enum_variant::extract_struct_from_enum_variant,
             extract_variable::extract_variable,
diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs
index d3dfe24e7..b91a816e8 100644
--- a/crates/assists/src/tests/generated.rs
+++ b/crates/assists/src/tests/generated.rs
@@ -238,6 +238,35 @@ fn qux(bar: Bar, baz: Baz) {}
 }
 
 #[test]
+fn doctest_extract_assignment() {
+    check_doc_test(
+        "extract_assignment",
+        r#####"
+fn main() {
+    let mut foo = 6;
+
+    if true {
+        <|>foo = 5;
+    } else {
+        foo = 4;
+    }
+}
+"#####,
+        r#####"
+fn main() {
+    let mut foo = 6;
+
+    foo = if true {
+        5
+    } else {
+        4
+    };
+}
+"#####,
+    )
+}
+
+#[test]
 fn doctest_extract_module_to_file() {
     check_doc_test(
         "extract_module_to_file",
diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs
index d41084b59..b05596446 100644
--- a/crates/assists/src/utils.rs
+++ b/crates/assists/src/utils.rs
@@ -94,14 +94,18 @@ pub fn filter_assoc_items(
             ast::AssocItem::MacroCall(_) => None,
         }
         .is_some()
-    };
+    }
 
     items
         .iter()
-        .map(|i| match i {
-            hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source(db).value),
-            hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source(db).value),
-            hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source(db).value),
+        // Note: This throws away items with no source.
+        .filter_map(|i| {
+            let item = match i {
+                hir::AssocItem::Function(i) => ast::AssocItem::Fn(i.source(db)?.value),
+                hir::AssocItem::TypeAlias(i) => ast::AssocItem::TypeAlias(i.source(db)?.value),
+                hir::AssocItem::Const(i) => ast::AssocItem::Const(i.source(db)?.value),
+            };
+            Some(item)
         })
         .filter(has_def_name)
         .filter(|it| match it {
diff --git a/crates/assists/src/utils/import_assets.rs b/crates/assists/src/utils/import_assets.rs
index ff5c0e78e..4ce82c1ba 100644
--- a/crates/assists/src/utils/import_assets.rs
+++ b/crates/assists/src/utils/import_assets.rs
@@ -179,25 +179,24 @@ impl ImportAssets {
             }
         };
 
-        let mut res =
-            imports_locator::find_exact_imports(sema, current_crate, &self.get_search_query())
-                .filter_map(filter)
-                .filter_map(|candidate| {
-                    let item: hir::ItemInNs = candidate.either(Into::into, Into::into);
-                    if let Some(prefix_kind) = prefixed {
-                        self.module_with_name_to_import.find_use_path_prefixed(
-                            db,
-                            item,
-                            prefix_kind,
-                        )
-                    } else {
-                        self.module_with_name_to_import.find_use_path(db, item)
-                    }
-                    .map(|path| (path, item))
-                })
-                .filter(|(use_path, _)| use_path.len() > 1)
-                .take(20)
-                .collect::<Vec<_>>();
+        let mut res = imports_locator::find_exact_imports(
+            sema,
+            current_crate,
+            self.get_search_query().to_string(),
+        )
+        .filter_map(filter)
+        .filter_map(|candidate| {
+            let item: hir::ItemInNs = candidate.either(Into::into, Into::into);
+            if let Some(prefix_kind) = prefixed {
+                self.module_with_name_to_import.find_use_path_prefixed(db, item, prefix_kind)
+            } else {
+                self.module_with_name_to_import.find_use_path(db, item)
+            }
+            .map(|path| (path, item))
+        })
+        .filter(|(use_path, _)| use_path.len() > 1)
+        .take(20)
+        .collect::<Vec<_>>();
         res.sort_by_key(|(path, _)| path.clone());
         res
     }