Repository: rust-analyzer/rust-analyzer
Pull request #7130 (2021-01-02 01:01:28)

Add extract_assignment assist
Add extract-assignment assist (#7006).

Assist is for now only implemented on if/match-statements where the assigment is the last statement in every block,
as for other cases, one would have to check whether the assignment has effects on the rest of the block and
extract a temporary variable for it in the block.



Jesse-Bakker: Added support for match. Getting correct indentation is a bit of a PITA and I'm not sure if I did it correctly
Veykril: Ye indentation is an unsolved problem still iirc. Ideally we would be able to just format the produced syntax but thats yet to be done.

LGTM overall, just a small nit.
bors d+
bors[bot]: :v: Jesse-Bakker can now approve this pull request. To approve and merge a pull request, simply reply with `bors r+`. More detailed instructions are available [here](https://bors.tech/documentation/getting-started/#reviewing-pull-requests).

Patch:
diff --git a/crates/assists/src/handlers/extract_assignment.rs b/crates/assists/src/handlers/extract_assignment.rs
new file mode 100644
index 0000000000..281cf5d24e
--- /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/lib.rs b/crates/assists/src/lib.rs
index fdec886e9d..212464f859 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/base_db/src/input.rs b/crates/base_db/src/input.rs
index 2dd8fbe679..9567bcc424 100644
--- a/crates/base_db/src/input.rs
+++ b/crates/base_db/src/input.rs
@@ -190,11 +190,10 @@ pub struct CrateData {
     pub proc_macro: Vec<ProcMacro>,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum Edition {
-    Edition2015,
     Edition2018,
-    Edition2021,
+    Edition2015,
 }
 
 #[derive(Default, Debug, Clone, PartialEq, Eq)]
@@ -394,7 +393,6 @@ impl FromStr for Edition {
         let res = match s {
             "2015" => Edition::Edition2015,
             "2018" => Edition::Edition2018,
-            "2021" => Edition::Edition2021,
             _ => return Err(ParseEditionError { invalid_input: s.to_string() }),
         };
         Ok(res)
@@ -406,7 +404,6 @@ impl fmt::Display for Edition {
         f.write_str(match self {
             Edition::Edition2015 => "2015",
             Edition::Edition2018 => "2018",
-            Edition::Edition2021 => "2021",
         })
     }
 }
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index 0a0d021e05..06f0a3ed9c 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -404,7 +404,7 @@ fn to_fragment_kind(db: &dyn AstDatabase, id: MacroCallId) -> FragmentKind {
         TRY_EXPR => FragmentKind::Expr,
         TUPLE_EXPR => FragmentKind::Expr,
         PAREN_EXPR => FragmentKind::Expr,
-        ARRAY_EXPR => FragmentKind::Expr,
+
         FOR_EXPR => FragmentKind::Expr,
         PATH_EXPR => FragmentKind::Expr,
         CLOSURE_EXPR => FragmentKind::Expr,
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index b4e453411a..8494157063 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -156,7 +156,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
         // FIXME: Due to shortcomings in the current type system implementation, only emit this
         // diagnostic if there are no type mismatches in the containing function.
         if self.infer.type_mismatches.iter().next().is_some() {
-            return None;
+            return Some(());
         }
 
         let is_method_call = matches!(expr, Expr::MethodCall { .. });
@@ -170,14 +170,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
                 let mut args = args.clone();
                 args.insert(0, *receiver);
 
-                let receiver = &self.infer.type_of_expr[*receiver];
-                if receiver.strip_references().is_unknown() {
-                    // if the receiver is of unknown type, it's very likely we
-                    // don't know enough to correctly resolve the method call.
-                    // This is kind of a band-aid for #6975.
-                    return None;
-                }
-
                 // FIXME: note that we erase information about substs here. This
                 // is not right, but, luckily, doesn't matter as we care only
                 // about the number of params
@@ -512,22 +504,6 @@ fn f() {
         );
     }
 
-    #[test]
-    fn method_unknown_receiver() {
-        // note: this is incorrect code, so there might be errors on this in the
-        // future, but we shouldn't emit an argument count diagnostic here
-        check_diagnostics(
-            r#"
-trait Foo { fn method(&self, arg: usize) {} }
-
-fn f() {
-    let x;
-    x.method();
-}
-"#,
-        );
-    }
-
     #[test]
     fn tuple_struct() {
         check_diagnostics(
diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs
index e00c7e1766..357bd92f99 100644
--- a/crates/hir_ty/src/lib.rs
+++ b/crates/hir_ty/src/lib.rs
@@ -791,10 +791,6 @@ impl Ty {
         matches!(self, Ty::Apply(ApplicationTy { ctor: TypeCtor::Never, .. }))
     }
 
-    pub fn is_unknown(&self) -> bool {
-        matches!(self, Ty::Unknown)
-    }
-
     /// If this is a `dyn Trait` type, this returns the `Trait` part.
     pub fn dyn_trait_ref(&self) -> Option<&TraitRef> {
         match self {
diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs
index c893afc7cf..8911832660 100644
--- a/crates/ide/src/runnables.rs
+++ b/crates/ide/src/runnables.rs
@@ -230,7 +230,7 @@ impl TestAttr {
 
 const RUSTDOC_FENCE: &str = "```";
 const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUNNABLE: &[&str] =
-    &["", "rust", "should_panic", "edition2015", "edition2018", "edition2021"];
+    &["", "rust", "should_panic", "edition2015", "edition2018"];
 
 fn has_runnable_doc_test(attrs: &hir::Attrs) -> bool {
     attrs.docs().map_or(false, |doc| {
diff --git a/crates/ide/src/syntax_highlighting/injection.rs b/crates/ide/src/syntax_highlighting/injection.rs
index 6cbd683c69..9eb184c74b 100644
--- a/crates/ide/src/syntax_highlighting/injection.rs
+++ b/crates/ide/src/syntax_highlighting/injection.rs
@@ -54,17 +54,8 @@ pub(super) fn highlight_injection(
 type RangesMap = BTreeMap<TextSize, TextSize>;
 
 const RUSTDOC_FENCE: &'static str = "```";
-const RUSTDOC_FENCE_TOKENS: &[&'static str] = &[
-    "",
-    "rust",
-    "should_panic",
-    "ignore",
-    "no_run",
-    "compile_fail",
-    "edition2015",
-    "edition2018",
-    "edition2021",
-];
+const RUSTDOC_FENCE_TOKENS: &[&'static str] =
+    &["", "rust", "should_panic", "ignore", "no_run", "compile_fail", "edition2015", "edition2018"];
 
 /// Extracts Rust code from documentation comments as well as a mapping from
 /// the extracted source code back to the original source ranges.
diff --git a/crates/project_model/src/project_json.rs b/crates/project_model/src/project_json.rs
index af884eb84a..aab279223a 100644
--- a/crates/project_model/src/project_json.rs
+++ b/crates/project_model/src/project_json.rs
@@ -139,8 +139,6 @@ enum EditionData {
     Edition2015,
     #[serde(rename = "2018")]
     Edition2018,
-    #[serde(rename = "2021")]
-    Edition2021,
 }
 
 impl From<EditionData> for Edition {
@@ -148,7 +146,6 @@ impl From<EditionData> for Edition {
         match data {
             EditionData::Edition2015 => Edition::Edition2015,
             EditionData::Edition2018 => Edition::Edition2018,
-            EditionData::Edition2021 => Edition::Edition2021,
         }
     }
 }
diff --git a/crates/rust-analyzer/src/markdown.rs b/crates/rust-analyzer/src/markdown.rs
index a49a58c000..968ea55f0c 100644
--- a/crates/rust-analyzer/src/markdown.rs
+++ b/crates/rust-analyzer/src/markdown.rs
@@ -1,17 +1,8 @@
 //! Transforms markdown
 
 const RUSTDOC_FENCE: &str = "```";
-const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUST_SPECIFIC: &[&str] = &[
-    "",
-    "rust",
-    "should_panic",
-    "ignore",
-    "no_run",
-    "compile_fail",
-    "edition2015",
-    "edition2018",
-    "edition2021",
-];
+const RUSTDOC_CODE_BLOCK_ATTRIBUTES_RUST_SPECIFIC: &[&str] =
+    &["", "rust", "should_panic", "ignore", "no_run", "compile_fail", "edition2015", "edition2018"];
 
 pub(crate) fn format_docs(src: &str) -> String {
     let mut processed_lines = Vec::new();
diff --git a/docs/user/manual.adoc b/docs/user/manual.adoc
index 75445a02e5..d4121b4011 100644
--- a/docs/user/manual.adoc
+++ b/docs/user/manual.adoc
@@ -339,7 +339,7 @@ interface Crate {
     /// Path to the root module of the crate.
     root_module: string;
     /// Edition of the crate.
-    edition: "2015" | "2018" | "2021";
+    edition: "2015" | "2018";
     /// Dependencies
     deps: Dep[];
     /// Should this crate be treated as a member of current "workspace".
diff --git a/xtask/src/dist.rs b/xtask/src/dist.rs
index 6bc34106b6..d59b88131a 100644
--- a/xtask/src/dist.rs
+++ b/xtask/src/dist.rs
@@ -63,7 +63,8 @@ fn dist_server() -> Result<()> {
         env::set_var("CC", "clang");
     }
 
-    cmd!("cargo build --manifest-path ./crates/rust-analyzer/Cargo.toml --bin rust-analyzer --target {target} --release").run()?;
+    let toolchain = toolchain(&target);
+    cmd!("cargo +{toolchain} build --manifest-path ./crates/rust-analyzer/Cargo.toml --bin rust-analyzer --target {target} --release").run()?;
 
     let suffix = exe_suffix(&target);
     let src =
@@ -117,6 +118,13 @@ fn exe_suffix(target: &str) -> String {
     }
 }
 
+fn toolchain(target: &str) -> String {
+    match target {
+        "aarch64-apple-darwin" => "beta".to_string(),
+        _ => "stable".to_string(),
+    }
+}
+
 fn gzip(src_path: &Path, dest_path: &Path) -> Result<()> {
     let mut encoder = GzEncoder::new(File::create(dest_path)?, Compression::best());
     let mut input = io::BufReader::new(File::open(src_path)?);


Test patch:
diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs
index d3dfe24e7c..b91a816e89 100644
--- a/crates/assists/src/tests/generated.rs
+++ b/crates/assists/src/tests/generated.rs
@@ -237,6 +237,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(
diff --git a/crates/hir_ty/src/tests/regression.rs b/crates/hir_ty/src/tests/regression.rs
index cffe8630b4..307a257b17 100644
--- a/crates/hir_ty/src/tests/regression.rs
+++ b/crates/hir_ty/src/tests/regression.rs
@@ -325,24 +325,6 @@ fn infer_paren_macro_call() {
     );
 }
 
-#[test]
-fn infer_array_macro_call() {
-    check_infer(
-        r#"
-        macro_rules! bar { () => {0u32} }
-        fn test() {
-            let a = [bar!()];
-        }
-        "#,
-        expect![[r#"
-            !0..4 '0u32': u32
-            44..69 '{     ...()]; }': ()
-            54..55 'a': [u32; _]
-            58..66 '[bar!()]': [u32; _]
-        "#]],
-    );
-}
-
 #[test]
 fn bug_1030() {
     check_infer(