4 files changed, 154 insertions, 255 deletions

diff --git a/crates/ra_ide/src/completion/complete_keyword.rs b/crates/ra_ide/src/completion/complete_keyword.rs
index d016f0c7c..0ab330b73 100644
--- a/crates/ra_ide/src/completion/complete_keyword.rs
+++ b/crates/ra_ide/src/completion/complete_keyword.rs
@@ -36,184 +36,98 @@ pub(super) fn complete_use_tree_keyword(acc: &mut Completions, ctx: &CompletionC
     }
 }
 
-fn keyword(ctx: &CompletionContext, kw: &str, snippet: &str) -> CompletionItem {
-    let res = CompletionItem::new(CompletionKind::Keyword, ctx.source_range(), kw)
-        .kind(CompletionItemKind::Keyword);
-
-    match ctx.config.snippet_cap {
-        Some(cap) => res.insert_snippet(cap, snippet),
-        _ => res.insert_text(if snippet.contains('$') { kw } else { snippet }),
-    }
-    .build()
-}
-
-fn add_keyword(
-    ctx: &CompletionContext,
-    acc: &mut Completions,
-    kw: &str,
-    snippet: &str,
-    should_add: bool,
-) {
-    if should_add {
-        acc.add(keyword(ctx, kw, snippet));
-    }
-}
-
 pub(super) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionContext) {
     let has_trait_or_impl_parent = ctx.has_impl_parent || ctx.has_trait_parent;
     if ctx.trait_as_prev_sibling || ctx.impl_as_prev_sibling {
-        add_keyword(ctx, acc, "where", "where ", true);
+        add_keyword(ctx, acc, "where", "where ");
         return;
     }
     if ctx.unsafe_is_prev {
-        add_keyword(
-            ctx,
-            acc,
-            "fn",
-            "fn $0() {}",
-            ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent,
-        );
-        add_keyword(
-            ctx,
-            acc,
-            "trait",
-            "trait $0 {}",
-            (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-                || ctx.block_expr_parent,
-        );
-        add_keyword(
-            ctx,
-            acc,
-            "impl",
-            "impl $0 {}",
-            (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-                || ctx.block_expr_parent,
-        );
+        if ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent {
+            add_keyword(ctx, acc, "fn", "fn $0() {}")
+        }
+
+        if (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
+            || ctx.block_expr_parent
+        {
+            add_keyword(ctx, acc, "trait", "trait $0 {}");
+            add_keyword(ctx, acc, "impl", "impl $0 {}");
+        }
+
         return;
     }
-    add_keyword(
-        ctx,
-        acc,
-        "fn",
-        "fn $0() {}",
-        ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "use",
-        "use ",
-        (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-            || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "impl",
-        "impl $0 {}",
-        (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-            || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "trait",
-        "trait $0 {}",
-        (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-            || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "enum",
-        "enum $0 {}",
-        ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "struct",
-        "struct $0 {}",
-        ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "union",
-        "union $0 {}",
-        ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent,
-    );
-    add_keyword(ctx, acc, "match", "match $0 {}", ctx.block_expr_parent || ctx.is_match_arm);
-    add_keyword(ctx, acc, "loop", "loop {$0}", ctx.block_expr_parent || ctx.is_match_arm);
-    add_keyword(ctx, acc, "while", "while $0 {}", ctx.block_expr_parent);
-    add_keyword(ctx, acc, "let", "let ", ctx.if_is_prev || ctx.block_expr_parent);
-    add_keyword(ctx, acc, "if", "if ", ctx.if_is_prev || ctx.block_expr_parent || ctx.is_match_arm);
-    add_keyword(
-        ctx,
-        acc,
-        "if let",
-        "if let ",
-        ctx.if_is_prev || ctx.block_expr_parent || ctx.is_match_arm,
-    );
-    add_keyword(ctx, acc, "else", "else {$0}", ctx.after_if);
-    add_keyword(ctx, acc, "else if", "else if $0 {}", ctx.after_if);
-    add_keyword(
-        ctx,
-        acc,
-        "mod",
-        "mod $0 {}",
-        (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-            || ctx.block_expr_parent,
-    );
-    add_keyword(ctx, acc, "mut", "mut ", ctx.bind_pat_parent || ctx.ref_pat_parent);
-    add_keyword(
-        ctx,
-        acc,
-        "const",
-        "const ",
-        ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "type",
-        "type ",
-        ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "static",
-        "static ",
-        (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-            || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "extern",
-        "extern ",
-        (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
-            || ctx.block_expr_parent,
-    );
-    add_keyword(
-        ctx,
-        acc,
-        "unsafe",
-        "unsafe ",
-        ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent || ctx.is_match_arm,
-    );
-    add_keyword(ctx, acc, "continue", "continue;", ctx.in_loop_body && ctx.can_be_stmt);
-    add_keyword(ctx, acc, "break", "break;", ctx.in_loop_body && ctx.can_be_stmt);
-    add_keyword(ctx, acc, "continue", "continue", ctx.in_loop_body && !ctx.can_be_stmt);
-    add_keyword(ctx, acc, "break", "break", ctx.in_loop_body && !ctx.can_be_stmt);
-    add_keyword(
-        ctx,
-        acc,
-        "pub",
-        "pub ",
-        ctx.has_item_list_or_source_file_parent && !ctx.has_trait_parent,
-    );
+    if ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent {
+        add_keyword(ctx, acc, "fn", "fn $0() {}");
+    }
+    if (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
+        || ctx.block_expr_parent
+    {
+        add_keyword(ctx, acc, "use", "use ");
+        add_keyword(ctx, acc, "impl", "impl $0 {}");
+        add_keyword(ctx, acc, "trait", "trait $0 {}");
+    }
+
+    if ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent {
+        add_keyword(ctx, acc, "enum", "enum $0 {}");
+        add_keyword(ctx, acc, "struct", "struct $0 {}");
+        add_keyword(ctx, acc, "union", "union $0 {}");
+    }
+
+    if ctx.block_expr_parent || ctx.is_match_arm {
+        add_keyword(ctx, acc, "match", "match $0 {}");
+        add_keyword(ctx, acc, "loop", "loop {$0}");
+    }
+    if ctx.block_expr_parent {
+        add_keyword(ctx, acc, "while", "while $0 {}");
+    }
+    if ctx.if_is_prev || ctx.block_expr_parent {
+        add_keyword(ctx, acc, "let", "let ");
+    }
+    if ctx.if_is_prev || ctx.block_expr_parent || ctx.is_match_arm {
+        add_keyword(ctx, acc, "if", "if ");
+        add_keyword(ctx, acc, "if let", "if let ");
+    }
+    if ctx.after_if {
+        add_keyword(ctx, acc, "else", "else {$0}");
+        add_keyword(ctx, acc, "else if", "else if $0 {}");
+    }
+    if (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
+        || ctx.block_expr_parent
+    {
+        add_keyword(ctx, acc, "mod", "mod $0 {}");
+    }
+    if ctx.bind_pat_parent || ctx.ref_pat_parent {
+        add_keyword(ctx, acc, "mut", "mut ");
+    }
+    if ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent {
+        add_keyword(ctx, acc, "const", "const ");
+        add_keyword(ctx, acc, "type", "type ");
+    }
+    if (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
+        || ctx.block_expr_parent
+    {
+        add_keyword(ctx, acc, "static", "static ");
+    };
+    if (ctx.has_item_list_or_source_file_parent && !has_trait_or_impl_parent)
+        || ctx.block_expr_parent
+    {
+        add_keyword(ctx, acc, "extern", "extern ");
+    }
+    if ctx.has_item_list_or_source_file_parent || ctx.block_expr_parent || ctx.is_match_arm {
+        add_keyword(ctx, acc, "unsafe", "unsafe ");
+    }
+    if ctx.in_loop_body {
+        if ctx.can_be_stmt {
+            add_keyword(ctx, acc, "continue", "continue;");
+            add_keyword(ctx, acc, "break", "break;");
+        } else {
+            add_keyword(ctx, acc, "continue", "continue");
+            add_keyword(ctx, acc, "break", "break");
+        }
+    }
+    if ctx.has_item_list_or_source_file_parent && !ctx.has_trait_parent {
+        add_keyword(ctx, acc, "pub", "pub ")
+    }
 
     if !ctx.is_trivial_path {
         return;
@@ -225,6 +139,21 @@ pub(super) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
     acc.add_all(complete_return(ctx, &fn_def, ctx.can_be_stmt));
 }
 
+fn keyword(ctx: &CompletionContext, kw: &str, snippet: &str) -> CompletionItem {
+    let res = CompletionItem::new(CompletionKind::Keyword, ctx.source_range(), kw)
+        .kind(CompletionItemKind::Keyword);
+
+    match ctx.config.snippet_cap {
+        Some(cap) => res.insert_snippet(cap, snippet),
+        _ => res.insert_text(if snippet.contains('$') { kw } else { snippet }),
+    }
+    .build()
+}
+
+fn add_keyword(ctx: &CompletionContext, acc: &mut Completions, kw: &str, snippet: &str) {
+    acc.add(keyword(ctx, kw, snippet));
+}
+
 fn complete_return(
     ctx: &CompletionContext,
     fn_def: &ast::FnDef,
diff --git a/crates/ra_ide/src/completion/completion_item.rs b/crates/ra_ide/src/completion/completion_item.rs
index d1a4dd881..98348b349 100644
--- a/crates/ra_ide/src/completion/completion_item.rs
+++ b/crates/ra_ide/src/completion/completion_item.rs
@@ -126,8 +126,9 @@ pub enum CompletionItemKind {
 }
 
 impl CompletionItemKind {
-    pub fn tag(&self) -> String {
-        let tag = match self {
+    #[cfg(test)]
+    pub(crate) fn tag(&self) -> &'static str {
+        match self {
             CompletionItemKind::Snippet => "sn",
             CompletionItemKind::Keyword => "kw",
             CompletionItemKind::Module => "md",
@@ -146,8 +147,7 @@ impl CompletionItemKind {
             CompletionItemKind::TypeParam => "tp",
             CompletionItemKind::Macro => "ma",
             CompletionItemKind::Attribute => "at",
-        };
-        tag.to_owned()
+        }
     }
 }
 
diff --git a/crates/ra_ide/src/completion/patterns.rs b/crates/ra_ide/src/completion/patterns.rs
index fdcd3faa5..b2fe13280 100644
--- a/crates/ra_ide/src/completion/patterns.rs
+++ b/crates/ra_ide/src/completion/patterns.rs
@@ -8,6 +8,9 @@ use ra_syntax::{
     SyntaxNode, SyntaxToken,
 };
 
+#[cfg(test)]
+use crate::completion::test_utils::check_pattern_is_applicable;
+
 pub(crate) fn has_trait_parent(element: SyntaxElement) -> bool {
     not_same_range_ancestor(element)
         .filter(|it| it.kind() == ITEM_LIST)
@@ -15,6 +18,10 @@ pub(crate) fn has_trait_parent(element: SyntaxElement) -> bool {
         .filter(|it| it.kind() == TRAIT_DEF)
         .is_some()
 }
+#[test]
+fn test_has_trait_parent() {
+    check_pattern_is_applicable(r"trait A { f<|> }", has_trait_parent);
+}
 
 pub(crate) fn has_impl_parent(element: SyntaxElement) -> bool {
     not_same_range_ancestor(element)
@@ -23,20 +30,38 @@ pub(crate) fn has_impl_parent(element: SyntaxElement) -> bool {
         .filter(|it| it.kind() == IMPL_DEF)
         .is_some()
 }
+#[test]
+fn test_has_impl_parent() {
+    check_pattern_is_applicable(r"impl A { f<|> }", has_impl_parent);
+}
 
 pub(crate) fn has_block_expr_parent(element: SyntaxElement) -> bool {
     not_same_range_ancestor(element).filter(|it| it.kind() == BLOCK_EXPR).is_some()
 }
+#[test]
+fn test_has_block_expr_parent() {
+    check_pattern_is_applicable(r"fn my_fn() { let a = 2; f<|> }", has_block_expr_parent);
+}
 
 pub(crate) fn has_bind_pat_parent(element: SyntaxElement) -> bool {
     element.ancestors().find(|it| it.kind() == BIND_PAT).is_some()
 }
+#[test]
+fn test_has_bind_pat_parent() {
+    check_pattern_is_applicable(r"fn my_fn(m<|>) {}", has_bind_pat_parent);
+    check_pattern_is_applicable(r"fn my_fn() { let m<|> }", has_bind_pat_parent);
+}
 
 pub(crate) fn has_ref_parent(element: SyntaxElement) -> bool {
     not_same_range_ancestor(element)
         .filter(|it| it.kind() == REF_PAT || it.kind() == REF_EXPR)
         .is_some()
 }
+#[test]
+fn test_has_ref_parent() {
+    check_pattern_is_applicable(r"fn my_fn(&m<|>) {}", has_ref_parent);
+    check_pattern_is_applicable(r"fn my() { let &m<|> }", has_ref_parent);
+}
 
 pub(crate) fn has_item_list_or_source_file_parent(element: SyntaxElement) -> bool {
     let ancestor = not_same_range_ancestor(element);
@@ -45,6 +70,11 @@ pub(crate) fn has_item_list_or_source_file_parent(element: SyntaxElement) -> boo
     }
     ancestor.filter(|it| it.kind() == SOURCE_FILE || it.kind() == ITEM_LIST).is_some()
 }
+#[test]
+fn test_has_item_list_or_source_file_parent() {
+    check_pattern_is_applicable(r"i<|>", has_item_list_or_source_file_parent);
+    check_pattern_is_applicable(r"impl { f<|> }", has_item_list_or_source_file_parent);
+}
 
 pub(crate) fn is_match_arm(element: SyntaxElement) -> bool {
     not_same_range_ancestor(element.clone()).filter(|it| it.kind() == MATCH_ARM).is_some()
@@ -53,6 +83,10 @@ pub(crate) fn is_match_arm(element: SyntaxElement) -> bool {
             .filter(|it| it.kind() == FAT_ARROW)
             .is_some()
 }
+#[test]
+fn test_is_match_arm() {
+    check_pattern_is_applicable(r"fn my_fn() { match () { () => m<|> } }", is_match_arm);
+}
 
 pub(crate) fn unsafe_is_prev(element: SyntaxElement) -> bool {
     element
@@ -61,6 +95,10 @@ pub(crate) fn unsafe_is_prev(element: SyntaxElement) -> bool {
         .filter(|it| it.kind() == UNSAFE_KW)
         .is_some()
 }
+#[test]
+fn test_unsafe_is_prev() {
+    check_pattern_is_applicable(r"unsafe i<|>", unsafe_is_prev);
+}
 
 pub(crate) fn if_is_prev(element: SyntaxElement) -> bool {
     element
@@ -69,14 +107,26 @@ pub(crate) fn if_is_prev(element: SyntaxElement) -> bool {
         .filter(|it| it.kind() == IF_KW)
         .is_some()
 }
+#[test]
+fn test_if_is_prev() {
+    check_pattern_is_applicable(r"if l<|>", if_is_prev);
+}
 
 pub(crate) fn has_trait_as_prev_sibling(element: SyntaxElement) -> bool {
     previous_sibling_or_ancestor_sibling(element).filter(|it| it.kind() == TRAIT_DEF).is_some()
 }
+#[test]
+fn test_has_trait_as_prev_sibling() {
+    check_pattern_is_applicable(r"trait A w<|> {}", has_trait_as_prev_sibling);
+}
 
 pub(crate) fn has_impl_as_prev_sibling(element: SyntaxElement) -> bool {
     previous_sibling_or_ancestor_sibling(element).filter(|it| it.kind() == IMPL_DEF).is_some()
 }
+#[test]
+fn test_has_impl_as_prev_sibling() {
+    check_pattern_is_applicable(r"impl A w<|> {}", has_impl_as_prev_sibling);
+}
 
 pub(crate) fn is_in_loop_body(element: SyntaxElement) -> bool {
     let leaf = match element {
@@ -142,83 +192,3 @@ fn previous_sibling_or_ancestor_sibling(element: SyntaxElement) -> Option<Syntax
         non_trivia_sibling(NodeOrToken::Node(prev_sibling_node), Direction::Prev)
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::{
-        has_bind_pat_parent, has_block_expr_parent, has_impl_as_prev_sibling, has_impl_parent,
-        has_item_list_or_source_file_parent, has_ref_parent, has_trait_as_prev_sibling,
-        has_trait_parent, if_is_prev, is_match_arm, unsafe_is_prev,
-    };
-    use crate::completion::test_utils::check_pattern_is_applicable;
-
-    #[test]
-    fn test_unsafe_is_prev() {
-        check_pattern_is_applicable(r"unsafe i<|>", unsafe_is_prev);
-    }
-
-    #[test]
-    fn test_if_is_prev() {
-        check_pattern_is_applicable(r"if l<|>", if_is_prev);
-    }
-
-    #[test]
-    fn test_has_trait_parent() {
-        check_pattern_is_applicable(r"trait A { f<|> }", has_trait_parent);
-    }
-
-    #[test]
-    fn test_has_impl_parent() {
-        check_pattern_is_applicable(r"impl A { f<|> }", has_impl_parent);
-    }
-
-    #[test]
-    fn test_has_trait_as_prev_sibling() {
-        check_pattern_is_applicable(r"trait A w<|> {}", has_trait_as_prev_sibling);
-    }
-
-    #[test]
-    fn test_has_impl_as_prev_sibling() {
-        check_pattern_is_applicable(r"impl A w<|> {}", has_impl_as_prev_sibling);
-    }
-
-    #[test]
-    fn test_parent_block_expr() {
-        check_pattern_is_applicable(r"fn my_fn() { let a = 2; f<|> }", has_block_expr_parent);
-    }
-
-    #[test]
-    fn test_has_ref_pat_parent_in_func_parameters() {
-        check_pattern_is_applicable(r"fn my_fn(&m<|>) {}", has_ref_parent);
-    }
-
-    #[test]
-    fn test_has_ref_pat_parent_in_let_statement() {
-        check_pattern_is_applicable(r"fn my() { let &m<|> }", has_ref_parent);
-    }
-
-    #[test]
-    fn test_has_bind_pat_parent_in_func_parameters() {
-        check_pattern_is_applicable(r"fn my_fn(m<|>) {}", has_bind_pat_parent);
-    }
-
-    #[test]
-    fn test_has_bind_pat_parent_in_let_statement() {
-        check_pattern_is_applicable(r"fn my_fn() { let m<|> }", has_bind_pat_parent);
-    }
-
-    #[test]
-    fn test_is_match_arm() {
-        check_pattern_is_applicable(r"fn my_fn() { match () { () => m<|> } }", is_match_arm);
-    }
-
-    #[test]
-    fn test_has_source_file_parent() {
-        check_pattern_is_applicable(r"i<|>", has_item_list_or_source_file_parent);
-    }
-
-    #[test]
-    fn test_has_item_list_parent() {
-        check_pattern_is_applicable(r"impl { f<|> }", has_item_list_or_source_file_parent);
-    }
-}
diff --git a/crates/ra_ide/src/completion/test_utils.rs b/crates/ra_ide/src/completion/test_utils.rs
index 8b838a0a5..ba724116d 100644
--- a/crates/ra_ide/src/completion/test_utils.rs
+++ b/crates/ra_ide/src/completion/test_utils.rs
@@ -25,7 +25,7 @@ pub(crate) fn do_completion_with_options(
         .into_iter()
         .filter(|c| c.completion_kind == kind)
         .collect();
-    kind_completions.sort_by_key(|c| c.label().to_owned());
+    kind_completions.sort_by(|l, r| l.label().cmp(r.label()));
     kind_completions
 }