Unnamed repository; edit this file 'description' to name the repository.
Diffstat (limited to 'crates/ide-assists/src/handlers/inline_local_variable.rs')
| -rw-r--r-- | crates/ide-assists/src/handlers/inline_local_variable.rs | 216 |
1 files changed, 148 insertions, 68 deletions
diff --git a/crates/ide-assists/src/handlers/inline_local_variable.rs b/crates/ide-assists/src/handlers/inline_local_variable.rs index 531adf62ba..7a82e8b2f3 100644 --- a/crates/ide-assists/src/handlers/inline_local_variable.rs +++ b/crates/ide-assists/src/handlers/inline_local_variable.rs @@ -1,19 +1,20 @@ use either::{Either, for_both}; -use hir::{PathResolution, Semantics}; +use hir::{EditionedFileId, PathResolution, Semantics}; use ide_db::{ - EditionedFileId, RootDatabase, + RootDatabase, defs::Definition, - search::{FileReference, FileReferenceNode, UsageSearchResult}, + search::{FileReference, UsageSearchResult}, }; use syntax::{ - Direction, TextRange, + Direction, T, TextRange, ast::{self, AstNode, AstToken, HasName}, - syntax_editor::{Element, SyntaxEditor}, + syntax_editor::{Element, Position, SyntaxEditor}, }; use crate::{ AssistId, assist_context::{AssistContext, Assists}, + utils::{cover_edit_range, original_range_in}, }; // Assist: inline_local_variable @@ -34,11 +35,12 @@ use crate::{ // ``` pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_, '_>) -> Option<()> { let file_id = ctx.file_id(); + let source = ctx.source_file().syntax(); let range = ctx.selection_trimmed(); let InlineData { let_stmt, delete_let, references, target } = - if let Some(path_expr) = ctx.find_node_at_offset::<ast::PathExpr>() { + if let Some(path_expr) = ctx.find_node_at_offset_with_descend::<ast::PathExpr>() { inline_usage(&ctx.sema, path_expr, range, file_id) - } else if let Some(let_stmt) = ctx.find_node_at_offset() { + } else if let Some(let_stmt) = ctx.find_node_at_offset_with_descend() { inline_let(&ctx.sema, let_stmt, range, file_id) } else { None @@ -47,71 +49,65 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_, ' either::Either::Left(it) => it.initializer()?, either::Either::Right(it) => it.expr()?, }; - - let wrap_in_parens = references - .into_iter() - .filter_map(|FileReference { range, name, .. }| match name { - FileReferenceNode::NameRef(name) => Some((range, name)), - _ => None, - }) - .map(|(range, name_ref)| { - if range != name_ref.syntax().text_range() { - // Do not rename inside macros - // FIXME: This feels like a bad heuristic for macros - return None; + let needs_parens = |name_ref: &ast::NameRef| { + let usage_node = + name_ref.syntax().ancestors().find(|it| ast::PathExpr::can_cast(it.kind())); + let usage_parent = usage_node.as_ref().and_then(|it| it.parent()); + match (usage_node, usage_parent) { + (Some(usage), Some(parent)) => { + initializer_expr.needs_parens_in_place_of(&parent, &usage) } - let usage_node = - name_ref.syntax().ancestors().find(|it| ast::PathExpr::can_cast(it.kind())); - let usage_parent_option = usage_node.as_ref().and_then(|it| it.parent()); - let usage_parent = match usage_parent_option { - Some(u) => u, - None => return Some((name_ref, false)), - }; - let should_wrap = initializer_expr - .needs_parens_in_place_of(&usage_parent, usage_node.as_ref().unwrap()); - Some((name_ref, should_wrap)) - }) - .collect::<Option<Vec<_>>>()?; - - let target = match target { - ast::NameOrNameRef::Name(it) => it.syntax().clone(), - ast::NameOrNameRef::NameRef(it) => it.syntax().clone(), + _ => false, + } }; acc.add( AssistId::refactor_inline("inline_local_variable"), "Inline variable", - target.text_range(), - move |builder| { - let editor = builder.make_editor(&target); + target, + |builder| { + let editor = builder.make_editor(source); let make = editor.make(); - if delete_let { - editor.delete(let_stmt.syntax()); - + if delete_let + && let Some(original) = original_range_in(file_id, &ctx.sema, let_stmt.syntax()) + { + let place = cover_edit_range(source, original); + editor.delete_all(place.clone()); + + // Processing let-expr in let-chain + // FIXME: process let-expr in macro, but this case is very rare if let Some(bin_expr) = let_stmt.syntax().parent().and_then(ast::BinExpr::cast) && let Some(op_token) = bin_expr.op_token() { editor.delete(&op_token); remove_whitespace(op_token, Direction::Prev, &editor); - remove_whitespace(let_stmt.syntax(), Direction::Prev, &editor); + remove_whitespace(place.start(), Direction::Prev, &editor); } else { - remove_whitespace(let_stmt.syntax(), Direction::Next, &editor); + remove_whitespace(place.end(), Direction::Next, &editor); } } - for (name, should_wrap) in wrap_in_parens { - let replacement = if should_wrap { + for FileReference { range, name, .. } in references { + let Some(name) = name.as_name_ref().cloned() else { continue }; + let replacement = if needs_parens(&name) { make.expr_paren(initializer_expr.clone()).into() } else { initializer_expr.clone() }; - if let Some(record_field) = ast::RecordExprField::for_field_name(&name) { + let place = cover_edit_range(source, range); + if ast::RecordExprField::for_field_name(&name).is_some() { cov_mark::hit!(inline_field_shorthand); - let replacement = make.record_expr_field(name, Some(replacement)); - editor.replace(record_field.syntax(), replacement.syntax()); + editor.insert_all( + Position::after(place.end()), + vec![ + make.token(T![:]).into(), + make.whitespace(" ").into(), + replacement.syntax().clone().into(), + ], + ); } else { - editor.replace(name.syntax(), replacement.syntax()); + editor.replace_all(place, vec![replacement.syntax().clone().into()]); } } builder.add_file_edits(ctx.vfs_file_id(), editor); @@ -122,7 +118,7 @@ pub(crate) fn inline_local_variable(acc: &mut Assists, ctx: &AssistContext<'_, ' struct InlineData { let_stmt: Either<ast::LetStmt, ast::LetExpr>, delete_let: bool, - target: ast::NameOrNameRef, + target: TextRange, references: Vec<FileReference>, } @@ -140,20 +136,18 @@ fn inline_let( cov_mark::hit!(test_not_inline_mut_variable); return None; } - if !bind_pat.syntax().text_range().contains_range(range) { + let target = original_range_in(file_id, sema, bind_pat.name()?.syntax())?; + if !target.contains_range(range) { cov_mark::hit!(not_applicable_outside_of_bind_pat); return None; } 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, - target: ast::NameOrNameRef::Name(bind_pat.name()?), - references, - }), + let UsageSearchResult { references } = Definition::Local(local).usages(sema).all(); + let references = references.into_iter().flat_map(|it| it.1).collect::<Vec<_>>(); + + match references.first() { + Some(_) => Some(InlineData { let_stmt, delete_let: true, target, references }), None => { cov_mark::hit!(test_not_applicable_if_variable_unused); None @@ -169,7 +163,8 @@ fn inline_usage( ) -> Option<InlineData> { let path = path_expr.path()?; let name = path.as_single_name_ref()?; - if !name.syntax().text_range().contains_range(range) { + let target = original_range_in(file_id, sema, name.syntax())?; + if !target.contains_range(range) { cov_mark::hit!(test_not_inline_selection_too_broad); return None; } @@ -193,12 +188,12 @@ fn inline_usage( let let_stmt = AstNode::cast(bind_pat.syntax().parent()?)?; - let UsageSearchResult { mut references } = Definition::Local(local).usages(sema).all(); - let mut references = references.remove(&file_id)?; + let UsageSearchResult { references } = Definition::Local(local).usages(sema).all(); + let mut references = references.into_iter().flat_map(|it| it.1).collect::<Vec<_>>(); let delete_let = references.len() == 1; references.retain(|fref| fref.name.as_name_ref() == Some(&name)); - Some(InlineData { let_stmt, delete_let, target: ast::NameOrNameRef::NameRef(name), references }) + Some(InlineData { let_stmt, delete_let, target, references }) } fn remove_whitespace(elem: impl Element, dir: Direction, editor: &SyntaxEditor) { @@ -969,8 +964,8 @@ fn main() { } #[test] - fn not_applicable_on_local_usage_in_macro() { - check_assist_not_applicable( + fn local_usage_in_macro() { + check_assist( inline_local_variable, r#" macro_rules! m { @@ -978,11 +973,19 @@ macro_rules! m { } fn f() { let xyz = 0; - m!(xyz$0); // replacing it would break the macro + m!(xyz$0); // some macros may break, but it's best to support them +} +"#, + r#" +macro_rules! m { + ($i:ident) => { $i } +} +fn f() { + m!(0); // some macros may break, but it's best to support them } "#, ); - check_assist_not_applicable( + check_assist( inline_local_variable, r#" macro_rules! m { @@ -990,7 +993,84 @@ macro_rules! m { } fn f() { let xyz$0 = 0; - m!(xyz); // replacing it would break the macro + m!(xyz); // some macros may break, but it's best to support them +} +"#, + r#" +macro_rules! m { + ($i:ident) => { $i } +} +fn f() { + m!(0); // some macros may break, but it's best to support them +} +"#, + ); + } + + #[test] + fn local_def_in_macro() { + check_assist( + inline_local_variable, + r#" +macro_rules! i { + ($($t:tt)*) => { $($t)* } +} +fn f() { + i!(let xyz = 0;); + _ = xyz$0; +} +"#, + r#" +macro_rules! i { + ($($t:tt)*) => { $($t)* } +} +fn f() { + i!(); + _ = 0; +} +"#, + ); + check_assist( + inline_local_variable, + r#" +macro_rules! i { + ($($t:tt)*) => { $($t)* } +} +fn f() { + i!(let xyz = 0;); + _ = xyz$0; + _ = xyz; +} +"#, + r#" +macro_rules! i { + ($($t:tt)*) => { $($t)* } +} +fn f() { + i!(let xyz = 0;); + _ = 0; + _ = xyz; +} +"#, + ); + check_assist( + inline_local_variable, + r#" +macro_rules! i { + ($($t:tt)*) => { $($t)* } +} +fn f() { + i!(let $0xyz = 0;); + _ = xyz; +} +"#, + r#" +macro_rules! i { + ($($t:tt)*) => { $($t)* } +} +fn f() { + i!(); + _ = 0; } "#, ); |