From 724b85ef11213e128985a22c073c7373d80fd792 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 10 Feb 2025 15:47:31 +0200 Subject: [PATCH] Fix postfix completions inside macros Previously the receiver text was taken directly from the AST, which in macros is missing trivia, leading to corruption (or just unintended replacement of user code). Now we upmap the range, and extract the original file text in it. --- .../ide-completion/src/completions/postfix.rs | 91 +++++++++++++------ 1 file changed, 64 insertions(+), 27 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide-completion/src/completions/postfix.rs b/src/tools/rust-analyzer/crates/ide-completion/src/completions/postfix.rs index 2c39a8fdfed7..28e2853096e0 100644 --- a/src/tools/rust-analyzer/crates/ide-completion/src/completions/postfix.rs +++ b/src/tools/rust-analyzer/crates/ide-completion/src/completions/postfix.rs @@ -2,17 +2,18 @@ mod format_like; -use hir::ItemInNs; -use ide_db::text_edit::TextEdit; +use base_db::SourceDatabase; +use hir::{ItemInNs, Semantics}; use ide_db::{ documentation::{Documentation, HasDocs}, imports::insert_use::ImportScope, + text_edit::TextEdit, ty_filter::TryEnum, - SnippetCap, + RootDatabase, SnippetCap, }; use stdx::never; use syntax::{ - ast::{self, make, AstNode, AstToken}, + ast::{self, AstNode, AstToken}, SyntaxKind::{BLOCK_EXPR, EXPR_STMT, FOR_EXPR, IF_EXPR, LOOP_EXPR, STMT_LIST, WHILE_EXPR}, TextRange, TextSize, }; @@ -48,7 +49,8 @@ pub(crate) fn complete_postfix( }; let expr_ctx = &dot_access.ctx; - let receiver_text = get_receiver_text(dot_receiver, receiver_is_ambiguous_float_literal); + let receiver_text = + get_receiver_text(&ctx.sema, dot_receiver, receiver_is_ambiguous_float_literal); let cap = match ctx.config.snippet_cap { Some(it) => it, @@ -172,13 +174,15 @@ pub(crate) fn complete_postfix( // The rest of the postfix completions create an expression that moves an argument, // so it's better to consider references now to avoid breaking the compilation - let (dot_receiver, node_to_replace_with) = include_references(dot_receiver); - let receiver_text = - get_receiver_text(&node_to_replace_with, receiver_is_ambiguous_float_literal); - let postfix_snippet = match build_postfix_snippet_builder(ctx, cap, &dot_receiver) { - Some(it) => it, - None => return, - }; + let (dot_receiver_including_refs, prefix) = include_references(dot_receiver); + let mut receiver_text = + get_receiver_text(&ctx.sema, dot_receiver, receiver_is_ambiguous_float_literal); + receiver_text.insert_str(0, &prefix); + let postfix_snippet = + match build_postfix_snippet_builder(ctx, cap, &dot_receiver_including_refs) { + Some(it) => it, + None => return, + }; if !ctx.config.snippets.is_empty() { add_custom_postfix_completions(acc, ctx, &postfix_snippet, &receiver_text); @@ -222,7 +226,7 @@ pub(crate) fn complete_postfix( postfix_snippet("call", "function(expr)", &format!("${{1}}({receiver_text})")) .add_to(acc, ctx.db); - if let Some(parent) = dot_receiver.syntax().parent().and_then(|p| p.parent()) { + if let Some(parent) = dot_receiver_including_refs.syntax().parent().and_then(|p| p.parent()) { if matches!(parent.kind(), STMT_LIST | EXPR_STMT) { postfix_snippet("let", "let", &format!("let $0 = {receiver_text};")) .add_to(acc, ctx.db); @@ -231,9 +235,9 @@ pub(crate) fn complete_postfix( } } - if let ast::Expr::Literal(literal) = dot_receiver.clone() { + if let ast::Expr::Literal(literal) = dot_receiver_including_refs.clone() { if let Some(literal_text) = ast::String::cast(literal.token()) { - add_format_like_completions(acc, ctx, &dot_receiver, cap, &literal_text); + add_format_like_completions(acc, ctx, &dot_receiver_including_refs, cap, &literal_text); } } @@ -260,14 +264,20 @@ pub(crate) fn complete_postfix( } } -fn get_receiver_text(receiver: &ast::Expr, receiver_is_ambiguous_float_literal: bool) -> String { - let mut text = if receiver_is_ambiguous_float_literal { - let text = receiver.syntax().text(); - let without_dot = ..text.len() - TextSize::of('.'); - text.slice(without_dot).to_string() - } else { - receiver.to_string() +fn get_receiver_text( + sema: &Semantics<'_, RootDatabase>, + receiver: &ast::Expr, + receiver_is_ambiguous_float_literal: bool, +) -> String { + // Do not just call `receiver.to_string()`, as that will mess up whitespaces inside macros. + let Some(mut range) = sema.original_range_opt(receiver.syntax()) else { + return receiver.to_string(); }; + if receiver_is_ambiguous_float_literal { + range.range = TextRange::at(range.range.start(), range.range.len() - TextSize::of('.')) + } + let file_text = sema.db.file_text(range.file_id.file_id()); + let mut text = file_text[range.range].to_owned(); // The receiver texts should be interpreted as-is, as they are expected to be // normal Rust expressions. @@ -284,7 +294,7 @@ fn escape_snippet_bits(text: &mut String) { stdx::replace(text, '$', "\\$"); } -fn include_references(initial_element: &ast::Expr) -> (ast::Expr, ast::Expr) { +fn include_references(initial_element: &ast::Expr) -> (ast::Expr, String) { let mut resulting_element = initial_element.clone(); while let Some(field_expr) = resulting_element.syntax().parent().and_then(ast::FieldExpr::cast) @@ -292,7 +302,7 @@ fn include_references(initial_element: &ast::Expr) -> (ast::Expr, ast::Expr) { resulting_element = ast::Expr::from(field_expr); } - let mut new_element_opt = initial_element.clone(); + let mut prefix = String::new(); while let Some(parent_deref_element) = resulting_element.syntax().parent().and_then(ast::PrefixExpr::cast) @@ -303,7 +313,7 @@ fn include_references(initial_element: &ast::Expr) -> (ast::Expr, ast::Expr) { resulting_element = ast::Expr::from(parent_deref_element); - new_element_opt = make::expr_prefix(syntax::T![*], new_element_opt).into(); + prefix.insert(0, '*'); } if let Some(first_ref_expr) = resulting_element.syntax().parent().and_then(ast::RefExpr::cast) { @@ -317,7 +327,7 @@ fn include_references(initial_element: &ast::Expr) -> (ast::Expr, ast::Expr) { let exclusive = parent_ref_element.mut_token().is_some(); resulting_element = ast::Expr::from(parent_ref_element); - new_element_opt = make::expr_ref(new_element_opt, exclusive); + prefix.insert_str(0, if exclusive { "&mut " } else { "&" }); } } else { // If we do not find any ref expressions, restore @@ -325,7 +335,7 @@ fn include_references(initial_element: &ast::Expr) -> (ast::Expr, ast::Expr) { resulting_element = initial_element.clone(); } - (resulting_element, new_element_opt) + (resulting_element, prefix) } fn build_postfix_snippet_builder<'ctx>( @@ -901,4 +911,31 @@ fn main() { "#, ); } + + #[test] + fn inside_macro() { + check_edit( + "box", + r#" +macro_rules! assert { + ( $it:expr $(,)? ) => { $it }; +} + +fn foo() { + let a = true; + assert!(if a == false { true } else { false }.$0); +} + "#, + r#" +macro_rules! assert { + ( $it:expr $(,)? ) => { $it }; +} + +fn foo() { + let a = true; + assert!(Box::new(if a == false { true } else { false })); +} + "#, + ); + } }