Merge #889

889: Refactor assits r=matklad a=matklad

* assign unique IDs to assists so that clients could do custom stuff
* specify kinds for assists, 
* make introduce_variable a `refactoring.extract` and make it available only when expression is selected
* introduce marks to assists

Co-authored-by: Aleksey Kladov <[email protected]>

18 files changed, 182 insertions, 133 deletions

diff --git a/crates/ra_assists/Cargo.toml b/crates/ra_assists/Cargo.toml
index 880bc4b18..d4056a349 100644
--- a/crates/ra_assists/Cargo.toml
+++ b/crates/ra_assists/Cargo.toml
@@ -13,6 +13,4 @@ ra_text_edit = { path = "../ra_text_edit" }
 ra_fmt = { path = "../ra_fmt" }
 ra_db = { path = "../ra_db" }
 hir = { path = "../ra_hir", package = "ra_hir" }
-
-[dev-dependencies]
 test_utils = { path = "../test_utils" }
diff --git a/crates/ra_assists/src/add_derive.rs b/crates/ra_assists/src/add_derive.rs
index 0556dd69c..e91b5eb8d 100644
--- a/crates/ra_assists/src/add_derive.rs
+++ b/crates/ra_assists/src/add_derive.rs
@@ -5,12 +5,12 @@ use ra_syntax::{
     TextUnit,
 };
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn add_derive(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let nominal = ctx.node_at_offset::<ast::NominalDef>()?;
     let node_start = derive_insertion_offset(nominal)?;
-    ctx.add_action("add `#[derive]`", |edit| {
+    ctx.add_action(AssistId("add_derive"), "add `#[derive]`", |edit| {
         let derive_attr = nominal
             .attrs()
             .filter_map(|x| x.as_call())
diff --git a/crates/ra_assists/src/add_impl.rs b/crates/ra_assists/src/add_impl.rs
index b40b9cc0c..b292f188d 100644
--- a/crates/ra_assists/src/add_impl.rs
+++ b/crates/ra_assists/src/add_impl.rs
@@ -5,12 +5,12 @@ use ra_syntax::{
     TextUnit,
 };
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn add_impl(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let nominal = ctx.node_at_offset::<ast::NominalDef>()?;
     let name = nominal.name()?;
-    ctx.add_action("add impl", |edit| {
+    ctx.add_action(AssistId("add_impl"), "add impl", |edit| {
         edit.target(nominal.syntax().range());
         let type_params = nominal.type_param_list();
         let start_offset = nominal.syntax().range().end();
diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs
index e9c4f0a23..4ad21c74b 100644
--- a/crates/ra_assists/src/assist_ctx.rs
+++ b/crates/ra_assists/src/assist_ctx.rs
@@ -7,7 +7,7 @@ use ra_syntax::{
 };
 use ra_fmt::{leading_indent, reindent};
 
-use crate::{AssistLabel, AssistAction};
+use crate::{AssistLabel, AssistAction, AssistId};
 
 #[derive(Clone, Debug)]
 pub(crate) enum Assist {
@@ -81,10 +81,11 @@ impl<'a, DB: HirDatabase> AssistCtx<'a, DB> {
 
     pub(crate) fn add_action(
         &mut self,
+        id: AssistId,
         label: impl Into<String>,
         f: impl FnOnce(&mut AssistBuilder),
     ) -> &mut Self {
-        let label = AssistLabel { label: label.into() };
+        let label = AssistLabel { label: label.into(), id };
         match &mut self.assist {
             Assist::Unresolved(labels) => labels.push(label),
             Assist::Resolved(labels_actions) => {
diff --git a/crates/ra_assists/src/auto_import.rs b/crates/ra_assists/src/auto_import.rs
index 105c888d5..685dbed06 100644
--- a/crates/ra_assists/src/auto_import.rs
+++ b/crates/ra_assists/src/auto_import.rs
@@ -4,7 +4,10 @@ use ra_syntax::{
     ast::{ self, NameOwner }, AstNode, SyntaxNode, Direction, TextRange,
     SyntaxKind::{ PATH, PATH_SEGMENT, COLONCOLON, COMMA }
 };
-use crate::assist_ctx::{AssistCtx, Assist, AssistBuilder};
+use crate::{
+    AssistId,
+    assist_ctx::{AssistCtx, Assist, AssistBuilder},
+};
 
 fn collect_path_segments(path: &ast::Path) -> Option<Vec<&ast::PathSegment>> {
     let mut v = Vec::new();
@@ -526,6 +529,7 @@ pub(crate) fn auto_import(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist
     if let Some(module) = path.syntax().ancestors().find_map(ast::Module::cast) {
         if let (Some(item_list), Some(name)) = (module.item_list(), module.name()) {
             ctx.add_action(
+                AssistId("auto_import"),
                 format!("import {} in mod {}", fmt_segments(&segments), name.text()),
                 |edit| {
                     apply_auto_import(item_list.syntax(), path, &segments, edit);
@@ -534,9 +538,13 @@ pub(crate) fn auto_import(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist
         }
     } else {
         let current_file = node.ancestors().find_map(ast::SourceFile::cast)?;
-        ctx.add_action(format!("import {} in the current file", fmt_segments(&segments)), |edit| {
-            apply_auto_import(current_file.syntax(), path, &segments, edit);
-        });
+        ctx.add_action(
+            AssistId("auto_import"),
+            format!("import {} in the current file", fmt_segments(&segments)),
+            |edit| {
+                apply_auto_import(current_file.syntax(), path, &segments, edit);
+            },
+        );
     }
 
     ctx.build()
diff --git a/crates/ra_assists/src/change_visibility.rs b/crates/ra_assists/src/change_visibility.rs
index c2ba897a4..50c1be5ae 100644
--- a/crates/ra_assists/src/change_visibility.rs
+++ b/crates/ra_assists/src/change_visibility.rs
@@ -5,7 +5,7 @@ use ra_syntax::{
     SyntaxKind::{VISIBILITY, FN_KW, MOD_KW, STRUCT_KW, ENUM_KW, TRAIT_KW, FN_DEF, MODULE, STRUCT_DEF, ENUM_DEF, TRAIT_DEF, IDENT, WHITESPACE, COMMENT, ATTR},
 };
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn change_visibility(ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     if let Some(vis) = ctx.node_at_offset::<ast::Visibility>() {
@@ -41,7 +41,7 @@ fn add_vis(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
         (vis_offset(field.syntax()), ident.range())
     };
 
-    ctx.add_action("make pub(crate)", |edit| {
+    ctx.add_action(AssistId("change_visibility"), "make pub(crate)", |edit| {
         edit.target(target);
         edit.insert(offset, "pub(crate) ");
         edit.set_cursor(offset);
@@ -63,7 +63,7 @@ fn vis_offset(node: &SyntaxNode) -> TextUnit {
 
 fn change_vis(mut ctx: AssistCtx<impl HirDatabase>, vis: &ast::Visibility) -> Option<Assist> {
     if vis.syntax().text() == "pub" {
-        ctx.add_action("change to pub(crate)", |edit| {
+        ctx.add_action(AssistId("change_visibility"), "change to pub(crate)", |edit| {
             edit.target(vis.syntax().range());
             edit.replace(vis.syntax().range(), "pub(crate)");
             edit.set_cursor(vis.syntax().range().start())
@@ -72,7 +72,7 @@ fn change_vis(mut ctx: AssistCtx<impl HirDatabase>, vis: &ast::Visibility) -> Op
         return ctx.build();
     }
     if vis.syntax().text() == "pub(crate)" {
-        ctx.add_action("change to pub", |edit| {
+        ctx.add_action(AssistId("change_visibility"), "change to pub", |edit| {
             edit.target(vis.syntax().range());
             edit.replace(vis.syntax().range(), "pub");
             edit.set_cursor(vis.syntax().range().start());
diff --git a/crates/ra_assists/src/fill_match_arms.rs b/crates/ra_assists/src/fill_match_arms.rs
index 6bf6e7332..30020b56e 100644
--- a/crates/ra_assists/src/fill_match_arms.rs
+++ b/crates/ra_assists/src/fill_match_arms.rs
@@ -6,7 +6,7 @@ use hir::{
 };
 use ra_syntax::ast::{self, AstNode};
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn fill_match_arms(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let match_expr = ctx.node_at_offset::<ast::MatchExpr>()?;
@@ -37,7 +37,7 @@ pub(crate) fn fill_match_arms(mut ctx: AssistCtx<impl HirDatabase>) -> Option<As
     let enum_name = enum_def.name(ctx.db)?;
     let db = ctx.db;
 
-    ctx.add_action("fill match arms", |edit| {
+    ctx.add_action(AssistId("fill_match_arms"), "fill match arms", |edit| {
         let mut buf = format!("match {} {{\n", expr.syntax().text().to_string());
         let variants = enum_def.variants(db);
         for variant in variants {
diff --git a/crates/ra_assists/src/flip_comma.rs b/crates/ra_assists/src/flip_comma.rs
index 0d4a789fc..6b98cac68 100644
--- a/crates/ra_assists/src/flip_comma.rs
+++ b/crates/ra_assists/src/flip_comma.rs
@@ -5,13 +5,13 @@ use ra_syntax::{
     algo::non_trivia_sibling,
 };
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn flip_comma(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let comma = ctx.leaf_at_offset().find(|leaf| leaf.kind() == COMMA)?;
     let prev = non_trivia_sibling(comma, Direction::Prev)?;
     let next = non_trivia_sibling(comma, Direction::Next)?;
-    ctx.add_action("flip comma", |edit| {
+    ctx.add_action(AssistId("flip_comma"), "flip comma", |edit| {
         edit.target(comma.range());
         edit.replace(prev.range(), next.text());
         edit.replace(next.range(), prev.text());
diff --git a/crates/ra_assists/src/introduce_variable.rs b/crates/ra_assists/src/introduce_variable.rs
index f0e012105..353bc4105 100644
--- a/crates/ra_assists/src/introduce_variable.rs
+++ b/crates/ra_assists/src/introduce_variable.rs
@@ -1,3 +1,4 @@
+use test_utils::tested_by;
 use hir::db::HirDatabase;
 use ra_syntax::{
     ast::{self, AstNode},
@@ -6,20 +7,24 @@ use ra_syntax::{
     }, SyntaxNode, TextUnit,
 };
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn introduce_variable(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
+    if ctx.frange.range.is_empty() {
+        return None;
+    }
     let node = ctx.covering_node();
-    if !valid_covering_node(node) {
+    if node.kind() == COMMENT {
+        tested_by!(introduce_var_in_comment_is_not_applicable);
         return None;
     }
-    let expr = node.ancestors().filter_map(valid_target_expr).next()?;
+    let expr = node.ancestors().find_map(valid_target_expr)?;
     let (anchor_stmt, wrap_in_block) = anchor_stmt(expr)?;
     let indent = anchor_stmt.prev_sibling()?;
     if indent.kind() != WHITESPACE {
         return None;
     }
-    ctx.add_action("introduce variable", move |edit| {
+    ctx.add_action(AssistId("introduce_variable"), "introduce variable", move |edit| {
         let mut buf = String::new();
 
         let cursor_offset = if wrap_in_block {
@@ -38,6 +43,7 @@ pub(crate) fn introduce_variable(mut ctx: AssistCtx<impl HirDatabase>) -> Option
             false
         };
         if is_full_stmt {
+            tested_by!(test_introduce_var_expr_stmt);
             if !full_stmt.unwrap().has_semi() {
                 buf.push_str(";");
             }
@@ -73,9 +79,6 @@ pub(crate) fn introduce_variable(mut ctx: AssistCtx<impl HirDatabase>) -> Option
     ctx.build()
 }
 
-fn valid_covering_node(node: &SyntaxNode) -> bool {
-    node.kind() != COMMENT
-}
 /// Check whether the node is a valid expression which can be extracted to a variable.
 /// In general that's true for any expression, but in some cases that would produce invalid code.
 fn valid_target_expr(node: &SyntaxNode) -> Option<&ast::Expr> {
@@ -101,6 +104,7 @@ fn anchor_stmt(expr: &ast::Expr) -> Option<(&SyntaxNode, bool)> {
 
         if let Some(expr) = node.parent().and_then(ast::Block::cast).and_then(|it| it.expr()) {
             if expr.syntax() == node {
+                tested_by!(test_introduce_var_last_expr);
                 return Some((node, false));
             }
         }
@@ -117,8 +121,11 @@ fn anchor_stmt(expr: &ast::Expr) -> Option<(&SyntaxNode, bool)> {
 
 #[cfg(test)]
 mod tests {
+    use test_utils::covers;
+
+    use crate::helpers::{check_assist_range_not_applicable, check_assist_range, check_assist_range_target};
+
     use super::*;
-    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_range, check_assist_target, check_assist_range_target};
 
     #[test]
     fn test_introduce_var_simple() {
@@ -137,7 +144,17 @@ fn foo() {
     }
 
     #[test]
+    fn introduce_var_in_comment_is_not_applicable() {
+        covers!(introduce_var_in_comment_is_not_applicable);
+        check_assist_range_not_applicable(
+            introduce_variable,
+            "fn main() { 1 + /* <|>comment<|> */ 1; }",
+        );
+    }
+
+    #[test]
     fn test_introduce_var_expr_stmt() {
+        covers!(test_introduce_var_expr_stmt);
         check_assist_range(
             introduce_variable,
             "
@@ -149,6 +166,19 @@ fn foo() {
     let <|>var_name = 1 + 1;
 }",
         );
+        check_assist_range(
+            introduce_variable,
+            "
+fn foo() {
+    <|>{ let x = 0; x }<|>
+    something_else();
+}",
+            "
+fn foo() {
+    let <|>var_name = { let x = 0; x };
+    something_else();
+}",
+        );
     }
 
     #[test]
@@ -169,6 +199,7 @@ fn foo() {
 
     #[test]
     fn test_introduce_var_last_expr() {
+        covers!(test_introduce_var_last_expr);
         check_assist_range(
             introduce_variable,
             "
@@ -181,10 +212,6 @@ fn foo() {
     bar(var_name)
 }",
         );
-    }
-
-    #[test]
-    fn test_introduce_var_last_full_expr() {
         check_assist_range(
             introduce_variable,
             "
@@ -196,24 +223,7 @@ fn foo() {
     let <|>var_name = bar(1 + 1);
     var_name
 }",
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_block_expr_second_to_last() {
-        check_assist_range(
-            introduce_variable,
-            "
-fn foo() {
-    <|>{ let x = 0; x }<|>
-    something_else();
-}",
-            "
-fn foo() {
-    let <|>var_name = { let x = 0; x };
-    something_else();
-}",
-        );
+        )
     }
 
     #[test]
@@ -309,11 +319,11 @@ fn main() {
 
     #[test]
     fn test_introduce_var_path_simple() {
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn main() {
-    let o = S<|>ome(true);
+    let o = <|>Some(true)<|>;
 }
 ",
             "
@@ -327,11 +337,11 @@ fn main() {
 
     #[test]
     fn test_introduce_var_path_method() {
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn main() {
-    let v = b<|>ar.foo();
+    let v = <|>bar.foo()<|>;
 }
 ",
             "
@@ -345,11 +355,11 @@ fn main() {
 
     #[test]
     fn test_introduce_var_return() {
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn foo() -> u32 {
-    r<|>eturn 2 + 2;
+    <|>return 2 + 2<|>;
 }
 ",
             "
@@ -363,13 +373,13 @@ fn foo() -> u32 {
 
     #[test]
     fn test_introduce_var_does_not_add_extra_whitespace() {
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn foo() -> u32 {
 
 
-    r<|>eturn 2 + 2;
+    <|>return 2 + 2<|>;
 }
 ",
             "
@@ -382,12 +392,12 @@ fn foo() -> u32 {
 ",
         );
 
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn foo() -> u32 {
 
-        r<|>eturn 2 + 2;
+        <|>return 2 + 2<|>;
 }
 ",
             "
@@ -399,7 +409,7 @@ fn foo() -> u32 {
 ",
         );
 
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn foo() -> u32 {
@@ -408,7 +418,7 @@ fn foo() -> u32 {
     // bar
 
 
-    r<|>eturn 2 + 2;
+    <|>return 2 + 2<|>;
 }
 ",
             "
@@ -427,12 +437,12 @@ fn foo() -> u32 {
 
     #[test]
     fn test_introduce_var_break() {
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn main() {
     let result = loop {
-        b<|>reak 2 + 2;
+        <|>break 2 + 2<|>;
     };
 }
 ",
@@ -449,11 +459,11 @@ fn main() {
 
     #[test]
     fn test_introduce_var_for_cast() {
-        check_assist(
+        check_assist_range(
             introduce_variable,
             "
 fn main() {
-    let v = 0f32 a<|>s u32;
+    let v = <|>0f32 as u32<|>;
 }
 ",
             "
@@ -467,57 +477,23 @@ fn main() {
 
     #[test]
     fn test_introduce_var_for_return_not_applicable() {
-        check_assist_not_applicable(
-            introduce_variable,
-            "
-fn foo() {
-    r<|>eturn;
-}
-",
-        );
+        check_assist_range_not_applicable(introduce_variable, "fn foo() { <|>return<|>; } ");
     }
 
     #[test]
     fn test_introduce_var_for_break_not_applicable() {
-        check_assist_not_applicable(
-            introduce_variable,
-            "
-fn main() {
-    loop {
-        b<|>reak;
-    };
-}
-",
-        );
-    }
-
-    #[test]
-    fn test_introduce_var_in_comment_not_applicable() {
-        check_assist_not_applicable(
+        check_assist_range_not_applicable(
             introduce_variable,
-            "
-fn main() {
-    let x = true;
-    let tuple = match x {
-        // c<|>omment
-        true => (2 + 2, true)
-        _ => (0, false)
-    };
-}
-",
+            "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(
+        check_assist_range_target(
             introduce_variable,
-            "
-fn foo() -> u32 {
-    r<|>eturn 2 + 2;
-}
-",
+            "fn foo() -> u32 { <|>return 2 + 2<|>; }",
             "2 + 2",
         );
 
diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs
index e1e899edc..6c3d75d79 100644
--- a/crates/ra_assists/src/lib.rs
+++ b/crates/ra_assists/src/lib.rs
@@ -6,6 +6,7 @@
 //! becomes available.
 
 mod assist_ctx;
+mod marks;
 
 use itertools::Itertools;
 
@@ -16,10 +17,16 @@ use hir::db::HirDatabase;
 
 pub(crate) use crate::assist_ctx::{AssistCtx, Assist};
 
+/// Unique identifier of the assist, should not be shown to the user
+/// directly.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct AssistId(pub &'static str);
+
 #[derive(Debug, Clone)]
 pub struct AssistLabel {
     /// Short description of the assist, as shown in the UI.
     pub label: String,
+    pub id: AssistId,
 }
 
 #[derive(Debug, Clone)]
@@ -253,6 +260,17 @@ mod helpers {
         let assist = AssistCtx::with_ctx(&db, frange, true, assist);
         assert!(assist.is_none());
     }
+
+    pub(crate) fn check_assist_range_not_applicable(
+        assist: fn(AssistCtx<MockDatabase>) -> Option<Assist>,
+        before: &str,
+    ) {
+        let (range, before) = extract_range(before);
+        let (db, _source_root, file_id) = MockDatabase::with_single_file(&before);
+        let frange = FileRange { file_id, range };
+        let assist = AssistCtx::with_ctx(&db, frange, true, assist);
+        assert!(assist.is_none());
+    }
 }
 
 #[cfg(test)]
@@ -260,7 +278,7 @@ mod tests {
     use hir::mock::MockDatabase;
     use ra_syntax::TextRange;
     use ra_db::FileRange;
-    use test_utils::{extract_offset};
+    use test_utils::{extract_offset, extract_range};
 
     #[test]
     fn assist_order_field_struct() {
@@ -280,16 +298,15 @@ mod tests {
     fn assist_order_if_expr() {
         let before = "
         pub fn test_some_range(a: int) -> bool {
-            if let 2..6 = 5<|> {
+            if let 2..6 = <|>5<|> {
                 true
             } else {
                 false
             }
         }";
-        let (before_cursor_pos, before) = extract_offset(before);
+        let (range, before) = extract_range(before);
         let (db, _source_root, file_id) = MockDatabase::with_single_file(&before);
-        let frange =
-            FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) };
+        let frange = FileRange { file_id, range };
         let assists = super::assists(&db, frange);
         let mut assists = assists.iter();
 
diff --git a/crates/ra_assists/src/marks.rs b/crates/ra_assists/src/marks.rs
new file mode 100644
index 000000000..a29f9f658
--- /dev/null
+++ b/crates/ra_assists/src/marks.rs
@@ -0,0 +1,5 @@
+test_utils::marks!(
+    introduce_var_in_comment_is_not_applicable
+    test_introduce_var_expr_stmt
+    test_introduce_var_last_expr
+);
diff --git a/crates/ra_assists/src/remove_dbg.rs b/crates/ra_assists/src/remove_dbg.rs
index 2bed270a1..6ea48d909 100644
--- a/crates/ra_assists/src/remove_dbg.rs
+++ b/crates/ra_assists/src/remove_dbg.rs
@@ -6,7 +6,7 @@ use ra_syntax::{
         L_PAREN, R_PAREN, L_CURLY, R_CURLY, L_BRACK, R_BRACK, EXCL
     },
 };
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn remove_dbg(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let macro_call = ctx.node_at_offset::<ast::MacroCall>()?;
@@ -46,7 +46,7 @@ pub(crate) fn remove_dbg(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist>
         macro_args.text().slice(start..end).to_string()
     };
 
-    ctx.add_action("remove dbg!()", |edit| {
+    ctx.add_action(AssistId("remove_dbg"), "remove dbg!()", |edit| {
         edit.target(macro_call.syntax().range());
         edit.replace(macro_range, macro_content);
         edit.set_cursor(cursor_pos);
diff --git a/crates/ra_assists/src/replace_if_let_with_match.rs b/crates/ra_assists/src/replace_if_let_with_match.rs
index 87a2c1185..230573499 100644
--- a/crates/ra_assists/src/replace_if_let_with_match.rs
+++ b/crates/ra_assists/src/replace_if_let_with_match.rs
@@ -2,7 +2,7 @@ use ra_syntax::{AstNode, ast};
 use ra_fmt::extract_trivial_expression;
 use hir::db::HirDatabase;
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn replace_if_let_with_match(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let if_expr: &ast::IfExpr = ctx.node_at_offset()?;
@@ -15,7 +15,7 @@ pub(crate) fn replace_if_let_with_match(mut ctx: AssistCtx<impl HirDatabase>) ->
         ast::ElseBranchFlavor::IfExpr(_) => return None,
     };
 
-    ctx.add_action("replace with match", |edit| {
+    ctx.add_action(AssistId("replace_if_let_with_match"), "replace with match", |edit| {
         let match_expr = build_match_expr(expr, pat, then_block, else_block);
         edit.target(if_expr.syntax().range());
         edit.replace_node_and_indent(if_expr.syntax(), match_expr);
diff --git a/crates/ra_assists/src/split_import.rs b/crates/ra_assists/src/split_import.rs
index f043be636..dd5be4e91 100644
--- a/crates/ra_assists/src/split_import.rs
+++ b/crates/ra_assists/src/split_import.rs
@@ -5,7 +5,7 @@ use ra_syntax::{
     algo::generate,
 };
 
-use crate::{AssistCtx, Assist};
+use crate::{AssistCtx, Assist, AssistId};
 
 pub(crate) fn split_import(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assist> {
     let colon_colon = ctx.leaf_at_offset().find(|leaf| leaf.kind() == COLONCOLON)?;
@@ -23,7 +23,7 @@ pub(crate) fn split_import(mut ctx: AssistCtx<impl HirDatabase>) -> Option<Assis
         None => top_path.syntax().range().end(),
     };
 
-    ctx.add_action("split import", |edit| {
+    ctx.add_action(AssistId("split_import"), "split import", |edit| {
         edit.target(colon_colon.range());
         edit.insert(l_curly, "{");
         edit.insert(r_curly, "}");
diff --git a/crates/ra_ide_api/src/assists.rs b/crates/ra_ide_api/src/assists.rs
index 7a9c66681..3c0475a51 100644
--- a/crates/ra_ide_api/src/assists.rs
+++ b/crates/ra_ide_api/src/assists.rs
@@ -2,20 +2,30 @@ use ra_db::{FileRange, FilePosition};
 
 use crate::{SourceFileEdit, SourceChange, db::RootDatabase};
 
-pub(crate) fn assists(db: &RootDatabase, frange: FileRange) -> Vec<SourceChange> {
+pub use ra_assists::AssistId;
+
+#[derive(Debug)]
+pub struct Assist {
+    pub id: AssistId,
+    pub change: SourceChange,
+}
+
+pub(crate) fn assists(db: &RootDatabase, frange: FileRange) -> Vec<Assist> {
     ra_assists::assists(db, frange)
         .into_iter()
         .map(|(label, action)| {
             let file_id = frange.file_id;
             let file_edit = SourceFileEdit { file_id, edit: action.edit };
-            SourceChange {
+            let id = label.id;
+            let change = SourceChange {
                 label: label.label,
                 source_file_edits: vec![file_edit],
                 file_system_edits: vec![],
                 cursor_position: action
                     .cursor_position
                     .map(|offset| FilePosition { offset, file_id }),
-            }
+            };
+            Assist { id, change }
         })
         .collect()
 }
diff --git a/crates/ra_ide_api/src/lib.rs b/crates/ra_ide_api/src/lib.rs
index 4b9fc9372..076a8396c 100644
--- a/crates/ra_ide_api/src/lib.rs
+++ b/crates/ra_ide_api/src/lib.rs
@@ -57,6 +57,7 @@ pub use crate::{
     runnables::{Runnable, RunnableKind},
     navigation_target::NavigationTarget,
     references::ReferenceSearchResult,
+    assists::{Assist, AssistId},
 };
 pub use ra_ide_api_light::{
     Fold, FoldKind, HighlightedRange, Severity, StructureNode, LocalEdit,
@@ -368,7 +369,7 @@ impl Analysis {
 
     /// Computes assists (aks code actons aka intentions) for the given
     /// position.
-    pub fn assists(&self, frange: FileRange) -> Cancelable<Vec<SourceChange>> {
+    pub fn assists(&self, frange: FileRange) -> Cancelable<Vec<Assist>> {
         self.with_db(|db| assists::assists(db, frange))
     }
 
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 9abd4054e..5da731801 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -1,6 +1,6 @@
 use gen_lsp_server::ErrorCode;
 use lsp_types::{
-    CodeActionResponse, CodeLens, Command, Diagnostic, DiagnosticSeverity,
+    CodeActionResponse, CodeLens, Command, Diagnostic, DiagnosticSeverity, CodeAction,
     DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange,
     FoldingRangeKind, FoldingRangeParams, Hover, HoverContents, Location, MarkupContent,
     MarkupKind, ParameterInformation, ParameterLabel, Position, PrepareRenameResponse, Range,
@@ -9,6 +9,7 @@ use lsp_types::{
 };
 use ra_ide_api::{
     FileId, FilePosition, FileRange, FoldKind, Query, RangeInfo, RunnableKind, Severity, Cancelable,
+    AssistId,
 };
 use ra_syntax::{AstNode, SyntaxKind, TextUnit};
 use rustc_hash::FxHashMap;
@@ -576,28 +577,57 @@ pub fn handle_code_action(
     let range = params.range.conv_with(&line_index);
 
     let assists = world.analysis().assists(FileRange { file_id, range })?.into_iter();
-    let fixes = world
-        .analysis()
-        .diagnostics(file_id)?
+    let diagnostics = world.analysis().diagnostics(file_id)?;
+    let mut res: Vec<CodeAction> = Vec::new();
+
+    let fixes_from_diagnostics = diagnostics
         .into_iter()
         .filter_map(|d| Some((d.range, d.fix?)))
         .filter(|(diag_range, _fix)| diag_range.intersection(&range).is_some())
         .map(|(_range, fix)| fix);
 
-    let mut res = Vec::new();
-    for source_edit in assists.chain(fixes) {
+    for source_edit in fixes_from_diagnostics {
         let title = source_edit.label.clone();
         let edit = source_edit.try_conv_with(&world)?;
 
-        let cmd = Command {
+        let command = Command {
+            title,
+            command: "rust-analyzer.applySourceChange".to_string(),
+            arguments: Some(vec![to_value(edit).unwrap()]),
+        };
+        let action = CodeAction {
+            title: command.title.clone(),
+            kind: None,
+            diagnostics: None,
+            edit: None,
+            command: Some(command),
+        };
+        res.push(action);
+    }
+
+    for assist in assists {
+        let title = assist.change.label.clone();
+        let edit = assist.change.try_conv_with(&world)?;
+
+        let command = Command {
             title,
             command: "rust-analyzer.applySourceChange".to_string(),
             arguments: Some(vec![to_value(edit).unwrap()]),
         };
-        res.push(cmd);
+        let action = CodeAction {
+            title: command.title.clone(),
+            kind: match assist.id {
+                AssistId("introduce_variable") => Some("refactor.extract.variable".to_string()),
+                _ => None,
+            },
+            diagnostics: None,
+            edit: None,
+            command: Some(command),
+        };
+        res.push(action);
     }
 
-    Ok(Some(CodeActionResponse::Commands(res)))
+    Ok(Some(CodeActionResponse::Actions(res)))
 }
 
 pub fn handle_code_lens(
diff --git a/crates/ra_lsp_server/tests/heavy_tests/main.rs b/crates/ra_lsp_server/tests/heavy_tests/main.rs
index e49c87169..996bf8e01 100644
--- a/crates/ra_lsp_server/tests/heavy_tests/main.rs
+++ b/crates/ra_lsp_server/tests/heavy_tests/main.rs
@@ -225,10 +225,12 @@ fn main() {}
             context: empty_context(),
         },
         json!([
-            {
+          {
+            "command": {
               "arguments": [
                 {
                   "cursorPosition": null,
+                  "label": "create module",
                   "workspaceEdit": {
                     "documentChanges": [
                       {
@@ -236,13 +238,14 @@ fn main() {}
                         "uri": "file:///[..]/src/bar.rs"
                       }
                     ]
-                  },
-                  "label": "create module"
+                  }
                 }
               ],
               "command": "rust-analyzer.applySourceChange",
               "title": "create module"
-            }
+            },
+            "title": "create module"
+          }
         ]),
     );