4 files changed, 151 insertions, 81 deletions

diff --git a/crates/ra_assists/src/handlers/change_return_type_to_result.rs b/crates/ra_assists/src/handlers/change_return_type_to_result.rs
index c6baa0a57..855baf187 100644
--- a/crates/ra_assists/src/handlers/change_return_type_to_result.rs
+++ b/crates/ra_assists/src/handlers/change_return_type_to_result.rs
@@ -4,6 +4,7 @@ use ra_syntax::{
 };
 
 use crate::{AssistContext, AssistId, Assists};
+use test_utils::mark;
 
 // Assist: change_return_type_to_result
 //
@@ -22,8 +23,13 @@ pub(crate) fn change_return_type_to_result(acc: &mut Assists, ctx: &AssistContex
     let fn_def = ret_type.syntax().parent().and_then(ast::FnDef::cast)?;
 
     let type_ref = &ret_type.type_ref()?;
-    if type_ref.syntax().text().to_string().starts_with("Result<") {
-        return None;
+    let ret_type_str = type_ref.syntax().text().to_string();
+    let first_part_ret_type = ret_type_str.splitn(2, '<').next();
+    if let Some(ret_type_first_part) = first_part_ret_type {
+        if ret_type_first_part.ends_with("Result") {
+            mark::hit!(change_return_type_to_result_simple_return_type_already_result);
+            return None;
+        }
     }
 
     let block_expr = &fn_def.body()?;
@@ -297,6 +303,29 @@ mod tests {
     }
 
     #[test]
+    fn change_return_type_to_result_simple_return_type_already_result_std() {
+        check_assist_not_applicable(
+            change_return_type_to_result,
+            r#"fn foo() -> std::result::Result<i32<|>, String> {
+                let test = "test";
+                return 42i32;
+            }"#,
+        );
+    }
+
+    #[test]
+    fn change_return_type_to_result_simple_return_type_already_result() {
+        mark::check!(change_return_type_to_result_simple_return_type_already_result);
+        check_assist_not_applicable(
+            change_return_type_to_result,
+            r#"fn foo() -> Result<i32<|>, String> {
+                let test = "test";
+                return 42i32;
+            }"#,
+        );
+    }
+
+    #[test]
     fn change_return_type_to_result_simple_with_cursor() {
         check_assist(
             change_return_type_to_result,
diff --git a/crates/ra_assists/src/handlers/introduce_variable.rs b/crates/ra_assists/src/handlers/extract_variable.rs
index 88b62278f..c4150d2bb 100644
--- a/crates/ra_assists/src/handlers/introduce_variable.rs
+++ b/crates/ra_assists/src/handlers/extract_variable.rs
@@ -11,7 +11,7 @@ use test_utils::mark;
 
 use crate::{AssistContext, AssistId, Assists};
 
-// Assist: introduce_variable
+// Assist: extract_variable
 //
 // Extracts subexpression into a variable.
 //
@@ -27,13 +27,13 @@ use crate::{AssistContext, AssistId, Assists};
 //     var_name * 4;
 // }
 // ```
-pub(crate) fn introduce_variable(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
+pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     if ctx.frange.range.is_empty() {
         return None;
     }
     let node = ctx.covering_element();
     if node.kind() == COMMENT {
-        mark::hit!(introduce_var_in_comment_is_not_applicable);
+        mark::hit!(extract_var_in_comment_is_not_applicable);
         return None;
     }
     let expr = node.ancestors().find_map(valid_target_expr)?;
@@ -43,13 +43,27 @@ pub(crate) fn introduce_variable(acc: &mut Assists, ctx: &AssistContext) -> Opti
         return None;
     }
     let target = expr.syntax().text_range();
-    acc.add(AssistId("introduce_variable"), "Extract into variable", target, move |edit| {
+    acc.add(AssistId("extract_variable"), "Extract into variable", target, move |edit| {
+        let field_shorthand = match expr.syntax().parent().and_then(ast::RecordField::cast) {
+            Some(field) => field.name_ref(),
+            None => None,
+        };
+
         let mut buf = String::new();
 
+        let var_name = match &field_shorthand {
+            Some(it) => it.to_string(),
+            None => "var_name".to_string(),
+        };
+        let expr_range = match &field_shorthand {
+            Some(it) => it.syntax().text_range().cover(expr.syntax().text_range()),
+            None => expr.syntax().text_range(),
+        };
+
         if wrap_in_block {
-            buf.push_str("{ let var_name = ");
+            format_to!(buf, "{{ let {} = ", var_name);
         } else {
-            buf.push_str("let var_name = ");
+            format_to!(buf, "let {} = ", var_name);
         };
         format_to!(buf, "{}", expr.syntax());
 
@@ -60,17 +74,17 @@ pub(crate) fn introduce_variable(acc: &mut Assists, ctx: &AssistContext) -> Opti
             false
         };
         if is_full_stmt {
-            mark::hit!(test_introduce_var_expr_stmt);
+            mark::hit!(test_extract_var_expr_stmt);
             if full_stmt.unwrap().semicolon_token().is_none() {
                 buf.push_str(";");
             }
-            let offset = expr.syntax().text_range();
             match ctx.config.snippet_cap {
                 Some(cap) => {
-                    let snip = buf.replace("let var_name", "let $0var_name");
-                    edit.replace_snippet(cap, offset, snip)
+                    let snip =
+                        buf.replace(&format!("let {}", var_name), &format!("let $0{}", var_name));
+                    edit.replace_snippet(cap, expr_range, snip)
                 }
-                None => edit.replace(offset, buf),
+                None => edit.replace(expr_range, buf),
             }
             return;
         }
@@ -88,11 +102,12 @@ pub(crate) fn introduce_variable(acc: &mut Assists, ctx: &AssistContext) -> Opti
             buf.push_str(text);
         }
 
-        edit.replace(expr.syntax().text_range(), "var_name".to_string());
+        edit.replace(expr_range, var_name.clone());
         let offset = anchor_stmt.text_range().start();
         match ctx.config.snippet_cap {
             Some(cap) => {
-                let snip = buf.replace("let var_name", "let $0var_name");
+                let snip =
+                    buf.replace(&format!("let {}", var_name), &format!("let $0{}", var_name));
                 edit.insert_snippet(cap, offset, snip)
             }
             None => edit.insert(offset, buf),
@@ -118,7 +133,7 @@ fn valid_target_expr(node: SyntaxNode) -> Option<ast::Expr> {
     }
 }
 
-/// Returns the syntax node which will follow the freshly introduced var
+/// Returns the syntax node which will follow the freshly extractd var
 /// and a boolean indicating whether we have to wrap it within a { } block
 /// to produce correct code.
 /// It can be a statement, the last in a block expression or a wanna be block
@@ -127,7 +142,7 @@ fn anchor_stmt(expr: ast::Expr) -> Option<(SyntaxNode, bool)> {
     expr.syntax().ancestors().find_map(|node| {
         if let Some(expr) = node.parent().and_then(ast::BlockExpr::cast).and_then(|it| it.expr()) {
             if expr.syntax() == &node {
-                mark::hit!(test_introduce_var_last_expr);
+                mark::hit!(test_extract_var_last_expr);
                 return Some((node, false));
             }
         }
@@ -155,9 +170,9 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_introduce_var_simple() {
+    fn test_extract_var_simple() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             r#"
 fn foo() {
     foo(<|>1 + 1<|>);
@@ -171,16 +186,16 @@ fn foo() {
     }
 
     #[test]
-    fn introduce_var_in_comment_is_not_applicable() {
-        mark::check!(introduce_var_in_comment_is_not_applicable);
-        check_assist_not_applicable(introduce_variable, "fn main() { 1 + /* <|>comment<|> */ 1; }");
+    fn extract_var_in_comment_is_not_applicable() {
+        mark::check!(extract_var_in_comment_is_not_applicable);
+        check_assist_not_applicable(extract_variable, "fn main() { 1 + /* <|>comment<|> */ 1; }");
     }
 
     #[test]
-    fn test_introduce_var_expr_stmt() {
-        mark::check!(test_introduce_var_expr_stmt);
+    fn test_extract_var_expr_stmt() {
+        mark::check!(test_extract_var_expr_stmt);
         check_assist(
-            introduce_variable,
+            extract_variable,
             r#"
 fn foo() {
     <|>1 + 1<|>;
@@ -191,7 +206,7 @@ fn foo() {
 }"#,
         );
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn foo() {
     <|>{ let x = 0; x }<|>
@@ -206,9 +221,9 @@ fn foo() {
     }
 
     #[test]
-    fn test_introduce_var_part_of_expr_stmt() {
+    fn test_extract_var_part_of_expr_stmt() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn foo() {
     <|>1<|> + 1;
@@ -222,10 +237,10 @@ fn foo() {
     }
 
     #[test]
-    fn test_introduce_var_last_expr() {
-        mark::check!(test_introduce_var_last_expr);
+    fn test_extract_var_last_expr() {
+        mark::check!(test_extract_var_last_expr);
         check_assist(
-            introduce_variable,
+            extract_variable,
             r#"
 fn foo() {
     bar(<|>1 + 1<|>)
@@ -239,7 +254,7 @@ fn foo() {
 "#,
         );
         check_assist(
-            introduce_variable,
+            extract_variable,
             r#"
 fn foo() {
     <|>bar(1 + 1)<|>
@@ -255,9 +270,9 @@ fn foo() {
     }
 
     #[test]
-    fn test_introduce_var_in_match_arm_no_block() {
+    fn test_extract_var_in_match_arm_no_block() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let x = true;
@@ -280,9 +295,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_in_match_arm_with_block() {
+    fn test_extract_var_in_match_arm_with_block() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let x = true;
@@ -312,9 +327,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_in_closure_no_block() {
+    fn test_extract_var_in_closure_no_block() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let lambda = |x: u32| <|>x * 2<|>;
@@ -329,9 +344,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_in_closure_with_block() {
+    fn test_extract_var_in_closure_with_block() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let lambda = |x: u32| { <|>x * 2<|> };
@@ -346,9 +361,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_path_simple() {
+    fn test_extract_var_path_simple() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let o = <|>Some(true)<|>;
@@ -364,9 +379,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_path_method() {
+    fn test_extract_var_path_method() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let v = <|>bar.foo()<|>;
@@ -382,9 +397,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_return() {
+    fn test_extract_var_return() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn foo() -> u32 {
     <|>return 2 + 2<|>;
@@ -400,9 +415,9 @@ fn foo() -> u32 {
     }
 
     #[test]
-    fn test_introduce_var_does_not_add_extra_whitespace() {
+    fn test_extract_var_does_not_add_extra_whitespace() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn foo() -> u32 {
 
@@ -421,7 +436,7 @@ fn foo() -> u32 {
         );
 
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn foo() -> u32 {
 
@@ -438,7 +453,7 @@ fn foo() -> u32 {
         );
 
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn foo() -> u32 {
     let foo = 1;
@@ -464,9 +479,9 @@ fn foo() -> u32 {
     }
 
     #[test]
-    fn test_introduce_var_break() {
+    fn test_extract_var_break() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let result = loop {
@@ -486,9 +501,9 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_for_cast() {
+    fn test_extract_var_for_cast() {
         check_assist(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let v = <|>0f32 as u32<|>;
@@ -504,22 +519,48 @@ fn main() {
     }
 
     #[test]
-    fn test_introduce_var_for_return_not_applicable() {
-        check_assist_not_applicable(introduce_variable, "fn foo() { <|>return<|>; } ");
+    fn extract_var_field_shorthand() {
+        check_assist(
+            extract_variable,
+            r#"
+struct S {
+    foo: i32
+}
+
+fn main() {
+    S { foo: <|>1 + 1<|> }
+}
+"#,
+            r#"
+struct S {
+    foo: i32
+}
+
+fn main() {
+    let $0foo = 1 + 1;
+    S { foo }
+}
+"#,
+        )
+    }
+
+    #[test]
+    fn test_extract_var_for_return_not_applicable() {
+        check_assist_not_applicable(extract_variable, "fn foo() { <|>return<|>; } ");
     }
 
     #[test]
-    fn test_introduce_var_for_break_not_applicable() {
-        check_assist_not_applicable(introduce_variable, "fn main() { loop { <|>break<|>; }; }");
+    fn test_extract_var_for_break_not_applicable() {
+        check_assist_not_applicable(extract_variable, "fn main() { loop { <|>break<|>; }; }");
     }
 
     // FIXME: This is not quite correct, but good enough(tm) for the sorting heuristic
     #[test]
-    fn introduce_var_target() {
-        check_assist_target(introduce_variable, "fn foo() -> u32 { <|>return 2 + 2<|>; }", "2 + 2");
+    fn extract_var_target() {
+        check_assist_target(extract_variable, "fn foo() -> u32 { <|>return 2 + 2<|>; }", "2 + 2");
 
         check_assist_target(
-            introduce_variable,
+            extract_variable,
             "
 fn main() {
     let x = true;
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index 185428bd5..1745f44a5 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -116,6 +116,7 @@ mod handlers {
     mod change_visibility;
     mod early_return;
     mod extract_struct_from_enum_variant;
+    mod extract_variable;
     mod fill_match_arms;
     mod fix_visibility;
     mod flip_binexpr;
@@ -123,7 +124,6 @@ mod handlers {
     mod flip_trait_bound;
     mod inline_local_variable;
     mod introduce_named_lifetime;
-    mod introduce_variable;
     mod invert_if;
     mod merge_imports;
     mod merge_match_arms;
@@ -157,6 +157,7 @@ mod handlers {
             change_visibility::change_visibility,
             early_return::convert_to_guarded_return,
             extract_struct_from_enum_variant::extract_struct_from_enum_variant,
+            extract_variable::extract_variable,
             fill_match_arms::fill_match_arms,
             fix_visibility::fix_visibility,
             flip_binexpr::flip_binexpr,
@@ -164,7 +165,6 @@ mod handlers {
             flip_trait_bound::flip_trait_bound,
             inline_local_variable::inline_local_variable,
             introduce_named_lifetime::introduce_named_lifetime,
-            introduce_variable::introduce_variable,
             invert_if::invert_if,
             merge_imports::merge_imports,
             merge_match_arms::merge_match_arms,
diff --git a/crates/ra_assists/src/tests/generated.rs b/crates/ra_assists/src/tests/generated.rs
index 40a223727..31ea888c5 100644
--- a/crates/ra_assists/src/tests/generated.rs
+++ b/crates/ra_assists/src/tests/generated.rs
@@ -353,6 +353,24 @@ enum A { One(One) }
 }
 
 #[test]
+fn doctest_extract_variable() {
+    check_doc_test(
+        "extract_variable",
+        r#####"
+fn main() {
+    <|>(1 + 2)<|> * 4;
+}
+"#####,
+        r#####"
+fn main() {
+    let $0var_name = (1 + 2);
+    var_name * 4;
+}
+"#####,
+    )
+}
+
+#[test]
 fn doctest_fill_match_arms() {
     check_doc_test(
         "fill_match_arms",
@@ -492,24 +510,6 @@ impl<'a> Cursor<'a> {
 }
 
 #[test]
-fn doctest_introduce_variable() {
-    check_doc_test(
-        "introduce_variable",
-        r#####"
-fn main() {
-    <|>(1 + 2)<|> * 4;
-}
-"#####,
-        r#####"
-fn main() {
-    let $0var_name = (1 + 2);
-    var_name * 4;
-}
-"#####,
-    )
-}
-
-#[test]
 fn doctest_invert_if() {
     check_doc_test(
         "invert_if",