7 files changed, 195 insertions, 65 deletions

diff --git a/crates/ra_analysis/src/completion/complete_dot.rs b/crates/ra_analysis/src/completion/complete_dot.rs
index 93d657576..f24835d17 100644
--- a/crates/ra_analysis/src/completion/complete_dot.rs
+++ b/crates/ra_analysis/src/completion/complete_dot.rs
@@ -6,20 +6,9 @@ use crate::completion::{CompletionContext, Completions, CompletionKind, Completi
 
 /// Complete dot accesses, i.e. fields or methods (currently only fields).
 pub(super) fn complete_dot(acc: &mut Completions, ctx: &CompletionContext) -> Cancelable<()> {
-    let module = if let Some(module) = &ctx.module {
-        module
-    } else {
-        return Ok(());
-    };
-    let function = if let Some(fn_def) = ctx.enclosing_fn {
-        hir::source_binder::function_from_module(ctx.db, module, fn_def)
-    } else {
-        return Ok(());
-    };
-    let receiver = if let Some(receiver) = ctx.dot_receiver {
-        receiver
-    } else {
-        return Ok(());
+    let (function, receiver) = match (&ctx.function, ctx.dot_receiver) {
+        (Some(function), Some(receiver)) => (function, receiver),
+        _ => return Ok(()),
     };
     let infer_result = function.infer(ctx.db)?;
     let receiver_ty = if let Some(ty) = infer_result.type_of_node(receiver.syntax()) {
diff --git a/crates/ra_analysis/src/completion/complete_keyword.rs b/crates/ra_analysis/src/completion/complete_keyword.rs
index 5427fcb11..b2486104a 100644
--- a/crates/ra_analysis/src/completion/complete_keyword.rs
+++ b/crates/ra_analysis/src/completion/complete_keyword.rs
@@ -18,7 +18,7 @@ pub(super) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
     if !ctx.is_trivial_path {
         return;
     }
-    let fn_def = match ctx.enclosing_fn {
+    let fn_def = match ctx.function_syntax {
         Some(it) => it,
         None => return,
     };
@@ -32,10 +32,15 @@ pub(super) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         acc.add(keyword("else if", "else if $0 {}"));
     }
     if is_in_loop_body(ctx.leaf) {
-        acc.add(keyword("continue", "continue"));
-        acc.add(keyword("break", "break"));
+        if ctx.can_be_stmt {
+            acc.add(keyword("continue", "continue;"));
+            acc.add(keyword("break", "break;"));
+        } else {
+            acc.add(keyword("continue", "continue"));
+            acc.add(keyword("break", "break"));
+        }
     }
-    acc.add_all(complete_return(fn_def, ctx.is_stmt));
+    acc.add_all(complete_return(fn_def, ctx.can_be_stmt));
 }
 
 fn is_in_loop_body(leaf: SyntaxNodeRef) -> bool {
@@ -57,8 +62,8 @@ fn is_in_loop_body(leaf: SyntaxNodeRef) -> bool {
     false
 }
 
-fn complete_return(fn_def: ast::FnDef, is_stmt: bool) -> Option<CompletionItem> {
-    let snip = match (is_stmt, fn_def.ret_type().is_some()) {
+fn complete_return(fn_def: ast::FnDef, can_be_stmt: bool) -> Option<CompletionItem> {
+    let snip = match (can_be_stmt, fn_def.ret_type().is_some()) {
         (true, true) => "return $0;",
         (true, false) => "return;",
         (false, true) => "return $0",
@@ -75,7 +80,7 @@ mod tests {
     }
 
     #[test]
-    fn test_completion_kewords() {
+    fn completes_various_keywords_in_function() {
         check_keyword_completion(
             r"
             fn quux() {
@@ -87,13 +92,13 @@ mod tests {
             match "match $0 {}"
             while "while $0 {}"
             loop "loop {$0}"
-            return "return"
+            return "return;"
             "#,
         );
     }
 
     #[test]
-    fn test_completion_else() {
+    fn completes_else_after_if() {
         check_keyword_completion(
             r"
             fn quux() {
@@ -109,7 +114,7 @@ mod tests {
             loop "loop {$0}"
             else "else {$0}"
             else if "else if $0 {}"
-            return "return"
+            return "return;"
             "#,
         );
     }
@@ -149,7 +154,7 @@ mod tests {
     }
 
     #[test]
-    fn test_completion_return_no_stmt() {
+    fn dont_add_semi_after_return_if_not_a_statement() {
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -169,7 +174,27 @@ mod tests {
     }
 
     #[test]
-    fn test_continue_break_completion() {
+    fn last_return_in_block_has_semi() {
+        check_keyword_completion(
+            r"
+            fn quux() -> i32 {
+                if condition {
+                    <|>
+                }
+            }
+            ",
+            r#"
+            if "if $0 {}"
+            match "match $0 {}"
+            while "while $0 {}"
+            loop "loop {$0}"
+            return "return $0;"
+            "#,
+        );
+    }
+
+    #[test]
+    fn completes_break_and_continue_in_loops() {
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -181,11 +206,12 @@ mod tests {
             match "match $0 {}"
             while "while $0 {}"
             loop "loop {$0}"
-            continue "continue"
-            break "break"
-            return "return $0"
+            continue "continue;"
+            break "break;"
+            return "return $0;"
             "#,
         );
+        // No completion: lambda isolates control flow
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -197,8 +223,32 @@ mod tests {
             match "match $0 {}"
             while "while $0 {}"
             loop "loop {$0}"
-            return "return $0"
+            return "return $0;"
             "#,
         );
     }
+
+    #[test]
+    fn no_semi_after_break_continue_in_expr() {
+        check_keyword_completion(
+            r"
+            fn f() {
+                loop {
+                    match () {
+                        () => br<|>
+                    }
+                }
+            }
+            ",
+            r#"
+            if "if $0 {}"
+            match "match $0 {}"
+            while "while $0 {}"
+            loop "loop {$0}"
+            continue "continue"
+            break "break"
+            return "return"
+            "#,
+        )
+    }
 }
diff --git a/crates/ra_analysis/src/completion/complete_path.rs b/crates/ra_analysis/src/completion/complete_path.rs
index aaa2c7cee..4723a65a6 100644
--- a/crates/ra_analysis/src/completion/complete_path.rs
+++ b/crates/ra_analysis/src/completion/complete_path.rs
@@ -1,6 +1,6 @@
 use crate::{
     Cancelable,
-    completion::{CompletionItem, Completions, CompletionKind, CompletionContext},
+    completion::{CompletionItem, CompletionItemKind, Completions, CompletionKind, CompletionContext},
 };
 
 pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) -> Cancelable<()> {
@@ -12,16 +12,25 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) -> C
         Some(it) => it,
         None => return Ok(()),
     };
-    let target_module = match def_id.resolve(ctx.db)? {
-        hir::Def::Module(it) => it,
+    match def_id.resolve(ctx.db)? {
+        hir::Def::Module(module) => {
+            let module_scope = module.scope(ctx.db)?;
+            module_scope.entries().for_each(|(name, res)| {
+                CompletionItem::new(CompletionKind::Reference, name.to_string())
+                    .from_resolution(ctx, res)
+                    .add_to(acc)
+            });
+        }
+        hir::Def::Enum(e) => e
+            .variants(ctx.db)?
+            .into_iter()
+            .for_each(|(name, _variant)| {
+                CompletionItem::new(CompletionKind::Reference, name.to_string())
+                    .kind(CompletionItemKind::EnumVariant)
+                    .add_to(acc)
+            }),
         _ => return Ok(()),
     };
-    let module_scope = target_module.scope(ctx.db)?;
-    module_scope.entries().for_each(|(name, res)| {
-        CompletionItem::new(CompletionKind::Reference, name.to_string())
-            .from_resolution(ctx.db, res)
-            .add_to(acc)
-    });
     Ok(())
 }
 
@@ -92,4 +101,28 @@ mod tests {
             "Spam",
         );
     }
+
+    #[test]
+    fn completes_enum_variant() {
+        check_reference_completion(
+            "
+            //- /lib.rs
+            enum E { Foo, Bar(i32) }
+            fn foo() { let _ = E::<|> }
+            ",
+            "Foo;Bar",
+        );
+    }
+
+    #[test]
+    fn dont_render_function_parens_in_use_item() {
+        check_reference_completion(
+            "
+            //- /lib.rs
+            mod m { pub fn foo() {} }
+            use crate::m::f<|>;
+            ",
+            "foo",
+        )
+    }
 }
diff --git a/crates/ra_analysis/src/completion/complete_scope.rs b/crates/ra_analysis/src/completion/complete_scope.rs
index a57670e3b..daf666505 100644
--- a/crates/ra_analysis/src/completion/complete_scope.rs
+++ b/crates/ra_analysis/src/completion/complete_scope.rs
@@ -14,8 +14,7 @@ pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) ->
         Some(it) => it,
         None => return Ok(()),
     };
-    if let Some(fn_def) = ctx.enclosing_fn {
-        let function = hir::source_binder::function_from_module(ctx.db, module, fn_def);
+    if let Some(function) = &ctx.function {
         let scopes = function.scopes(ctx.db);
         complete_fn(acc, &scopes, ctx.offset);
     }
@@ -35,7 +34,7 @@ pub(super) fn complete_scope(acc: &mut Completions, ctx: &CompletionContext) ->
         })
         .for_each(|(name, res)| {
             CompletionItem::new(CompletionKind::Reference, name.to_string())
-                .from_resolution(ctx.db, res)
+                .from_resolution(ctx, res)
                 .add_to(acc)
         });
     Ok(())
@@ -75,7 +74,7 @@ mod tests {
                 let z = ();
             }
             ",
-            "y;x;quux",
+            r#"y;x;quux "quux($0)""#,
         );
     }
 
@@ -93,7 +92,7 @@ mod tests {
                 }
             }
             ",
-            "b;a;quux",
+            r#"b;a;quux "quux()$0""#,
         );
     }
 
@@ -107,7 +106,7 @@ mod tests {
                 }
             }
             ",
-            "x;quux",
+            r#"x;quux "quux()$0""#,
         );
     }
 
@@ -121,7 +120,7 @@ mod tests {
                 <|>
             }
             ",
-            "quux;Foo;Baz",
+            r#"quux "quux()$0";Foo;Baz"#,
         );
     }
 
