7 files changed, 200 insertions, 146 deletions

diff --git a/crates/ide_assists/src/assist_context.rs b/crates/ide_assists/src/assist_context.rs
index 19e9f179e..112939948 100644
--- a/crates/ide_assists/src/assist_context.rs
+++ b/crates/ide_assists/src/assist_context.rs
@@ -187,7 +187,29 @@ pub(crate) struct AssistBuilder {
     source_change: SourceChange,
 
     /// Maps the original, immutable `SyntaxNode` to a `clone_for_update` twin.
-    mutated_tree: Option<(SyntaxNode, SyntaxNode)>,
+    mutated_tree: Option<TreeMutator>,
+}
+
+pub(crate) struct TreeMutator {
+    immutable: SyntaxNode,
+    mutable_clone: SyntaxNode,
+}
+
+impl TreeMutator {
+    pub(crate) fn new(immutable: &SyntaxNode) -> TreeMutator {
+        let immutable = immutable.ancestors().last().unwrap();
+        let mutable_clone = immutable.clone_for_update();
+        TreeMutator { immutable, mutable_clone }
+    }
+
+    pub(crate) fn make_mut<N: AstNode>(&self, node: &N) -> N {
+        N::cast(self.make_syntax_mut(node.syntax())).unwrap()
+    }
+
+    pub(crate) fn make_syntax_mut(&self, node: &SyntaxNode) -> SyntaxNode {
+        let ptr = SyntaxNodePtr::new(node);
+        ptr.to_node(&self.mutable_clone)
+    }
 }
 
 impl AssistBuilder {
@@ -206,8 +228,8 @@ impl AssistBuilder {
     }
 
     fn commit(&mut self) {
-        if let Some((old, new)) = self.mutated_tree.take() {
-            algo::diff(&old, &new).into_text_edit(&mut self.edit)
+        if let Some(tm) = self.mutated_tree.take() {
+            algo::diff(&tm.immutable, &tm.mutable_clone).into_text_edit(&mut self.edit)
         }
 
         let edit = mem::take(&mut self.edit).finish();
@@ -230,16 +252,7 @@ impl AssistBuilder {
     /// phase, and then get their mutable couterparts using `make_mut` in the
     /// mutable state.
     pub(crate) fn make_mut(&mut self, node: SyntaxNode) -> SyntaxNode {
-        let root = &self
-            .mutated_tree
-            .get_or_insert_with(|| {
-                let immutable = node.ancestors().last().unwrap();
-                let mutable = immutable.clone_for_update();
-                (immutable, mutable)
-            })
-            .1;
-        let ptr = SyntaxNodePtr::new(&&node);
-        ptr.to_node(root)
+        self.mutated_tree.get_or_insert_with(|| TreeMutator::new(&node)).make_syntax_mut(&node)
     }
 
     /// Remove specified `range` of text.
diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs
index b30652a9d..93b28370c 100644
--- a/crates/ide_assists/src/handlers/extract_function.rs
+++ b/crates/ide_assists/src/handlers/extract_function.rs
@@ -16,12 +16,13 @@ use syntax::{
         edit::{AstNodeEdit, IndentLevel},
         AstNode,
     },
+    ted,
     SyntaxKind::{self, BLOCK_EXPR, BREAK_EXPR, COMMENT, PATH_EXPR, RETURN_EXPR},
     SyntaxNode, SyntaxToken, TextRange, TextSize, TokenAtOffset, WalkEvent, T,
 };
 
 use crate::{
-    assist_context::{AssistContext, Assists},
+    assist_context::{AssistContext, Assists, TreeMutator},
     AssistId,
 };
 
@@ -1366,7 +1367,10 @@ fn rewrite_body_segment(
 
 /// change all usages to account for added `&`/`&mut` for some params
 fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode) -> SyntaxNode {
-    let mut rewriter = SyntaxRewriter::default();
+    let mut usages_for_param: Vec<(&Param, Vec<ast::Expr>)> = Vec::new();
+
+    let tm = TreeMutator::new(syntax);
+
     for param in params {
         if !param.kind().is_ref() {
             continue;
@@ -1376,30 +1380,39 @@ fn fix_param_usages(ctx: &AssistContext, params: &[Param], syntax: &SyntaxNode)
         let usages = usages
             .iter()
             .filter(|reference| syntax.text_range().contains_range(reference.range))
-            .filter_map(|reference| path_element_of_reference(syntax, reference));
-        for path in usages {
-            match path.syntax().ancestors().skip(1).find_map(ast::Expr::cast) {
+            .filter_map(|reference| path_element_of_reference(syntax, reference))
+            .map(|expr| tm.make_mut(&expr));
+
+        usages_for_param.push((param, usages.collect()));
+    }
+
+    let res = tm.make_syntax_mut(syntax);
+
+    for (param, usages) in usages_for_param {
+        for usage in usages {
+            match usage.syntax().ancestors().skip(1).find_map(ast::Expr::cast) {
                 Some(ast::Expr::MethodCallExpr(_)) | Some(ast::Expr::FieldExpr(_)) => {
                     // do nothing
                 }
                 Some(ast::Expr::RefExpr(node))
                     if param.kind() == ParamKind::MutRef && node.mut_token().is_some() =>
                 {
-                    rewriter.replace_ast(&node.clone().into(), &node.expr().unwrap());
+                    ted::replace(node.syntax(), node.expr().unwrap().syntax());
                 }
                 Some(ast::Expr::RefExpr(node))
                     if param.kind() == ParamKind::SharedRef && node.mut_token().is_none() =>
                 {
-                    rewriter.replace_ast(&node.clone().into(), &node.expr().unwrap());
+                    ted::replace(node.syntax(), node.expr().unwrap().syntax());
                 }
                 Some(_) | None => {
-                    rewriter.replace_ast(&path, &make::expr_prefix(T![*], path.clone()));
+                    let p = &make::expr_prefix(T![*], usage.clone()).clone_for_update();
+                    ted::replace(usage.syntax(), p.syntax())
                 }
-            };
+            }
         }
     }
 
-    rewriter.rewrite(syntax)
+    res
 }
 
 fn update_external_control_flow(handler: &FlowHandler, syntax: &SyntaxNode) -> SyntaxNode {
diff --git a/crates/ide_assists/src/handlers/reorder_impl.rs b/crates/ide_assists/src/handlers/reorder_impl.rs
index 72d889248..54a9a468e 100644
--- a/crates/ide_assists/src/handlers/reorder_impl.rs
+++ b/crates/ide_assists/src/handlers/reorder_impl.rs
@@ -79,9 +79,12 @@ pub(crate) fn reorder_impl(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
         "Sort methods",
         target,
         |builder| {
-            methods.into_iter().zip(sorted).for_each(|(old, new)| {
-                ted::replace(builder.make_ast_mut(old).syntax(), new.clone_for_update().syntax())
-            });
+            let methods =
+                methods.into_iter().map(|fn_| builder.make_ast_mut(fn_)).collect::<Vec<_>>();
+            methods
+                .into_iter()
+                .zip(sorted)
+                .for_each(|(old, new)| ted::replace(old.syntax(), new.clone_for_update().syntax()));
         },
     )
 }
@@ -160,7 +163,7 @@ $0impl Bar for Foo {}
     }
 
     #[test]
-    fn reorder_impl_trait_methods() {
+    fn reorder_impl_trait_functions() {
         check_assist(
             reorder_impl,
             r#"
@@ -197,4 +200,33 @@ impl Bar for Foo {
         "#,
         )
     }
+
+    #[test]
+    fn reorder_impl_trait_methods_uneven_ident_lengths() {
+        check_assist(
+            reorder_impl,
+            r#"
+trait Bar {
+    fn foo(&mut self) {}
+    fn fooo(&mut self) {}
+}
+
+struct Foo;
+impl Bar for Foo {
+    fn fooo(&mut self) {}
+    fn foo(&mut self) {$0}
+}"#,
+            r#"
+trait Bar {
+    fn foo(&mut self) {}
+    fn fooo(&mut self) {}
+}
+
+struct Foo;
+impl Bar for Foo {
+    fn foo(&mut self) {}
+    fn fooo(&mut self) {}
+}"#,
+        )
+    }
 }
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index 969249df6..d8f23d1eb 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -737,28 +737,6 @@ fn f() {}
     }
 
     #[test]
-    fn completes_function() {
-        check(
-            r#"
-fn foo(
-    a: i32,
-    b: i32
-) {
-
-}
-
-fn main() {
-    fo$0
-}
-"#,
-            expect![[r#"
-                fn main() fn()
-                fn foo(…) fn(i32, i32)
-            "#]],
-        );
-    }
-
-    #[test]
     fn completes_self_enum() {
         check(
             r#"
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index b005bd773..f3fcb712c 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -301,103 +301,108 @@ impl<'a> CompletionContext<'a> {
             .find_map(ast::Impl::cast);
     }
 
+    fn expected_type_and_name(&self) -> (Option<Type>, Option<NameOrNameRef>) {
+        let mut node = match self.token.parent() {
+            Some(it) => it,
+            None => return (None, None),
+        };
+        loop {
+            break match_ast! {
+                match node {
+                    ast::LetStmt(it) => {
+                        cov_mark::hit!(expected_type_let_with_leading_char);
+                        cov_mark::hit!(expected_type_let_without_leading_char);
+                        let ty = it.pat()
+                            .and_then(|pat| self.sema.type_of_pat(&pat));
+                        let name = if let Some(ast::Pat::IdentPat(ident)) = it.pat() {
+                            ident.name().map(NameOrNameRef::Name)
+                        } else {
+                            None
+                        };
+
+                        (ty, name)
+                    },
+                    ast::ArgList(_it) => {
+                        cov_mark::hit!(expected_type_fn_param_with_leading_char);
+                        cov_mark::hit!(expected_type_fn_param_without_leading_char);
+                        ActiveParameter::at_token(
+                            &self.sema,
+                            self.token.clone(),
+                        ).map(|ap| {
+                            let name = ap.ident().map(NameOrNameRef::Name);
+                            (Some(ap.ty), name)
+                        })
+                        .unwrap_or((None, None))
+                    },
+                    ast::RecordExprFieldList(_it) => {
+                        cov_mark::hit!(expected_type_struct_field_without_leading_char);
+                        self.token.prev_sibling_or_token()
+                            .and_then(|se| se.into_node())
+                            .and_then(|node| ast::RecordExprField::cast(node))
+                            .and_then(|rf| self.sema.resolve_record_field(&rf).zip(Some(rf)))
+                            .map(|(f, rf)|(
+                                Some(f.0.ty(self.db)),
+                                rf.field_name().map(NameOrNameRef::NameRef),
+                            ))
+                            .unwrap_or((None, None))
+                    },
+                    ast::RecordExprField(it) => {
+                        cov_mark::hit!(expected_type_struct_field_with_leading_char);
+                        self.sema
+                            .resolve_record_field(&it)
+                            .map(|f|(
+                                Some(f.0.ty(self.db)),
+                                it.field_name().map(NameOrNameRef::NameRef),
+                            ))
+                            .unwrap_or((None, None))
+                    },
+                    ast::MatchExpr(it) => {
+                        cov_mark::hit!(expected_type_match_arm_without_leading_char);
+                        let ty = it.expr()
+                            .and_then(|e| self.sema.type_of_expr(&e));
+                        (ty, None)
+                    },
+                    ast::IfExpr(it) => {
+                        cov_mark::hit!(expected_type_if_let_without_leading_char);
+                        let ty = it.condition()
+                            .and_then(|cond| cond.expr())
+                            .and_then(|e| self.sema.type_of_expr(&e));
+                        (ty, None)
+                    },
+                    ast::IdentPat(it) => {
+                        cov_mark::hit!(expected_type_if_let_with_leading_char);
+                        cov_mark::hit!(expected_type_match_arm_with_leading_char);
+                        let ty = self.sema.type_of_pat(&ast::Pat::from(it));
+                        (ty, None)
+                    },
+                    ast::Fn(it) => {
+                        cov_mark::hit!(expected_type_fn_ret_with_leading_char);
+                        cov_mark::hit!(expected_type_fn_ret_without_leading_char);
+                        let def = self.sema.to_def(&it);
+                        (def.map(|def| def.ret_type(self.db)), None)
+                    },
+                    ast::Stmt(it) => (None, None),
+                    _ => {
+                        match node.parent() {
+                            Some(n) => {
+                                node = n;
+                                continue;
+                            },
+                            None => (None, None),
+                        }
+                    },
+                }
+            };
+        }
+    }
+
     fn fill(
         &mut self,
         original_file: &SyntaxNode,
         file_with_fake_ident: SyntaxNode,
         offset: TextSize,
     ) {
-        let (expected_type, expected_name) = {
-            let mut node = match self.token.parent() {
-                Some(it) => it,
-                None => return,
-            };
-            loop {
-                break match_ast! {
-                    match node {
-                        ast::LetStmt(it) => {
-                            cov_mark::hit!(expected_type_let_with_leading_char);
-                            cov_mark::hit!(expected_type_let_without_leading_char);
-                            let ty = it.pat()
-                                .and_then(|pat| self.sema.type_of_pat(&pat));
-                            let name = if let Some(ast::Pat::IdentPat(ident)) = it.pat() {
-                                ident.name().map(NameOrNameRef::Name)
-                            } else {
-                                None
-                            };
-
-                            (ty, name)
-                        },
-                        ast::ArgList(_it) => {
-                            cov_mark::hit!(expected_type_fn_param_with_leading_char);
-                            cov_mark::hit!(expected_type_fn_param_without_leading_char);
-                            ActiveParameter::at_token(
-                                &self.sema,
-                                self.token.clone(),
-                            ).map(|ap| {
-                                let name = ap.ident().map(NameOrNameRef::Name);
-                                (Some(ap.ty), name)
-                            })
-                            .unwrap_or((None, None))
-                        },
-                        ast::RecordExprFieldList(_it) => {
-                            cov_mark::hit!(expected_type_struct_field_without_leading_char);
-                            self.token.prev_sibling_or_token()
-                                .and_then(|se| se.into_node())
-                                .and_then(|node| ast::RecordExprField::cast(node))
-                                .and_then(|rf| self.sema.resolve_record_field(&rf).zip(Some(rf)))
-                                .map(|(f, rf)|(
-                                    Some(f.0.ty(self.db)),
-                                    rf.field_name().map(NameOrNameRef::NameRef),
-                                ))
-                                .unwrap_or((None, None))
-                        },
-                        ast::RecordExprField(it) => {
-                            cov_mark::hit!(expected_type_struct_field_with_leading_char);
-                            self.sema
-                                .resolve_record_field(&it)
-                                .map(|f|(
-                                    Some(f.0.ty(self.db)),
-                                    it.field_name().map(NameOrNameRef::NameRef),
-                                ))
-                                .unwrap_or((None, None))
-                        },
-                        ast::MatchExpr(it) => {
-                            cov_mark::hit!(expected_type_match_arm_without_leading_char);
-                            let ty = it.expr()
-                                .and_then(|e| self.sema.type_of_expr(&e));
-
-                            (ty, None)
-                        },
-                        ast::IdentPat(it) => {
-                            cov_mark::hit!(expected_type_if_let_with_leading_char);
-                            cov_mark::hit!(expected_type_match_arm_with_leading_char);
-                            let ty = self.sema.type_of_pat(&ast::Pat::from(it));
-
-                            (ty, None)
-                        },
-                        ast::Fn(_it) => {
-                            cov_mark::hit!(expected_type_fn_ret_with_leading_char);
-                            cov_mark::hit!(expected_type_fn_ret_without_leading_char);
-                            let ty = self.token.ancestors()
-                                .find_map(|ancestor| ast::Expr::cast(ancestor))
-                                .and_then(|expr| self.sema.type_of_expr(&expr));
-
-                            (ty, None)
-                        },
-                        _ => {
-                            match node.parent() {
-                                Some(n) => {
-                                    node = n;
-                                    continue;
-                                },
-                                None => (None, None),
-                            }
-                        },
-                    }
-                };
-            }
-        };
+        let (expected_type, expected_name) = self.expected_type_and_name();
         self.expected_type = expected_type;
         self.expected_name = expected_name;
         self.attribute_under_caret = find_node_at_offset(&file_with_fake_ident, offset);
@@ -802,6 +807,7 @@ fn foo() {
 
     #[test]
     fn expected_type_if_let_without_leading_char() {
+        cov_mark::check!(expected_type_if_let_without_leading_char);
         check_expected_type_and_name(
             r#"
 enum Foo { Bar, Baz, Quux }
@@ -811,8 +817,8 @@ fn foo() {
     if let $0 = f { }
 }
 "#,
-            expect![[r#"ty: (), name: ?"#]],
-        ) // FIXME should be `ty: u32, name: ?`
+            expect![[r#"ty: Foo, name: ?"#]],
+        )
     }
 
     #[test]
@@ -840,8 +846,8 @@ fn foo() -> u32 {
     $0
 }
 "#,
-            expect![[r#"ty: (), name: ?"#]],
-        ) // FIXME this should be `ty: u32, name: ?`
+            expect![[r#"ty: u32, name: ?"#]],
+        )
     }
 
     #[test]
@@ -856,4 +862,16 @@ fn foo() -> u32 {
             expect![[r#"ty: u32, name: ?"#]],
         )
     }
+
+    #[test]
+    fn expected_type_fn_ret_fn_ref_fully_typed() {
+        check_expected_type_and_name(
+            r#"
+fn foo() -> u32 {
+    foo$0
+}
+"#,
+            expect![[r#"ty: u32, name: ?"#]],
+        )
+    }
 }
diff --git a/crates/proc_macro_api/Cargo.toml b/crates/proc_macro_api/Cargo.toml
index 1ba1e4abd..2ce5eeedd 100644
--- a/crates/proc_macro_api/Cargo.toml
+++ b/crates/proc_macro_api/Cargo.toml
@@ -15,7 +15,7 @@ serde_json = { version = "1.0", features = ["unbounded_depth"] }
 log = "0.4.8"
 crossbeam-channel = "0.5.0"
 jod-thread = "0.1.1"
-memmap = "0.7.0"
+memmap2 = "0.2.0"
 object = { version = "0.23.0", default-features = false, features = ["std", "read_core", "elf", "macho", "pe", "unaligned"] }
 snap = "1.0"
 
diff --git a/crates/proc_macro_api/src/version.rs b/crates/proc_macro_api/src/version.rs
index dcf8fae8f..6dbac50b4 100644
--- a/crates/proc_macro_api/src/version.rs
+++ b/crates/proc_macro_api/src/version.rs
@@ -6,7 +6,7 @@ use std::{
     path::Path,
 };
 
-use memmap::Mmap;
+use memmap2::Mmap;
 use object::read::{File as BinaryFile, Object, ObjectSection};
 use snap::read::FrameDecoder as SnapDecoder;