Unnamed repository; edit this file 'description' to name the repository.
Fix extract variable preserving whitespace from macro input
Signed-off-by: cuishuang <[email protected]>
| -rw-r--r-- | crates/ide-assists/src/handlers/extract_variable.rs | 79 |
1 files changed, 44 insertions, 35 deletions
diff --git a/crates/ide-assists/src/handlers/extract_variable.rs b/crates/ide-assists/src/handlers/extract_variable.rs index da8c5bc48d..1c03751e62 100644 --- a/crates/ide-assists/src/handlers/extract_variable.rs +++ b/crates/ide-assists/src/handlers/extract_variable.rs @@ -1,10 +1,12 @@ -use hir::{HirDisplay, Module, TypeInfo}; +use std::ops::RangeInclusive; + +use hir::{HirDisplay, TypeInfo}; use ide_db::{ assists::GroupLabel, syntax_helpers::{LexedStr, suggest_name}, }; use syntax::{ - Direction, NodeOrToken, SyntaxKind, SyntaxNode, SyntaxToken, T, TextRange, + Direction, NodeOrToken, SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, T, TextRange, algo::{ancestors_at_offset, skip_trivia_token}, ast::{ self, AstNode, @@ -93,7 +95,7 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_, '_>) - let node = node.ancestors().take_while(|anc| anc.text_range() == node.text_range()).last()?; let range = node.text_range(); - let (to_replace, analysis, original_expr) = if node.kind() == SyntaxKind::TOKEN_TREE { + let (to_replace, analysis, prefer_source_expr) = if node.kind() == SyntaxKind::TOKEN_TREE { let (first, last) = extract_token_range_of(&node, ctx.selection_trimmed())?; let first_descend = ctx.sema.descend_into_macros_single_exact(first.clone()); @@ -112,15 +114,14 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_, '_>) - if !node.text_range().contains_range(original_range.range) { return None; } - let original_expr = original_expr(ctx, original_range.file_id, original_range.range); - (cover_edit_range(&node, original_range.range), expr, original_expr) + (cover_edit_range(&node, original_range.range), expr, true) } else { let expr = node .descendants() .take_while(|it| range.contains_range(it.text_range())) .find_map(valid_target_expr(ctx))?; let to_extract = expr.syntax().syntax_element(); - (to_extract.clone()..=to_extract, expr, None) + (to_extract.clone()..=to_extract, expr, false) }; let place = match to_replace.start() { NodeOrToken::Node(node) => node.clone(), @@ -219,10 +220,12 @@ pub(crate) fn extract_variable(acc: &mut Assists, ctx: &AssistContext<'_, '_>) - editor.add_annotation(pat_name.syntax().clone(), tabstop); } - let to_extract_no_ref = original_expr - .clone() - .map(peel_parens) - .unwrap_or_else(|| prettify_macro_expr(ctx, module, to_extract_no_ref.clone())); + let to_extract_no_ref = if prefer_source_expr { + source_expr(ctx, to_replace.clone()) + .unwrap_or_else(|| to_extract_no_ref.clone()) + } else { + to_extract_no_ref.clone() + }; let initializer = match ty.as_ref().filter(|_| needs_ref) { Some(receiver_type) if receiver_type.is_mutable_reference() => { make.expr_ref(to_extract_no_ref.clone(), true) @@ -337,36 +340,15 @@ fn peel_parens(mut expr: ast::Expr) -> ast::Expr { expr } -fn original_expr( +fn source_expr( ctx: &AssistContext<'_, '_>, - file_id: hir::EditionedFileId, - range: TextRange, + range: RangeInclusive<SyntaxElement>, ) -> Option<ast::Expr> { - if file_id != ctx.file_id() { - return None; - } - + let range = range.start().text_range().cover(range.end().text_range()); let text = ctx.source_file().syntax().text().slice(range).to_string(); parse_expr_from_str(&text, ctx.edition()) } -fn prettify_macro_expr(ctx: &AssistContext<'_, '_>, module: Module, expr: ast::Expr) -> ast::Expr { - let Some(scope) = ctx.sema.scope(expr.syntax()) else { - return expr; - }; - let Some(macro_file) = scope.file_id().macro_file() else { - return expr; - }; - - let prettified = hir::prettify_macro_expansion( - ctx.db(), - expr.syntax().clone(), - macro_file.expansion_span_map(ctx.db()), - module.krate(ctx.db()).into(), - ); - ast::Expr::cast(prettified).unwrap_or(expr) -} - /// Check whether the node is a valid expression which can be extracted to a variable. /// In general that's true for any expression, but in some cases that would produce invalid code. fn valid_target_expr(ctx: &AssistContext<'_, '_>) -> impl Fn(SyntaxNode) -> Option<ast::Expr> { @@ -2864,7 +2846,6 @@ fn main() { #[test] fn extract_variable_in_token_tree() { - // FIXME: Keep the original trivia instead of extracting macro expanded? check_assist_by_label( extract_variable, r#" @@ -2981,6 +2962,34 @@ fn main() { "#, "Extract into variable", ); + + check_assist_by_label( + extract_variable, + r#" +macro_rules! identity { + ($e:expr) => { + $e + }; +} + +fn main() { + let x = identity!($0(1+2)$0); +} +"#, + r#" +macro_rules! identity { + ($e:expr) => { + $e + }; +} + +fn main() { + let $0var_name = (1+2); + let x = identity!(var_name); +} +"#, + "Extract into variable", + ); } #[test] |