1 files changed, 79 insertions, 11 deletions

diff --git a/crates/ra_editor/src/code_actions.rs b/crates/ra_editor/src/code_actions.rs
index 0139b19d3..6979251d1 100644
--- a/crates/ra_editor/src/code_actions.rs
+++ b/crates/ra_editor/src/code_actions.rs
@@ -4,7 +4,7 @@ use ra_syntax::{
     algo::{find_covering_node, find_leaf_at_offset},
     ast::{self, AstNode, AttrsOwner, NameOwner, TypeParamsOwner},
     Direction, SourceFileNode,
-    SyntaxKind::{COMMA, WHITESPACE},
+    SyntaxKind::{COMMA, WHITESPACE, COMMENT},
     SyntaxNodeRef, TextRange, TextUnit,
 };
 
@@ -41,7 +41,8 @@ pub fn add_derive<'a>(
     offset: TextUnit,
 ) -> Option<impl FnOnce() -> LocalEdit + 'a> {
     let nominal = find_node_at_offset::<ast::NominalDef>(file.syntax(), offset)?;
-    Some(move || {
+    let node_start = derive_insertion_offset(nominal)?;
+    return Some(move || {
         let derive_attr = nominal
             .attrs()
             .filter_map(|x| x.as_call())
@@ -51,7 +52,6 @@ pub fn add_derive<'a>(
         let mut edit = EditBuilder::new();
         let offset = match derive_attr {
             None => {
-                let node_start = nominal.syntax().range().start();
                 edit.insert(node_start, "#[derive()]\n".to_string());
                 node_start + TextUnit::of_str("#[derive(")
             }
@@ -61,7 +61,16 @@ pub fn add_derive<'a>(
             edit: edit.finish(),
             cursor_position: Some(offset),
         }
-    })
+    });
+
+    // Insert `derive` after doc comments.
+    fn derive_insertion_offset(nominal: ast::NominalDef) -> Option<TextUnit> {
+        let non_ws_child = nominal
+            .syntax()
+            .children()
+            .find(|it| it.kind() != COMMENT && it.kind() != WHITESPACE)?;
+        Some(non_ws_child.range().start())
+    }
 }
 
 pub fn add_impl<'a>(
@@ -113,7 +122,7 @@ pub fn introduce_variable<'a>(
     let node = find_covering_node(file.syntax(), range);
     let expr = node.ancestors().filter_map(ast::Expr::cast).next()?;
 
-    let anchor_stmt = ahchor_stmt(expr)?;
+    let anchor_stmt = anchor_stmt(expr)?;
     let indent = anchor_stmt.prev_sibling()?;
     if indent.kind() != WHITESPACE {
         return None;
@@ -124,7 +133,12 @@ pub fn introduce_variable<'a>(
 
         buf.push_str("let var_name = ");
         expr.syntax().text().push_to(&mut buf);
-        if expr.syntax().range().start() == anchor_stmt.range().start() {
+        let is_full_stmt = if let Some(expr_stmt) = ast::ExprStmt::cast(anchor_stmt) {
+            Some(expr.syntax()) == expr_stmt.expr().map(|e| e.syntax())
+        } else {
+            false
+        };
+        if is_full_stmt {
             edit.replace(expr.syntax().range(), buf);
         } else {
             buf.push_str(";");
@@ -141,7 +155,7 @@ pub fn introduce_variable<'a>(
 
     /// Statement or last in the block expression, which will follow
     /// the freshly introduced var.
-    fn ahchor_stmt(expr: ast::Expr) -> Option<SyntaxNodeRef> {
+    fn anchor_stmt(expr: ast::Expr) -> Option<SyntaxNodeRef> {
         expr.syntax().ancestors().find(|&node| {
             if ast::Stmt::cast(node).is_some() {
                 return true;
@@ -181,7 +195,7 @@ mod tests {
     }
 
     #[test]
-    fn test_add_derive() {
+    fn add_derive_new() {
         check_action(
             "struct Foo { a: i32, <|>}",
             "#[derive(<|>)]\nstruct Foo { a: i32, }",
@@ -192,6 +206,10 @@ mod tests {
             "#[derive(<|>)]\nstruct Foo {  a: i32, }",
             |file, off| add_derive(file, off).map(|f| f()),
         );
+    }
+
+    #[test]
+    fn add_derive_existing() {
         check_action(
             "#[derive(Clone)]\nstruct Foo { a: i32<|>, }",
             "#[derive(Clone<|>)]\nstruct Foo { a: i32, }",
@@ -200,6 +218,24 @@ mod tests {
     }
 
     #[test]
+    fn add_derive_new_with_doc_comment() {
+        check_action(
+            "
+/// `Foo` is a pretty important struct.
+/// It does stuff.
+struct Foo { a: i32<|>, }
+            ",
+            "
+/// `Foo` is a pretty important struct.
+/// It does stuff.
+#[derive(<|>)]
+struct Foo { a: i32, }
+            ",
+            |file, off| add_derive(file, off).map(|f| f()),
+        );
+    }
+
+    #[test]
     fn test_add_impl() {
         check_action(
             "struct Foo {<|>}\n",
@@ -219,7 +255,7 @@ mod tests {
     }
 
     #[test]
-    fn test_intrdoduce_var_simple() {
+    fn test_introduce_var_simple() {
         check_action_range(
             "
 fn foo() {
@@ -235,7 +271,7 @@ fn foo() {
     }
 
     #[test]
-    fn test_intrdoduce_var_expr_stmt() {
+    fn test_introduce_var_expr_stmt() {
         check_action_range(
             "
 fn foo() {
@@ -250,7 +286,23 @@ fn foo() {
     }
 
     #[test]
-    fn test_intrdoduce_var_last_expr() {
+    fn test_introduce_var_part_of_expr_stmt() {
+        check_action_range(
+            "
+fn foo() {
+    <|>1<|> + 1;
+}",
+            "
+fn foo() {
+    let <|>var_name = 1;
+    var_name + 1;
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
+    #[test]
+    fn test_introduce_var_last_expr() {
         check_action_range(
             "
 fn foo() {
@@ -265,4 +317,20 @@ fn foo() {
         );
     }
 
+    #[test]
+    fn test_introduce_var_last_full_expr() {
+        check_action_range(
+            "
+fn foo() {
+    <|>bar(1 + 1)<|>
+}",
+            "
+fn foo() {
+    let <|>var_name = bar(1 + 1);
+    var_name
+}",
+            |file, range| introduce_variable(file, range).map(|f| f()),
+        );
+    }
+
 }