diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs index 757c7543396..692c3f7f071 100644 --- a/crates/ide_assists/src/handlers/extract_function.rs +++ b/crates/ide_assists/src/handlers/extract_function.rs @@ -1080,8 +1080,7 @@ fn node_to_insert_after(body: &FunctionBody, anchor: Anchor) -> Option String { let ret_ty = fun.return_type(ctx); - let args = fun.params.iter().map(|param| param.to_arg(ctx)); - let args = make::arg_list(args); + let args = make::arg_list(fun.params.iter().map(|param| param.to_arg(ctx))); let name = fun.name.clone(); let call_expr = if fun.self_param.is_some() { let self_arg = make::expr_path(make::ext::ident_path("self")); @@ -1103,12 +1102,12 @@ fn make_call(ctx: &AssistContext, fun: &Function, indent: IndentLevel) -> String [var] => { format_to!(buf, "let {}{} = ", mut_modifier(var), var.local.name(ctx.db()).unwrap()) } - [v0, vs @ ..] => { + vars => { buf.push_str("let ("); - format_to!(buf, "{}{}", mut_modifier(v0), v0.local.name(ctx.db()).unwrap()); - for var in vs { - format_to!(buf, ", {}{}", mut_modifier(var), var.local.name(ctx.db()).unwrap()); - } + let bindings = vars.iter().format_with(", ", |local, f| { + f(&format_args!("{}{}", mut_modifier(local), local.local.name(ctx.db()).unwrap())) + }); + format_to!(buf, "{}", bindings); buf.push_str(") = "); } } diff --git a/crates/ide_assists/src/handlers/inline_local_variable.rs b/crates/ide_assists/src/handlers/inline_local_variable.rs index dec8c320fdc..19e09d80686 100644 --- a/crates/ide_assists/src/handlers/inline_local_variable.rs +++ b/crates/ide_assists/src/handlers/inline_local_variable.rs @@ -1,8 +1,10 @@ use either::Either; -use hir::PathResolution; +use hir::{PathResolution, Semantics}; use ide_db::{ + base_db::{FileId, FileRange}, defs::Definition, search::{FileReference, UsageSearchResult}, + RootDatabase, }; use syntax::{ ast::{self, AstNode, AstToken, NameOwner}, @@ -31,8 +33,15 @@ // } // ``` pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + let FileRange { file_id, range } = ctx.frange; let InlineData { let_stmt, delete_let, references, target } = - inline_let(ctx).or_else(|| inline_usage(ctx))?; + if let Some(let_stmt) = ctx.find_node_at_offset::() { + inline_let(&ctx.sema, let_stmt, range, file_id) + } else if let Some(path_expr) = ctx.find_node_at_offset::() { + inline_usage(&ctx.sema, path_expr, range, file_id) + } else { + None + }?; let initializer_expr = let_stmt.initializer()?; let delete_range = delete_let.then(|| { @@ -139,8 +148,12 @@ struct InlineData { references: Vec, } -fn inline_let(ctx: &AssistContext) -> Option { - let let_stmt = ctx.find_node_at_offset::()?; +fn inline_let( + sema: &Semantics, + let_stmt: ast::LetStmt, + range: TextRange, + file_id: FileId, +) -> Option { let bind_pat = match let_stmt.pat()? { ast::Pat::IdentPat(pat) => pat, _ => return None, @@ -149,14 +162,14 @@ fn inline_let(ctx: &AssistContext) -> Option { cov_mark::hit!(test_not_inline_mut_variable); return None; } - if !bind_pat.syntax().text_range().contains_inclusive(ctx.offset()) { + if !bind_pat.syntax().text_range().contains_range(range) { cov_mark::hit!(not_applicable_outside_of_bind_pat); return None; } - let local = ctx.sema.to_def(&bind_pat)?; - let UsageSearchResult { mut references } = Definition::Local(local).usages(&ctx.sema).all(); - match references.remove(&ctx.frange.file_id) { + let local = sema.to_def(&bind_pat)?; + let UsageSearchResult { mut references } = Definition::Local(local).usages(sema).all(); + match references.remove(&file_id) { Some(references) => Some(InlineData { let_stmt, delete_let: true, @@ -170,29 +183,37 @@ fn inline_let(ctx: &AssistContext) -> Option { } } -fn inline_usage(ctx: &AssistContext) -> Option { - let path_expr = ctx.find_node_at_offset::()?; +fn inline_usage( + sema: &Semantics, + path_expr: ast::PathExpr, + range: TextRange, + file_id: FileId, +) -> Option { let path = path_expr.path()?; let name = path.as_single_name_ref()?; + if !name.syntax().text_range().contains_range(range) { + cov_mark::hit!(test_not_inline_selection_too_broad); + return None; + } - let local = match ctx.sema.resolve_path(&path)? { + let local = match sema.resolve_path(&path)? { PathResolution::Local(local) => local, _ => return None, }; - if local.is_mut(ctx.db()) { + if local.is_mut(sema.db) { cov_mark::hit!(test_not_inline_mut_variable_use); return None; } - let bind_pat = match local.source(ctx.db()).value { + let bind_pat = match local.source(sema.db).value { Either::Left(ident) => ident, _ => return None, }; let let_stmt = ast::LetStmt::cast(bind_pat.syntax().parent()?)?; - let UsageSearchResult { mut references } = Definition::Local(local).usages(&ctx.sema).all(); - let mut references = references.remove(&ctx.frange.file_id)?; + let UsageSearchResult { mut references } = Definition::Local(local).usages(sema).all(); + let mut references = references.remove(&file_id)?; let delete_let = references.len() == 1; references.retain(|fref| fref.name.as_name_ref() == Some(&name)); @@ -875,6 +896,21 @@ fn f() { let xyz$0 = 0; m!(xyz); // replacing it would break the macro } +"#, + ); + } + + #[test] + fn test_not_inline_selection_too_broad() { + cov_mark::check!(test_not_inline_selection_too_broad); + check_assist_not_applicable( + inline_local_variable, + r#" +fn f() { + let foo = 0; + let bar = 0; + $0foo + bar$0; +} "#, ); }