22 files changed, 299 insertions, 88 deletions

diff --git a/.gitignore b/.gitignore
index 76bc7bf57..68158e41c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,4 @@ crates/*/target
 .idea/*
 .vscode/*
 *.log
+*.iml
diff --git a/Cargo.lock b/Cargo.lock
index fa3363bd2..8e962d352 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -696,7 +696,7 @@ dependencies = [
  "rayon 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "relative-path 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "salsa 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "salsa 0.9.0 (git+https://github.com/matklad/salsa.git?branch=transitive-untracked)",
  "test_utils 0.1.0",
 ]
 
@@ -722,7 +722,7 @@ dependencies = [
  "ra_syntax 0.1.0",
  "relative-path 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "salsa 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "salsa 0.9.0 (git+https://github.com/matklad/salsa.git?branch=transitive-untracked)",
  "test_utils 0.1.0",
 ]
 
@@ -755,7 +755,7 @@ dependencies = [
  "ra_syntax 0.1.0",
  "relative-path 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-hash 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "salsa 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "salsa 0.9.0 (git+https://github.com/matklad/salsa.git?branch=transitive-untracked)",
  "test_utils 0.1.0",
 ]
 
@@ -790,6 +790,7 @@ dependencies = [
  "text_unit 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "thread_worker 0.1.0",
  "threadpool 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "tools 0.1.0",
  "url_serde 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "walkdir 2.2.7 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -1059,7 +1060,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "salsa"
 version = "0.9.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
+source = "git+https://github.com/matklad/salsa.git?branch=transitive-untracked#a2198f1f8a1d09894b842035a1af6b0b4c133c93"
 dependencies = [
  "derive-new 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1631,7 +1632,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum rusty-fork 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "9591f190d2852720b679c21f66ad929f9f1d7bb09d1193c26167586029d8489c"
 "checksum ryu 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "eb9e9b8cde282a9fe6a42dd4681319bfb63f121b8a8ee9439c6f4107e58a46f7"
 "checksum safemem 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8dca453248a96cb0749e36ccdfe2b0b4e54a61bfef89fb97ec621eb8e0a93dd9"
-"checksum salsa 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1b0246018bf824f282cb89c2ba9b9ef81093c563487c29ffafe20e1c129a6850"
+"checksum salsa 0.9.0 (git+https://github.com/matklad/salsa.git?branch=transitive-untracked)" = "<none>"
 "checksum same-file 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "8f20c4be53a8a1ff4c1f1b2bd14570d2f634628709752f0702ecdd2b3f9a5267"
 "checksum scopeguard 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "94258f53601af11e6a49f722422f6e3425c52b06245a5cf9bc09908b174f5e27"
 "checksum semver 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403"
diff --git a/Cargo.toml b/Cargo.toml
index 95830561d..dd8499c06 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -4,3 +4,6 @@ members = [ "crates/*" ]
 [profile.release]
 incremental = true
 debug = true
+
+[patch.'crates-io']
+salsa = { git = "https://github.com/matklad/salsa.git", branch = "transitive-untracked" }
diff --git a/crates/ra_analysis/src/completion/complete_keyword.rs b/crates/ra_analysis/src/completion/complete_keyword.rs
index d1e0a20a8..2869e67e0 100644
--- a/crates/ra_analysis/src/completion/complete_keyword.rs
+++ b/crates/ra_analysis/src/completion/complete_keyword.rs
@@ -35,7 +35,7 @@ pub(super) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         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 +57,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 +75,7 @@ mod tests {
     }
 
     #[test]
-    fn test_completion_kewords() {
+    fn completes_various_keywords_in_function() {
         check_keyword_completion(
             r"
             fn quux() {
@@ -87,13 +87,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 +109,7 @@ mod tests {
             loop "loop {$0}"
             else "else {$0}"
             else if "else if $0 {}"
-            return "return"
+            return "return;"
             "#,
         );
     }
@@ -149,7 +149,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 +169,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 {
@@ -183,9 +203,10 @@ mod tests {
             loop "loop {$0}"
             continue "continue"
             break "break"
-            return "return $0"
+            return "return $0;"
             "#,
         );
+        // No completion: lambda isolates control flow
         check_keyword_completion(
             r"
             fn quux() -> i32 {
@@ -197,7 +218,7 @@ mod tests {
             match "match $0 {}"
             while "while $0 {}"
             loop "loop {$0}"
-            return "return $0"
+            return "return $0;"
             "#,
         );
     }
diff --git a/crates/ra_analysis/src/completion/complete_path.rs b/crates/ra_analysis/src/completion/complete_path.rs
index c73a083a4..4723a65a6 100644
--- a/crates/ra_analysis/src/completion/complete_path.rs
+++ b/crates/ra_analysis/src/completion/complete_path.rs
@@ -17,7 +17,7 @@ pub(super) fn complete_path(acc: &mut Completions, ctx: &CompletionContext) -> C
             let module_scope = module.scope(ctx.db)?;
             module_scope.entries().for_each(|(name, res)| {
                 CompletionItem::new(CompletionKind::Reference, name.to_string())
-                    .from_resolution(ctx.db, res)
+                    .from_resolution(ctx, res)
                     .add_to(acc)
             });
         }
@@ -113,4 +113,16 @@ mod tests {
             "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 514fd2f88..daf666505 100644
--- a/crates/ra_analysis/src/completion/complete_scope.rs
+++ b/crates/ra_analysis/src/completion/complete_scope.rs
@@ -34,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(())
@@ -74,7 +74,7 @@ mod tests {
                 let z = ();
             }
             ",
-            "y;x;quux",
+            r#"y;x;quux "quux($0)""#,
         );
     }
 
@@ -92,7 +92,7 @@ mod tests {
                 }
             }
             ",
-            "b;a;quux",
+            r#"b;a;quux "quux()$0""#,
         );
     }
 
@@ -106,7 +106,7 @@ mod tests {
                 }
             }
             ",
-            "x;quux",
+            r#"x;quux "quux()$0""#,
         );
     }
 
@@ -120,7 +120,7 @@ mod tests {
                 <|>
             }
             ",
-            "quux;Foo;Baz",
+            r#"quux "quux()$0";Foo;Baz"#,
         );
     }
 
@@ -134,7 +134,7 @@ mod tests {
                 fn quux() { <|> }
             }
             ",
-            "quux;Bar",
+            r#"quux "quux()$0";Bar"#,
         );
     }
 
@@ -145,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() -> {
@@ -161,7 +161,7 @@ mod tests {
                 }
             }
             ",
-            "bar;foo",
+            r#"bar;foo "foo()$0""#,
         )
     }
 
@@ -169,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/completion_context.rs b/crates/ra_analysis/src/completion/completion_context.rs
index 71bf7fd32..4685c9328 100644
--- a/crates/ra_analysis/src/completion/completion_context.rs
+++ b/crates/ra_analysis/src/completion/completion_context.rs
@@ -24,13 +24,15 @@ pub(super) struct CompletionContext<'a> {
     pub(super) module: Option<hir::Module>,
     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.<|>
@@ -55,11 +57,12 @@ impl<'a> CompletionContext<'a> {
             module,
             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,
@@ -114,6 +117,8 @@ impl<'a> CompletionContext<'a> {
             _ => (),
         }
 
+        self.use_item_syntax = self.leaf.ancestors().find_map(ast::UseItem::cast);
+
         self.function_syntax = self
             .leaf
             .ancestors()
@@ -143,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 1d294c553..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.
@@ -118,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(..)),
@@ -138,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 {
diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index fcb4cd957..bff2e00c9 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -140,6 +140,9 @@ impl fmt::Debug for AnalysisImpl {
 }
 
 impl AnalysisImpl {
+    pub fn file_text(&self, file_id: FileId) -> Arc<String> {
+        self.db.file_text(file_id)
+    }
     pub fn file_syntax(&self, file_id: FileId) -> SourceFileNode {
         self.db.source_file(file_id)
     }
diff --git a/crates/ra_analysis/src/lib.rs b/crates/ra_analysis/src/lib.rs
index 67b1c1482..9f5e9f358 100644
--- a/crates/ra_analysis/src/lib.rs
+++ b/crates/ra_analysis/src/lib.rs
@@ -274,6 +274,9 @@ pub struct Analysis {
 }
 
 impl Analysis {
+    pub fn file_text(&self, file_id: FileId) -> Arc<String> {
+        self.imp.file_text(file_id)
+    }
     pub fn file_syntax(&self, file_id: FileId) -> SourceFileNode {
         self.imp.file_syntax(file_id).clone()
     }
diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index b41a7429a..5a8ca3b47 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -36,17 +36,17 @@ pub trait HirDatabase: SyntaxDatabase
 
     fn infer(def_id: DefId) -> Cancelable<Arc<InferenceResult>> {
         type InferQuery;
-        use fn query_definitions::infer;
+        use fn crate::ty::infer;
     }
 
     fn type_for_def(def_id: DefId) -> Cancelable<Ty> {
         type TypeForDefQuery;
-        use fn query_definitions::type_for_def;
+        use fn crate::ty::type_for_def;
     }
 
     fn type_for_field(def_id: DefId, field: Name) -> Cancelable<Ty> {
         type TypeForFieldQuery;
-        use fn query_definitions::type_for_field;
+        use fn crate::ty::type_for_field;
     }
 
     fn file_items(file_id: FileId) -> Arc<SourceFileItems> {
diff --git a/crates/ra_hir/src/query_definitions.rs b/crates/ra_hir/src/query_definitions.rs
index 016d86ee6..721bd4195 100644
--- a/crates/ra_hir/src/query_definitions.rs
+++ b/crates/ra_hir/src/query_definitions.rs
@@ -19,7 +19,6 @@ use crate::{
         imp::Submodule,
         nameres::{InputModuleItems, ItemMap, Resolver},
     },
-    ty::{self, InferenceResult, Ty},
     adt::{StructData, EnumData},
 };
 
@@ -30,19 +29,6 @@ pub(super) fn fn_scopes(db: &impl HirDatabase, def_id: DefId) -> Arc<FnScopes> {
     Arc::new(res)
 }
 
-pub(super) fn infer(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Arc<InferenceResult>> {
-    let function = Function::new(def_id);
-    ty::infer(db, function).map(Arc::new)
-}
-
-pub(super) fn type_for_def(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Ty> {
-    ty::type_for_def(db, def_id)
-}
-
-pub(super) fn type_for_field(db: &impl HirDatabase, def_id: DefId, field: Name) -> Cancelable<Ty> {
-    ty::type_for_field(db, def_id, field)
-}
-
 pub(super) fn struct_data(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Arc<StructData>> {
     let def_loc = def_id.loc(db);
     assert!(def_loc.kind == DefKind::Struct);
diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs
index 4ebd44d27..719b3f7cd 100644
--- a/crates/ra_hir/src/ty.rs
+++ b/crates/ra_hir/src/ty.rs
@@ -1,3 +1,18 @@
+//! The type system. We currently use this to infer types for completion.
+//!
+//! For type inference, compare the implementations in rustc (the various
+//! check_* methods in librustc_typeck/check/mod.rs are a good entry point) and
+//! IntelliJ-Rust (org.rust.lang.core.types.infer). Our entry point for
+//! inference here is the `infer` function, which infers the types of all
+//! expressions in a given function.
+//!
+//! The central struct here is `Ty`, which represents a type. During inference,
+//! it can contain type 'variables' which represent currently unknown types; as
+//! we walk through the expressions, we might determine that certain variables
+//! need to be equal to each other, or to certain types. To record this, we use
+//! the union-find implementation from the `ena` crate, which is extracted from
+//! rustc.
+
 mod primitive;
 #[cfg(test)]
 mod tests;
@@ -21,6 +36,7 @@ use crate::{
     type_ref::{TypeRef, Mutability},
 };
 
+/// The ID of a type variable.
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 pub struct TypeVarId(u32);
 
@@ -40,6 +56,8 @@ impl UnifyKey for TypeVarId {
     }
 }
 
+/// The value of a type variable: either we already know the type, or we don't
+/// know it yet.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum TypeVarValue {
     Known(Ty),
@@ -47,7 +65,7 @@ pub enum TypeVarValue {
 }
 
 impl TypeVarValue {
-    pub fn known(&self) -> Option<&Ty> {
+    fn known(&self) -> Option<&Ty> {
         match self {
             TypeVarValue::Known(ty) => Some(ty),
             TypeVarValue::Unknown => None,
@@ -75,10 +93,13 @@ impl UnifyValue for TypeVarValue {
     }
 }
 
+/// The kinds of placeholders we need during type inference. Currently, we only
+/// have type variables; in the future, we will probably also need int and float
+/// variables, for inference of literal values (e.g. `100` could be one of
+/// several integer types).
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
 pub enum InferTy {
     TypeVar(TypeVarId),
-    // later we'll have IntVar and FloatVar as well
 }
 
 /// When inferring an expression, we propagate downward whatever type hint we
@@ -92,15 +113,21 @@ struct Expectation {
 }
 
 impl Expectation {
+    /// The expectation that the type of the expression needs to equal the given
+    /// type.
     fn has_type(ty: Ty) -> Self {
         Expectation { ty }
     }
 
+    /// This expresses no expectation on the type.
     fn none() -> Self {
         Expectation { ty: Ty::Unknown }
     }
 }
 
+/// A type. This is based on the `TyKind` enum in rustc (librustc/ty/sty.rs).
+///
+/// This should be cheap to clone.
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
 pub enum Ty {
     /// The primitive boolean type. Written as `bool`.
@@ -134,14 +161,14 @@ pub enum Ty {
     // An array with the given length. Written as `[T; n]`.
     // Array(Ty, ty::Const),
     /// The pointee of an array slice.  Written as `[T]`.
-    Slice(TyRef),
+    Slice(Arc<Ty>),
 
     /// A raw pointer. Written as `*mut T` or `*const T`
-    RawPtr(TyRef, Mutability),
+    RawPtr(Arc<Ty>, Mutability),
 
     /// A reference; a pointer with an associated lifetime. Written as
     /// `&'a mut T` or `&'a T`.
-    Ref(TyRef, Mutability),
+    Ref(Arc<Ty>, Mutability),
 
     /// A pointer to a function.  Written as `fn() -> i32`.
     ///
@@ -153,6 +180,10 @@ pub enum Ty {
     /// ```
     FnPtr(Arc<FnSig>),
 
+    // rustc has a separate type for each function, which just coerces to the
+    // above function pointer type. Once we implement generics, we will probably
+    // need this as well.
+
     // A trait, defined with `dyn trait`.
     // Dynamic(),
     // The anonymous type of a closure. Used to represent the type of
@@ -166,7 +197,7 @@ pub enum Ty {
     // A type representin the types stored inside a generator.
     // This should only appear in GeneratorInteriors.
     // GeneratorWitness(Binder<&'tcx List<Ty<'tcx>>>),
-    /// The never type `!`
+    /// The never type `!`.
     Never,
 
     /// A tuple type.  For example, `(i32, bool)`.
@@ -177,10 +208,6 @@ pub enum Ty {
     // Projection(ProjectionTy),
 
     // Opaque (`impl Trait`) type found in a return type.
-    // The `DefId` comes either from
-    // * the `impl Trait` ast::Ty node,
-    // * or the `existential type` declaration
-    // The substitutions are for the generics of the function in question.
     // Opaque(DefId, Substs),
 
     // A type parameter; for example, `T` in `fn f<T>(x: T) {}
@@ -192,12 +219,12 @@ pub enum Ty {
     /// A placeholder for a type which could not be computed; this is propagated
     /// to avoid useless error messages. Doubles as a placeholder where type
     /// variables are inserted before type checking, since we want to try to
-    /// infer a better type here anyway.
+    /// infer a better type here anyway -- for the IDE use case, we want to try
+    /// to infer as much as possible even in the presence of type errors.
     Unknown,
 }
 
-type TyRef = Arc<Ty>;
-
+/// A function signature.
 #[derive(Clone, PartialEq, Eq, Hash, Debug)]
 pub struct FnSig {
     input: Vec<Ty>,
@@ -368,7 +395,11 @@ impl fmt::Display for Ty {
     }
 }
 
-pub fn type_for_fn(db: &impl HirDatabase, f: Function) -> Cancelable<Ty> {
+// Functions returning declared types for items
+
+/// Compute the declared type of a function. This should not need to look at the
+/// function body (but currently uses the function AST, so does anyway - TODO).
+fn type_for_fn(db: &impl HirDatabase, f: Function) -> Cancelable<Ty> {
     let syntax = f.syntax(db);
     let module = f.module(db)?;
     let node = syntax.borrowed();
@@ -390,7 +421,7 @@ pub fn type_for_fn(db: &impl HirDatabase, f: Function) -> Cancelable<Ty> {
     Ok(Ty::FnPtr(Arc::new(sig)))
 }
 
-pub fn type_for_struct(db: &impl HirDatabase, s: Struct) -> Cancelable<Ty> {
+fn type_for_struct(db: &impl HirDatabase, s: Struct) -> Cancelable<Ty> {
     Ok(Ty::Adt {
         def_id: s.def_id(),
         name: s.name(db)?.unwrap_or_else(Name::missing),
@@ -404,7 +435,7 @@ pub fn type_for_enum(db: &impl HirDatabase, s: Enum) -> Cancelable<Ty> {
     })
 }
 
-pub fn type_for_def(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Ty> {
+pub(super) fn type_for_def(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Ty> {
     let def = def_id.resolve(db)?;
     match def {
         Def::Module(..) => {
@@ -444,19 +475,25 @@ pub(super) fn type_for_field(db: &impl HirDatabase, def_id: DefId, field: Name)
     Ty::from_hir(db, &module, &type_ref)
 }
 
+/// The result of type inference: A mapping from expressions and patterns to types.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub struct InferenceResult {
     type_of: FxHashMap<LocalSyntaxPtr, Ty>,
 }
 
 impl InferenceResult {
+    /// Returns the type of the given syntax node, if it was inferred. Will
+    /// return `None` for syntax nodes not in the inferred function or not
+    /// pointing to an expression/pattern, `Some(Ty::Unknown)` for
+    /// expressions/patterns that could not be inferred.
     pub fn type_of_node(&self, node: SyntaxNodeRef) -> Option<Ty> {
         self.type_of.get(&LocalSyntaxPtr::new(node)).cloned()
     }
 }
 
+/// The inference context contains all information needed during type inference.
 #[derive(Clone, Debug)]
-pub struct InferenceContext<'a, D: HirDatabase> {
+struct InferenceContext<'a, D: HirDatabase> {
     db: &'a D,
     scopes: Arc<FnScopes>,
     module: Module,
@@ -738,6 +775,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
             ast::Expr::ParenExpr(e) => self.infer_expr_opt(e.expr(), expected)?,
             ast::Expr::Label(_e) => Ty::Unknown,
             ast::Expr::ReturnExpr(e) => {
+                // TODO expect return type of function
                 self.infer_expr_opt(e.expr(), &Expectation::none())?;
                 Ty::Never
             }
@@ -870,7 +908,8 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
     }
 }
 
-pub fn infer(db: &impl HirDatabase, function: Function) -> Cancelable<InferenceResult> {
+pub fn infer(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Arc<InferenceResult>> {
+    let function = Function::new(def_id); // TODO: consts also need inference
     let scopes = function.scopes(db);
     let module = function.module(db)?;
     let mut ctx = InferenceContext::new(db, scopes, module);
@@ -909,5 +948,5 @@ pub fn infer(db: &impl HirDatabase, function: Function) -> Cancelable<InferenceR
         ctx.infer_block(block, &Expectation::has_type(ret_ty))?;
     }
 
-    Ok(ctx.resolve_all())
+    Ok(Arc::new(ctx.resolve_all()))
 }
diff --git a/crates/ra_lsp_server/Cargo.toml b/crates/ra_lsp_server/Cargo.toml
index c53106a62..646df2497 100644
--- a/crates/ra_lsp_server/Cargo.toml
+++ b/crates/ra_lsp_server/Cargo.toml
@@ -37,3 +37,4 @@ ra_vfs = { path = "../ra_vfs" }
 [dev-dependencies]
 tempdir = "0.3.7"
 test_utils = { path = "../test_utils" }
+tools = { path = "../tools" }
diff --git a/crates/ra_lsp_server/src/caps.rs b/crates/ra_lsp_server/src/caps.rs
index 5f7038f63..8d508a3ba 100644
--- a/crates/ra_lsp_server/src/caps.rs
+++ b/crates/ra_lsp_server/src/caps.rs
@@ -33,7 +33,7 @@ pub fn server_capabilities() -> ServerCapabilities {
         workspace_symbol_provider: Some(true),
         code_action_provider: Some(CodeActionProviderCapability::Simple(true)),
         code_lens_provider: None,
-        document_formatting_provider: None,
+        document_formatting_provider: Some(true),
         document_range_formatting_provider: None,
         document_on_type_formatting_provider: Some(DocumentOnTypeFormattingOptions {
             first_trigger_character: "=".to_string(),
diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs
index 1edb9fae4..97c1be778 100644
--- a/crates/ra_lsp_server/src/main_loop.rs
+++ b/crates/ra_lsp_server/src/main_loop.rs
@@ -295,6 +295,7 @@ fn on_request(
         .on::<req::PrepareRenameRequest>(handlers::handle_prepare_rename)?
         .on::<req::Rename>(handlers::handle_rename)?
         .on::<req::References>(handlers::handle_references)?
+        .on::<req::Formatting>(handlers::handle_formatting)?
         .finish();
     match req {
         Ok(id) => {
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index d6f3dbe28..a2c12a4c1 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -6,13 +6,16 @@ use languageserver_types::{
     DiagnosticSeverity, DocumentSymbol, Documentation, FoldingRange, FoldingRangeKind,
     FoldingRangeParams, Location, MarkupContent, MarkupKind, MarkedString, Position,
     PrepareRenameResponse, RenameParams, SymbolInformation, TextDocumentIdentifier, TextEdit,
+    Range,
     WorkspaceEdit, ParameterInformation, ParameterLabel, SignatureInformation, Hover, HoverContents,
+    DocumentFormattingParams,
 };
 use ra_analysis::{FileId, FoldKind, Query, RunnableKind, FileRange, FilePosition, Severity};
 use ra_syntax::{TextUnit, text_utils::intersect};
 use ra_text_edit::text_utils::contains_offset_nonstrict;
 use rustc_hash::FxHashMap;
 use serde_json::to_value;
+use std::io::Write;
 
 use crate::{
     conv::{to_location, Conv, ConvWith, MapConvWith, TryConvWith},
@@ -601,6 +604,40 @@ pub fn handle_references(
     ))
 }
 
+pub fn handle_formatting(
+    world: ServerWorld,
+    params: DocumentFormattingParams,
+) -> Result<Option<Vec<TextEdit>>> {
+    let file_id = params.text_document.try_conv_with(&world)?;
+    let file = world.analysis().file_text(file_id);
+
+    let file_line_index = world.analysis().file_line_index(file_id);
+    let end_position = TextUnit::of_str(&file).conv_with(&file_line_index);
+
+    use std::process;
+    let mut rustfmt = process::Command::new("rustfmt")
+        .stdin(process::Stdio::piped())
+        .stdout(process::Stdio::piped())
+        .spawn()?;
+
+    rustfmt.stdin.as_mut().unwrap().write_all(file.as_bytes())?;
+
+    let output = rustfmt.wait_with_output()?;
+    let captured_stdout = String::from_utf8(output.stdout)?;
+    if !output.status.success() {
+        failure::bail!(
+            "rustfmt exited with error code {}: {}.",
+            output.status,
+            captured_stdout,
+        );
+    }
+
+    Ok(Some(vec![TextEdit {
+        range: Range::new(Position::new(0, 0), end_position),
+        new_text: captured_stdout,
+    }]))
+}
+
 pub fn handle_code_action(
     world: ServerWorld,
     params: req::CodeActionParams,
diff --git a/crates/ra_lsp_server/tests/heavy_tests/main.rs b/crates/ra_lsp_server/tests/heavy_tests/main.rs
index 1f5cc5e8b..b0e1e65b6 100644
--- a/crates/ra_lsp_server/tests/heavy_tests/main.rs
+++ b/crates/ra_lsp_server/tests/heavy_tests/main.rs
@@ -1,8 +1,8 @@
 mod support;
 
 use serde_json::json;
-use ra_lsp_server::req::{Runnables, RunnablesParams, CodeActionRequest, CodeActionParams};
-use languageserver_types::{Position, Range, CodeActionContext};
+use ra_lsp_server::req::{Runnables, RunnablesParams, CodeActionRequest, CodeActionParams, Formatting};
+use languageserver_types::{Position, Range, CodeActionContext, DocumentFormattingParams, FormattingOptions};
 
 use crate::support::project;
 
@@ -118,6 +118,60 @@ fn test_eggs() {}
     );
 }
 
+use std::collections::HashMap;
+#[test]
+fn test_format_document() {
+    tools::install_rustfmt().unwrap();
+
+    let server = project(
+        r#"
+[package]
+name = "foo"
+version = "0.0.0"
+
+//- src/lib.rs
+mod bar;
+
+fn main() {
+}
+
+pub use std::collections::HashMap;
+"#,
+    );
+    server.wait_for_feedback("workspace loaded");
+
+    server.request::<Formatting>(
+        DocumentFormattingParams {
+            text_document: server.doc_id("src/lib.rs"),
+            options: FormattingOptions {
+                tab_size: 4,
+                insert_spaces: false,
+                properties: HashMap::new(),
+            },
+        },
+        json!([
+            {
+                "newText": r#"mod bar;
+
+fn main() {}
+
+pub use std::collections::HashMap;
+"#,
+                "range": {
+                    "end": {
+                        "character": 0,
+                        "line": 6
+                    },
+                    "start": {
+                        "character": 0,
+                        "line": 0
+                    }
+                }
+            }
+        ]),
+    );
+}
+
 #[test]
 fn test_missing_module_code_action() {
     let server = project(
diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs
index be400bae9..4cfdb83da 100644
--- a/crates/ra_vfs/src/io.rs
+++ b/crates/ra_vfs/src/io.rs
@@ -8,7 +8,7 @@ use walkdir::{DirEntry, WalkDir};
 use thread_worker::{WorkerHandle};
 use relative_path::RelativePathBuf;
 
-use crate::VfsRoot;
+use crate::{VfsRoot, has_rs_extension};
 
 pub(crate) struct Task {
     pub(crate) root: VfsRoot,
@@ -59,7 +59,7 @@ fn load_root(root: &Path, filter: &dyn Fn(&DirEntry) -> bool) -> Vec<(RelativePa
             continue;
         }
         let path = entry.path();
-        if path.extension().and_then(|os| os.to_str()) != Some("rs") {
+        if !has_rs_extension(path) {
             continue;
         }
         let text = match fs::read_to_string(path) {
diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs
index 4de07b093..90d5e21f4 100644
--- a/crates/ra_vfs/src/lib.rs
+++ b/crates/ra_vfs/src/lib.rs
@@ -2,11 +2,13 @@
 //!
 //! When doing analysis, we don't want to do any IO, we want to keep all source
 //! code in memory. However, the actual source code is stored on disk, so you
-//! component which does this.
 //! need to get it into the memory in the first place somehow. VFS is the
+//! component which does this.
 //!
-//! It also is responsible for watching the disk for changes, and for merging
+//! It is also responsible for watching the disk for changes, and for merging
 //! editor state (modified, unsaved files) with disk state.
+//! TODO: Some LSP clients support watching the disk, so this crate should
+//! to support custom watcher events (related to https://github.com/rust-analyzer/rust-analyzer/issues/131)
 //!
 //! VFS is based on a concept of roots: a set of directories on the file system
 //! whihc are watched for changes. Typically, there will be a root for each
@@ -29,7 +31,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
 use relative_path::RelativePathBuf;
 use crossbeam_channel::Receiver;
 use walkdir::DirEntry;
-use thread_worker::{WorkerHandle};
+use thread_worker::WorkerHandle;
 
 use crate::{
     arena::{ArenaId, Arena},
@@ -57,12 +59,8 @@ impl RootFilter {
         if !(self.file_filter)(path) {
             return None;
         }
-        if !(path.starts_with(&self.root)) {
-            return None;
-        }
-        let path = path.strip_prefix(&self.root).unwrap();
-        let path = RelativePathBuf::from_path(path).unwrap();
-        Some(path)
+        let path = path.strip_prefix(&self.root).ok()?;
+        RelativePathBuf::from_path(path).ok()
     }
 }
 
diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs
index 6f96b8120..e5b32c25c 100644
--- a/crates/tools/src/lib.rs
+++ b/crates/tools/src/lib.rs
@@ -117,7 +117,7 @@ pub fn run_rustfmt(mode: Mode) -> Result<()> {
     Ok(())
 }
 
-fn install_rustfmt() -> Result<()> {
+pub fn install_rustfmt() -> Result<()> {
     run(&format!("rustup install {}", TOOLCHAIN), ".")?;
     run(
         &format!("rustup component add rustfmt --toolchain {}", TOOLCHAIN),
diff --git a/editors/README.md b/editors/README.md
index ad2ce1695..a63ced725 100644
--- a/editors/README.md
+++ b/editors/README.md
@@ -45,6 +45,8 @@ It's better to remove existing Rust plugins to avoid interference.
     `#[test]`, this action runs this specific test. If the cursor is
     outside of the test function, this re-runs the last test. Do bind
     this to a shortcut!
+  - **Format document**. Formats the current file with rustfmt.
+    Rustfmt must be installed separately with `rustup component add rustfmt`.
 
 * Typing assists
   - typing `let =` tries to smartly add `;` if `=` is followed by an existing expression.