@@ -135,7 +134,7 @@ mod tests {
                 fn quux() { <|> }
             }
             ",
-            "quux;Bar",
+            r#"quux "quux()$0";Bar"#,
         );
     }
 
@@ -146,12 +145,12 @@ mod tests {
             struct Foo;
             fn x() -> <|>
             ",
-            "Foo;x",
+            r#"Foo;x "x()$0""#,
         )
     }
 
     #[test]
-    fn dont_show_to_completions_for_shadowing() {
+    fn dont_show_both_completions_for_shadowing() {
         check_reference_completion(
             r"
             fn foo() -> {
@@ -162,7 +161,7 @@ mod tests {
                 }
             }
             ",
-            "bar;foo",
+            r#"bar;foo "foo()$0""#,
         )
     }
 
@@ -170,4 +169,24 @@ mod tests {
     fn completes_self_in_methods() {
         check_reference_completion(r"impl S { fn foo(&self) { <|> } }", "self")
     }
+
+    #[test]
+    fn inserts_parens_for_function_calls() {
+        check_reference_completion(
+            r"
+            fn no_args() {}
+            fn main() { no_<|> }
+            ",
+            r#"no_args "no_args()$0"
+               main "main()$0""#,
+        );
+        check_reference_completion(
+            r"
+            fn with_args(x: i32, y: String) {}
+            fn main() { with_<|> }
+            ",
+            r#"main "main()$0"
+               with_args "with_args($0)""#,
+        );
+    }
 }
diff --git a/crates/ra_analysis/src/completion/complete_snippet.rs b/crates/ra_analysis/src/completion/complete_snippet.rs
index fb9da0a4f..a495751dd 100644
--- a/crates/ra_analysis/src/completion/complete_snippet.rs
+++ b/crates/ra_analysis/src/completion/complete_snippet.rs
@@ -7,7 +7,7 @@ fn snippet(label: &str, snippet: &str) -> Builder {
 }
 
 pub(super) fn complete_expr_snippet(acc: &mut Completions, ctx: &CompletionContext) {
-    if !(ctx.is_trivial_path && ctx.enclosing_fn.is_some()) {
+    if !(ctx.is_trivial_path && ctx.function_syntax.is_some()) {
         return;
     }
     snippet("pd", "eprintln!(\"$0 = {:?}\", $0);").add_to(acc);
diff --git a/crates/ra_analysis/src/completion/completion_context.rs b/crates/ra_analysis/src/completion/completion_context.rs
index 978772fd4..4685c9328 100644
--- a/crates/ra_analysis/src/completion/completion_context.rs
+++ b/crates/ra_analysis/src/completion/completion_context.rs
@@ -22,14 +22,17 @@ pub(super) struct CompletionContext<'a> {
     pub(super) offset: TextUnit,
     pub(super) leaf: SyntaxNodeRef<'a>,
     pub(super) module: Option<hir::Module>,
-    pub(super) enclosing_fn: Option<ast::FnDef<'a>>,
+    pub(super) function: Option<hir::Function>,
+    pub(super) function_syntax: Option<ast::FnDef<'a>>,
+    pub(super) use_item_syntax: Option<ast::UseItem<'a>>,
     pub(super) is_param: bool,
     /// A single-indent path, like `foo`.
     pub(super) is_trivial_path: bool,
     /// If not a trivial, path, the prefix (qualifier).
     pub(super) path_prefix: Option<hir::Path>,
     pub(super) after_if: bool,
-    pub(super) is_stmt: bool,
+    /// `true` if we are a statement or a last expr in the block.
+    pub(super) can_be_stmt: bool,
     /// Something is typed at the "top" level, in module or impl/trait.
     pub(super) is_new_item: bool,
     /// The receiver if this is a field or method access, i.e. writing something.<|>
@@ -52,12 +55,14 @@ impl<'a> CompletionContext<'a> {
             leaf,
             offset: position.offset,
             module,
-            enclosing_fn: None,
+            function: None,
+            function_syntax: None,
+            use_item_syntax: None,
             is_param: false,
             is_trivial_path: false,
             path_prefix: None,
             after_if: false,
-            is_stmt: false,
+            can_be_stmt: false,
             is_new_item: false,
             dot_receiver: None,
             is_method_call: false,
@@ -112,11 +117,20 @@ impl<'a> CompletionContext<'a> {
             _ => (),
         }
 
-        self.enclosing_fn = self
+        self.use_item_syntax = self.leaf.ancestors().find_map(ast::UseItem::cast);
+
+        self.function_syntax = self
             .leaf
             .ancestors()
             .take_while(|it| it.kind() != SOURCE_FILE && it.kind() != MODULE)
             .find_map(ast::FnDef::cast);
+        match (&self.module, self.function_syntax) {
+            (Some(module), Some(fn_def)) => {
+                let function = source_binder::function_from_module(self.db, module, fn_def);
+                self.function = Some(function);
+            }
+            _ => (),
+        }
 
         let parent = match name_ref.syntax().parent() {
             Some(it) => it,
@@ -134,13 +148,22 @@ impl<'a> CompletionContext<'a> {
             if path.qualifier().is_none() {
                 self.is_trivial_path = true;
 
-                self.is_stmt = match name_ref
+                self.can_be_stmt = match name_ref
                     .syntax()
                     .ancestors()
                     .filter_map(ast::ExprStmt::cast)
                     .next()
                 {
-                    None => false,
+                    None => {
+                        name_ref
+                            .syntax()
+                            .ancestors()
+                            .filter_map(ast::Block::cast)
+                            .next()
+                            .and_then(|block| block.expr())
+                            .map(|e| e.syntax().range())
+                            == Some(name_ref.syntax().range())
+                    }
                     Some(expr_stmt) => expr_stmt.syntax().range() == name_ref.syntax().range(),
                 };
 
diff --git a/crates/ra_analysis/src/completion/completion_item.rs b/crates/ra_analysis/src/completion/completion_item.rs
index c9f9f495d..cd4d529f9 100644
--- a/crates/ra_analysis/src/completion/completion_item.rs
+++ b/crates/ra_analysis/src/completion/completion_item.rs
@@ -1,7 +1,7 @@
-use crate::db;
-
 use hir::PerNs;
 
+use crate::completion::CompletionContext;
+
 /// `CompletionItem` describes a single completion variant in the editor pop-up.
 /// It is basically a POD with various properties. To construct a
 /// `CompletionItem`, use `new` method and the `Builder` struct.
@@ -29,6 +29,7 @@ pub enum CompletionItemKind {
     Function,
     Struct,
     Enum,
+    EnumVariant,
     Binding,
     Field,
 }
@@ -117,12 +118,12 @@ impl Builder {
         self.kind = Some(kind);
         self
     }
-    pub(crate) fn from_resolution(
+    pub(super) fn from_resolution(
         mut self,
-        db: &db::RootDatabase,
+        ctx: &CompletionContext,
         resolution: &hir::Resolution,
     ) -> Builder {
-        let resolved = resolution.def_id.and_then(|d| d.resolve(db).ok());
+        let resolved = resolution.def_id.and_then(|d| d.resolve(ctx.db).ok());
         let kind = match resolved {
             PerNs {
                 types: Some(hir::Def::Module(..)),
@@ -137,14 +138,29 @@ impl Builder {
                 ..
             } => CompletionItemKind::Enum,
             PerNs {
-                values: Some(hir::Def::Function(..)),
+                values: Some(hir::Def::Function(function)),
                 ..
-            } => CompletionItemKind::Function,
+            } => return self.from_function(ctx, function),
             _ => return self,
         };
         self.kind = Some(kind);
         self
     }
+
+    fn from_function(mut self, ctx: &CompletionContext, function: hir::Function) -> Builder {
+        // If not an import, add parenthesis automatically.
+        if ctx.use_item_syntax.is_none() {
+            if let Some(sig_info) = function.signature_info(ctx.db) {
+                if sig_info.params.is_empty() {
+                    self.snippet = Some(format!("{}()$0", self.label));
+                } else {
+                    self.snippet = Some(format!("{}($0)", self.label));
+                }
+            }
+        }
+        self.kind = Some(CompletionItemKind::Function);
+        self
+    }
 }
 
 impl Into<CompletionItem> for Builder {