6 files changed, 261 insertions, 36 deletions

diff --git a/crates/ra_assists/src/handlers/add_explicit_type.rs b/crates/ra_assists/src/handlers/add_explicit_type.rs
index d86d804b2..6c56d93d8 100644
--- a/crates/ra_assists/src/handlers/add_explicit_type.rs
+++ b/crates/ra_assists/src/handlers/add_explicit_type.rs
@@ -52,21 +52,22 @@ pub(crate) fn add_explicit_type(ctx: AssistCtx) -> Option<Assist> {
     }
     // Infer type
     let ty = ctx.sema.type_of_expr(&expr)?;
-    // Assist not applicable if the type is unknown
-    if ty.contains_unknown() {
+
+    if ty.contains_unknown() || ty.is_closure() {
         return None;
     }
 
     let db = ctx.db;
+    let new_type_string = ty.display_truncated(db, None).to_string();
     ctx.add_assist(
         AssistId("add_explicit_type"),
-        format!("Insert explicit type '{}'", ty.display(db)),
+        format!("Insert explicit type '{}'", new_type_string),
         |edit| {
             edit.target(pat_range);
             if let Some(ascribed_ty) = ascribed_ty {
-                edit.replace(ascribed_ty.syntax().text_range(), format!("{}", ty.display(db)));
+                edit.replace(ascribed_ty.syntax().text_range(), new_type_string);
             } else {
-                edit.insert(name_range.end(), format!(": {}", ty.display(db)));
+                edit.insert(name_range.end(), format!(": {}", new_type_string));
             }
         },
     )
@@ -174,4 +175,41 @@ mod tests {
             "fn f() <|>{let a = match 1 {2 => 3, 3 => 5};}",
         )
     }
+
+    #[test]
+    fn closure_parameters_are_not_added() {
+        check_assist_not_applicable(
+            add_explicit_type,
+            r#"
+fn main() {
+    let multiply_by_two<|> = |i| i * 3;
+    let six = multiply_by_two(2);
+}"#,
+        )
+    }
+
+    #[test]
+    fn default_generics_should_not_be_added() {
+        check_assist(
+            add_explicit_type,
+            r#"
+struct Test<K, T = u8> {
+    k: K,
+    t: T,
+}
+
+fn main() {
+    let test<|> = Test { t: 23, k: 33 };
+}"#,
+            r#"
+struct Test<K, T = u8> {
+    k: K,
+    t: T,
+}
+
+fn main() {
+    let test<|>: Test<i32> = Test { t: 23, k: 33 };
+}"#,
+        );
+    }
 }
diff --git a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
index 864373aa5..0621487e8 100644
--- a/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
+++ b/crates/ra_assists/src/handlers/add_from_impl_for_enum.rs
@@ -98,7 +98,7 @@ fn already_has_from_impl(
     };
     let var_ty = hir_enum_var.fields(sema.db)[0].signature_ty(sema.db);
 
-    e_ty.impls_trait(sema.db, from_trait, &[var_ty.clone()])
+    e_ty.impls_trait(sema.db, from_trait, &[var_ty])
 }
 
 #[cfg(test)]
diff --git a/crates/ra_assists/src/handlers/add_function.rs b/crates/ra_assists/src/handlers/add_function.rs
index ad4ab66ed..f185cffdb 100644
--- a/crates/ra_assists/src/handlers/add_function.rs
+++ b/crates/ra_assists/src/handlers/add_function.rs
@@ -3,8 +3,8 @@ use ra_syntax::{
     SyntaxKind, SyntaxNode, TextUnit,
 };
 
-use crate::{Assist, AssistCtx, AssistId};
-use ast::{edit::IndentLevel, ArgListOwner, CallExpr, Expr};
+use crate::{Assist, AssistCtx, AssistFile, AssistId};
+use ast::{edit::IndentLevel, ArgListOwner, ModuleItemOwner};
 use hir::HirDisplay;
 use rustc_hash::{FxHashMap, FxHashSet};
 
@@ -16,7 +16,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
 // struct Baz;
 // fn baz() -> Baz { Baz }
 // fn foo() {
-//      bar<|>("", baz());
+//     bar<|>("", baz());
 // }
 //
 // ```
@@ -25,7 +25,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
 // struct Baz;
 // fn baz() -> Baz { Baz }
 // fn foo() {
-//      bar("", baz());
+//     bar("", baz());
 // }
 //
 // fn bar(arg: &str, baz: Baz) {
