From b105e9b342c08e66c6049cd22f10b3a98f4d877e Mon Sep 17 00:00:00 2001 From: Ryan Mehri Date: Fri, 27 Oct 2023 10:46:22 -0700 Subject: [PATCH] fix: use original range to deal with macros in promote_local_to_const --- .../ide-assists/src/handlers/bool_to_enum.rs | 27 ++---- .../src/handlers/promote_local_to_const.rs | 89 +++++++++++++++++-- crates/ide-assists/src/utils.rs | 18 ++++ 3 files changed, 104 insertions(+), 30 deletions(-) diff --git a/crates/ide-assists/src/handlers/bool_to_enum.rs b/crates/ide-assists/src/handlers/bool_to_enum.rs index 0751be1fd14..b7b00e7ed06 100644 --- a/crates/ide-assists/src/handlers/bool_to_enum.rs +++ b/crates/ide-assists/src/handlers/bool_to_enum.rs @@ -20,7 +20,10 @@ use syntax::{ }; use text_edit::TextRange; -use crate::assist_context::{AssistContext, Assists}; +use crate::{ + assist_context::{AssistContext, Assists}, + utils, +}; // Assist: bool_to_enum // @@ -238,7 +241,7 @@ fn replace_usages( cov_mark::hit!(replaces_record_expr); let enum_expr = bool_expr_to_enum_expr(initializer); - replace_record_field_expr(ctx, edit, record_field, enum_expr); + utils::replace_record_field_expr(ctx, edit, record_field, enum_expr); } else if let Some(pat) = find_record_pat_field_usage(&name) { match pat { ast::Pat::IdentPat(ident_pat) => { @@ -281,7 +284,7 @@ fn replace_usages( |record_field| record_field.expr().map(|expr| (record_field, expr)), ) { - replace_record_field_expr( + utils::replace_record_field_expr( ctx, edit, record_field, @@ -310,24 +313,6 @@ fn replace_usages( } } -/// Replaces the record expression, handling field shorthands. -fn replace_record_field_expr( - ctx: &AssistContext<'_>, - edit: &mut SourceChangeBuilder, - record_field: ast::RecordExprField, - initializer: ast::Expr, -) { - if let Some(ast::Expr::PathExpr(path_expr)) = record_field.expr() { - // replace field shorthand - let file_range = ctx.sema.original_range(path_expr.syntax()); - edit.insert(file_range.range.end(), format!(": {}", initializer.syntax().text())) - } else if let Some(expr) = record_field.expr() { - // just replace expr - let file_range = ctx.sema.original_range(expr.syntax()); - edit.replace(file_range.range, initializer.syntax().text()); - } -} - struct FileReferenceWithImport { range: TextRange, name: ast::NameLike, diff --git a/crates/ide-assists/src/handlers/promote_local_to_const.rs b/crates/ide-assists/src/handlers/promote_local_to_const.rs index 6ed9bd85fcc..67fea772c79 100644 --- a/crates/ide-assists/src/handlers/promote_local_to_const.rs +++ b/crates/ide-assists/src/handlers/promote_local_to_const.rs @@ -11,7 +11,10 @@ use syntax::{ ted, AstNode, WalkEvent, }; -use crate::assist_context::{AssistContext, Assists}; +use crate::{ + assist_context::{AssistContext, Assists}, + utils, +}; // Assist: promote_local_to_const // @@ -79,15 +82,13 @@ pub(crate) fn promote_local_to_const(acc: &mut Assists, ctx: &AssistContext<'_>) let name_ref = make::name_ref(&name); for usage in usages { - let Some(usage) = usage.name.as_name_ref().cloned() else { continue }; - if let Some(record_field) = ast::RecordExprField::for_name_ref(&usage) { - let record_field = edit.make_mut(record_field); - let name_expr = - make::expr_path(make::path_from_text(&name)).clone_for_update(); - record_field.replace_expr(name_expr); + let Some(usage_name) = usage.name.as_name_ref().cloned() else { continue }; + if let Some(record_field) = ast::RecordExprField::for_name_ref(&usage_name) { + let name_expr = make::expr_path(make::path_from_text(&name)); + utils::replace_record_field_expr(ctx, edit, record_field, name_expr); } else { - let usage = edit.make_mut(usage); - ted::replace(usage.syntax(), name_ref.clone_for_update().syntax()); + let usage_range = usage.range; + edit.replace(usage_range, name_ref.syntax().text()); } } } @@ -212,6 +213,76 @@ fn main() { ) } + #[test] + fn usage_in_macro() { + check_assist( + promote_local_to_const, + r" +macro_rules! identity { + ($body:expr) => { + $body + } +} + +fn baz() -> usize { + let $0foo = 2; + identity![foo] +} +", + r" +macro_rules! identity { + ($body:expr) => { + $body + } +} + +fn baz() -> usize { + const $0FOO: usize = 2; + identity![FOO] +} +", + ) + } + + #[test] + fn usage_shorthand_in_macro() { + check_assist( + promote_local_to_const, + r" +struct Foo { + foo: usize, +} + +macro_rules! identity { + ($body:expr) => { + $body + }; +} + +fn baz() -> Foo { + let $0foo = 2; + identity![Foo { foo }] +} +", + r" +struct Foo { + foo: usize, +} + +macro_rules! identity { + ($body:expr) => { + $body + }; +} + +fn baz() -> Foo { + const $0FOO: usize = 2; + identity![Foo { foo: FOO }] +} +", + ) + } + #[test] fn not_applicable_non_const_meth_call() { cov_mark::check!(promote_local_non_const); diff --git a/crates/ide-assists/src/utils.rs b/crates/ide-assists/src/utils.rs index f51e99a914e..927a8e3c19a 100644 --- a/crates/ide-assists/src/utils.rs +++ b/crates/ide-assists/src/utils.rs @@ -813,3 +813,21 @@ fn test_required_hashes() { assert_eq!(3, required_hashes("#ab\"##c")); assert_eq!(5, required_hashes("#ab\"##\"####c")); } + +/// Replaces the record expression, handling field shorthands including inside macros. +pub(crate) fn replace_record_field_expr( + ctx: &AssistContext<'_>, + edit: &mut SourceChangeBuilder, + record_field: ast::RecordExprField, + initializer: ast::Expr, +) { + if let Some(ast::Expr::PathExpr(path_expr)) = record_field.expr() { + // replace field shorthand + let file_range = ctx.sema.original_range(path_expr.syntax()); + edit.insert(file_range.range.end(), format!(": {}", initializer.syntax().text())) + } else if let Some(expr) = record_field.expr() { + // just replace expr + let file_range = ctx.sema.original_range(expr.syntax()); + edit.replace(file_range.range, initializer.syntax().text()); + } +}