@@ -38,21 +38,30 @@ pub(crate) fn add_function(ctx: AssistCtx) -> Option<Assist> {
     let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?;
     let path = path_expr.path()?;
 
-    if path.qualifier().is_some() {
-        return None;
-    }
-
     if ctx.sema.resolve_path(&path).is_some() {
         // The function call already resolves, no need to add a function
         return None;
     }
 
-    let function_builder = FunctionBuilder::from_call(&ctx, &call)?;
+    let target_module = if let Some(qualifier) = path.qualifier() {
+        if let Some(hir::PathResolution::Def(hir::ModuleDef::Module(module))) =
+            ctx.sema.resolve_path(&qualifier)
+        {
+            Some(module.definition_source(ctx.sema.db))
+        } else {
+            return None;
+        }
+    } else {
+        None
+    };
+
+    let function_builder = FunctionBuilder::from_call(&ctx, &call, &path, target_module)?;
 
     ctx.add_assist(AssistId("add_function"), "Add function", |edit| {
         edit.target(call.syntax().text_range());
 
         if let Some(function_template) = function_builder.render() {
+            edit.set_file(function_template.file);
             edit.set_cursor(function_template.cursor_offset);
             edit.insert(function_template.insert_offset, function_template.fn_def.to_string());
         }
@@ -63,29 +72,67 @@ struct FunctionTemplate {
     insert_offset: TextUnit,
     cursor_offset: TextUnit,
     fn_def: ast::SourceFile,
+    file: AssistFile,
 }
 
 struct FunctionBuilder {
-    append_fn_at: SyntaxNode,
+    target: GeneratedFunctionTarget,
     fn_name: ast::Name,
     type_params: Option<ast::TypeParamList>,
     params: ast::ParamList,
+    file: AssistFile,
+    needs_pub: bool,
 }
 
 impl FunctionBuilder {
-    fn from_call(ctx: &AssistCtx, call: &ast::CallExpr) -> Option<Self> {
-        let append_fn_at = next_space_for_fn(&call)?;
-        let fn_name = fn_name(&call)?;
+    /// Prepares a generated function that matches `call` in `generate_in`
+    /// (or as close to `call` as possible, if `generate_in` is `None`)
+    fn from_call(
+        ctx: &AssistCtx,
+        call: &ast::CallExpr,
+        path: &ast::Path,
+        target_module: Option<hir::InFile<hir::ModuleSource>>,
+    ) -> Option<Self> {
+        let needs_pub = target_module.is_some();
+        let mut file = AssistFile::default();
+        let target = if let Some(target_module) = target_module {
+            let (in_file, target) = next_space_for_fn_in_module(ctx.sema.db, target_module)?;
+            file = in_file;
+            target
+        } else {
+            next_space_for_fn_after_call_site(&call)?
+        };
+        let fn_name = fn_name(&path)?;
         let (type_params, params) = fn_args(ctx, &call)?;
-        Some(Self { append_fn_at, fn_name, type_params, params })
+        Some(Self { target, fn_name, type_params, params, file, needs_pub })
     }
+
     fn render(self) -> Option<FunctionTemplate> {
         let placeholder_expr = ast::make::expr_todo();
         let fn_body = ast::make::block_expr(vec![], Some(placeholder_expr));
-        let fn_def = ast::make::fn_def(self.fn_name, self.type_params, self.params, fn_body);
-        let fn_def = ast::make::add_newlines(2, fn_def);
-        let fn_def = IndentLevel::from_node(&self.append_fn_at).increase_indent(fn_def);
-        let insert_offset = self.append_fn_at.text_range().end();
+        let mut fn_def = ast::make::fn_def(self.fn_name, self.type_params, self.params, fn_body);
+        if self.needs_pub {
+            fn_def = ast::make::add_pub_crate_modifier(fn_def);
+        }
+
+        let (fn_def, insert_offset) = match self.target {
+            GeneratedFunctionTarget::BehindItem(it) => {
+                let with_leading_blank_line = ast::make::add_leading_newlines(2, fn_def);
+                let indented = IndentLevel::from_node(&it).increase_indent(with_leading_blank_line);
+                (indented, it.text_range().end())
+            }
+            GeneratedFunctionTarget::InEmptyItemList(it) => {
+                let indent_once = IndentLevel(1);
+                let indent = IndentLevel::from_node(it.syntax());
+
+                let fn_def = ast::make::add_leading_newlines(1, fn_def);
+                let fn_def = indent_once.increase_indent(fn_def);
+                let fn_def = ast::make::add_trailing_newlines(1, fn_def);
+                let fn_def = indent.increase_indent(fn_def);
+                (fn_def, it.syntax().text_range().start() + TextUnit::from_usize(1))
+            }
+        };
+
         let cursor_offset_from_fn_start = fn_def
             .syntax()
             .descendants()
@@ -94,19 +141,24 @@ impl FunctionBuilder {
             .text_range()
             .start();
         let cursor_offset = insert_offset + cursor_offset_from_fn_start;
-        Some(FunctionTemplate { insert_offset, cursor_offset, fn_def })
+        Some(FunctionTemplate { insert_offset, cursor_offset, fn_def, file: self.file })
     }
 }
 
-fn fn_name(call: &CallExpr) -> Option<ast::Name> {
-    let name = call.expr()?.syntax().to_string();
+enum GeneratedFunctionTarget {
+    BehindItem(SyntaxNode),
+    InEmptyItemList(ast::ItemList),
+}
+
+fn fn_name(call: &ast::Path) -> Option<ast::Name> {
+    let name = call.segment()?.syntax().to_string();
     Some(ast::make::name(&name))
 }
 
 /// Computes the type variables and arguments required for the generated function
 fn fn_args(
     ctx: &AssistCtx,
-    call: &CallExpr,
+    call: &ast::CallExpr,
 ) -> Option<(Option<ast::TypeParamList>, ast::ParamList)> {
     let mut arg_names = Vec::new();
     let mut arg_types = Vec::new();
@@ -158,9 +210,9 @@ fn deduplicate_arg_names(arg_names: &mut Vec<String>) {
     }
 }
 
-fn fn_arg_name(fn_arg: &Expr) -> Option<String> {
+fn fn_arg_name(fn_arg: &ast::Expr) -> Option<String> {
     match fn_arg {
-        Expr::CastExpr(cast_expr) => fn_arg_name(&cast_expr.expr()?),
+        ast::Expr::CastExpr(cast_expr) => fn_arg_name(&cast_expr.expr()?),
         _ => Some(
             fn_arg
                 .syntax()
@@ -172,7 +224,7 @@ fn fn_arg_name(fn_arg: &Expr) -> Option<String> {
     }
 }
 
-fn fn_arg_type(ctx: &AssistCtx, fn_arg: &Expr) -> Option<String> {
+fn fn_arg_type(ctx: &AssistCtx, fn_arg: &ast::Expr) -> Option<String> {
     let ty = ctx.sema.type_of_expr(fn_arg)?;
     if ty.is_unknown() {
         return None;
@@ -184,7 +236,7 @@ fn fn_arg_type(ctx: &AssistCtx, fn_arg: &Expr) -> Option<String> {
 /// directly after the current block
 /// We want to write the generated function directly after
 /// fns, impls or macro calls, but inside mods
-fn next_space_for_fn(expr: &CallExpr) -> Option<SyntaxNode> {
+fn next_space_for_fn_after_call_site(expr: &ast::CallExpr) -> Option<GeneratedFunctionTarget> {
     let mut ancestors = expr.syntax().ancestors().peekable();
     let mut last_ancestor: Option<SyntaxNode> = None;
     while let Some(next_ancestor) = ancestors.next() {
@@ -201,7 +253,32 @@ fn next_space_for_fn(expr: &CallExpr) -> Option<SyntaxNode> {
         }
         last_ancestor = Some(next_ancestor);
     }
-    last_ancestor
+    last_ancestor.map(GeneratedFunctionTarget::BehindItem)
+}
+
+fn next_space_for_fn_in_module(
+    db: &dyn hir::db::AstDatabase,
+    module: hir::InFile<hir::ModuleSource>,
+) -> Option<(AssistFile, GeneratedFunctionTarget)> {
+    let file = module.file_id.original_file(db);
+    let assist_file = AssistFile::TargetFile(file);
+    let assist_item = match module.value {
+        hir::ModuleSource::SourceFile(it) => {
+            if let Some(last_item) = it.items().last() {
+                GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())
+            } else {
+                GeneratedFunctionTarget::BehindItem(it.syntax().clone())
+            }
+        }
+        hir::ModuleSource::Module(it) => {
+            if let Some(last_item) = it.item_list().and_then(|it| it.items().last()) {
+                GeneratedFunctionTarget::BehindItem(last_item.syntax().clone())
+            } else {
+                GeneratedFunctionTarget::InEmptyItemList(it.item_list()?)
+            }
+        }
+    };
+    Some((assist_file, assist_item))
 }
 
 #[cfg(test)]
@@ -714,6 +791,111 @@ fn bar(baz_1: Baz, baz_2: Baz, arg_1: &str, arg_2: &str) {
     }
 
     #[test]
+    fn add_function_in_module() {
+        check_assist(
+            add_function,
+            r"
+mod bar {}
+
+fn foo() {
+    bar::my_fn<|>()
+}
+",
+            r"
+mod bar {
+    pub(crate) fn my_fn() {
+        <|>todo!()
+    }
+}
+
+fn foo() {
+    bar::my_fn()
+}
+",
+        )
+    }
+
+    #[test]
+    fn add_function_in_module_containing_other_items() {
+        check_assist(
+            add_function,
+            r"
+mod bar {
+    fn something_else() {}
+}
+
+fn foo() {
+    bar::my_fn<|>()
+}
+",
+            r"
+mod bar {
+    fn something_else() {}
+
+    pub(crate) fn my_fn() {
+        <|>todo!()
+    }
+}
+
+fn foo() {
+    bar::my_fn()
+}
+",
+        )
+    }
+
+    #[test]
+    fn add_function_in_nested_module() {
+        check_assist(
+            add_function,
+            r"
+mod bar {
+    mod baz {}
+}
+
+fn foo() {
+    bar::baz::my_fn<|>()
+}
+",
+            r"
+mod bar {
+    mod baz {
+        pub(crate) fn my_fn() {
+            <|>todo!()
+        }
+    }
+}
+
+fn foo() {
+    bar::baz::my_fn()
+}
+",
+        )
+    }
+
+    #[test]
+    fn add_function_in_another_file() {
+        check_assist(
+            add_function,
+            r"
+//- /main.rs
+mod foo;
+
+fn main() {
+    foo::bar<|>()
+}
+//- /foo.rs
+",
+            r"
+
+
+pub(crate) fn bar() {
+    <|>todo!()
+}",
+        )
+    }
+
+    #[test]
     fn add_function_not_applicable_if_function_already_exists() {
         check_assist_not_applicable(
             add_function,
diff --git a/crates/ra_assists/src/handlers/introduce_variable.rs b/crates/ra_assists/src/handlers/introduce_variable.rs
index 8d0f7e922..8c09e6bcd 100644
--- a/crates/ra_assists/src/handlers/introduce_variable.rs
+++ b/crates/ra_assists/src/handlers/introduce_variable.rs
@@ -124,7 +124,7 @@ fn anchor_stmt(expr: ast::Expr) -> Option<(SyntaxNode, bool)> {
             }
         }
 
-        if ast::Stmt::cast(node.clone().into()).is_some() {
+        if ast::Stmt::cast(node.clone()).is_some() {
             return Some((node, false));
         }
 
diff --git a/crates/ra_assists/src/handlers/merge_imports.rs b/crates/ra_assists/src/handlers/merge_imports.rs
index ef0ce0586..4be1238f1 100644
--- a/crates/ra_assists/src/handlers/merge_imports.rs
+++ b/crates/ra_assists/src/handlers/merge_imports.rs
@@ -30,7 +30,7 @@ pub(crate) fn merge_imports(ctx: AssistCtx) -> Option<Assist> {
             .filter_map(|dir| neighbor(&use_item, dir))
             .filter_map(|it| Some((it.clone(), it.use_tree()?)))
             .find_map(|(use_item, use_tree)| {
-                Some((try_merge_trees(&tree, &use_tree)?, use_item.clone()))
+                Some((try_merge_trees(&tree, &use_tree)?, use_item))
             })?;
 
         rewriter.replace_ast(&tree, &merged);
diff --git a/crates/ra_assists/src/handlers/split_import.rs b/crates/ra_assists/src/handlers/split_import.rs
index d9244f22d..f25826796 100644
--- a/crates/ra_assists/src/handlers/split_import.rs
+++ b/crates/ra_assists/src/handlers/split_import.rs
@@ -37,7 +37,7 @@ pub(crate) fn split_import(ctx: AssistCtx) -> Option<Assist> {
 
 #[cfg(test)]
 mod tests {
-    use crate::helpers::{check_assist, check_assist_target};
+    use crate::helpers::{check_assist, check_assist_not_applicable, check_assist_target};
 
     use super::*;
 
@@ -63,4 +63,9 @@ mod tests {
     fn split_import_target() {
         check_assist_target(split_import, "use crate::<|>db::{RootDatabase, FileSymbol}", "::");
     }
+
+    #[test]
+    fn issue4044() {
+        check_assist_not_applicable(split_import, "use crate::<|>:::self;")
+    }
 }