diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 5349ebb7c82a..4f26d8aefd28 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -103,7 +103,7 @@ pub use crate::{ move_item::Direction, navigation_target::{NavigationTarget, TryToNav, UpmappingResult}, references::ReferenceSearchResult, - rename::RenameError, + rename::{RenameConfig, RenameError}, runnables::{Runnable, RunnableKind, TestId, UpdateTest}, signature_help::SignatureHelp, static_index::{ @@ -783,8 +783,9 @@ impl Analysis { &self, position: FilePosition, new_name: &str, + config: &RenameConfig, ) -> Cancellable> { - self.with_db(|db| rename::rename(db, position, new_name)) + self.with_db(|db| rename::rename(db, position, new_name, config)) } pub fn prepare_rename( diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 8922a8eb4858..00a3fb2ed22f 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -4,7 +4,7 @@ //! tests. This module also implements a couple of magic tricks, like renaming //! `self` and to `self` (to switch between associated function and method). -use hir::{AsAssocItem, InFile, Name, Semantics, sym}; +use hir::{AsAssocItem, HasContainer, HirDisplay, ImportPathConfig, InFile, Name, Semantics, sym}; use ide_db::{ FileId, FileRange, RootDatabase, defs::{Definition, NameClass, NameRefClass}, @@ -27,6 +27,23 @@ pub use ide_db::rename::RenameError; type RenameResult = Result; +pub struct RenameConfig { + pub prefer_no_std: bool, + pub prefer_prelude: bool, + pub prefer_absolute: bool, +} + +impl RenameConfig { + fn import_path_config(&self) -> ImportPathConfig { + ImportPathConfig { + prefer_no_std: self.prefer_no_std, + prefer_prelude: self.prefer_prelude, + prefer_absolute: self.prefer_absolute, + allow_unstable: true, + } + } +} + /// This is similar to `collect::, _>>`, but unlike it, it succeeds if there is *any* `Ok` item. fn ok_if_any(iter: impl Iterator>) -> Result, E> { let mut err = None; @@ -100,6 +117,7 @@ pub(crate) fn rename( db: &RootDatabase, position: FilePosition, new_name: &str, + config: &RenameConfig, ) -> RenameResult { let sema = Semantics::new(db); let file_id = sema @@ -158,7 +176,14 @@ pub(crate) fn rename( if let Definition::Local(local) = def { if let Some(self_param) = local.as_self_param(sema.db) { cov_mark::hit!(rename_self_to_param); - return rename_self_to_param(&sema, local, self_param, &new_name, kind); + return rename_self_to_param( + &sema, + local, + self_param, + &new_name, + kind, + config.import_path_config(), + ); } if kind == IdentifierKind::LowercaseSelf { cov_mark::hit!(rename_to_self); @@ -360,7 +385,7 @@ fn transform_assoc_fn_into_method_call( f: hir::Function, ) { let calls = Definition::Function(f).usages(sema).all(); - for (file_id, calls) in calls { + for (_file_id, calls) in calls { for call in calls { let Some(fn_name) = call.name.as_name_ref() else { continue }; let Some(path) = fn_name.syntax().parent().and_then(ast::PathSegment::cast) else { @@ -409,6 +434,12 @@ fn transform_assoc_fn_into_method_call( .unwrap_or_else(|| arg_list.syntax().text_range().end()), }; let replace_range = TextRange::new(replace_start, replace_end); + let macro_file = sema.hir_file_for(fn_name.syntax()); + let Some((replace_range, _)) = + InFile::new(macro_file, replace_range).original_node_file_range_opt(sema.db) + else { + continue; + }; let Some(macro_mapped_self) = sema.original_range_opt(self_arg.syntax()) else { continue; @@ -426,8 +457,8 @@ fn transform_assoc_fn_into_method_call( replacement.push('('); source_change.insert_source_edit( - file_id.file_id(sema.db), - TextEdit::replace(replace_range, replacement), + replace_range.file_id.file_id(sema.db), + TextEdit::replace(replace_range.range, replacement), ); } } @@ -514,12 +545,189 @@ fn rename_to_self( Ok(source_change) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CallReceiverAdjust { + Deref, + Ref, + RefMut, + None, +} + +fn method_to_assoc_fn_call_self_adjust( + sema: &Semantics<'_, RootDatabase>, + self_arg: &ast::Expr, +) -> CallReceiverAdjust { + let mut result = CallReceiverAdjust::None; + let self_adjust = sema.expr_adjustments(self_arg); + if let Some(self_adjust) = self_adjust { + let mut i = 0; + while i < self_adjust.len() { + if matches!(self_adjust[i].kind, hir::Adjust::Deref(..)) + && matches!( + self_adjust.get(i + 1), + Some(hir::Adjustment { kind: hir::Adjust::Borrow(..), .. }) + ) + { + // Deref then ref (reborrow), skip them. + i += 2; + continue; + } + + match self_adjust[i].kind { + hir::Adjust::Deref(_) if result == CallReceiverAdjust::None => { + // Autoref takes precedence over deref, because if given a `&Type` the compiler will deref + // it automatically. + result = CallReceiverAdjust::Deref; + } + hir::Adjust::Borrow(hir::AutoBorrow::Ref(mutability)) => { + match (result, mutability) { + (CallReceiverAdjust::RefMut, hir::Mutability::Shared) => {} + (_, hir::Mutability::Mut) => result = CallReceiverAdjust::RefMut, + (_, hir::Mutability::Shared) => result = CallReceiverAdjust::Ref, + } + } + _ => {} + } + + i += 1; + } + } + result +} + +fn transform_method_call_into_assoc_fn( + sema: &Semantics<'_, RootDatabase>, + source_change: &mut SourceChange, + f: hir::Function, + import_path_config: ImportPathConfig, +) { + let calls = Definition::Function(f).usages(sema).all(); + for (_file_id, calls) in calls { + for call in calls { + let Some(fn_name) = call.name.as_name_ref() else { continue }; + let Some(method_call) = fn_name.syntax().parent().and_then(ast::MethodCallExpr::cast) + else { + continue; + }; + let Some(mut self_arg) = method_call.receiver() else { + continue; + }; + + let Some(scope) = sema.scope(fn_name.syntax()) else { + continue; + }; + let self_adjust = method_to_assoc_fn_call_self_adjust(sema, &self_arg); + + // Strip parentheses, function arguments have higher precedence than any operator. + while let ast::Expr::ParenExpr(it) = &self_arg { + self_arg = match it.expr() { + Some(it) => it, + None => break, + }; + } + + let needs_comma = method_call.arg_list().is_some_and(|it| it.args().next().is_some()); + + let self_needs_parens = self_adjust != CallReceiverAdjust::None + && self_arg.precedence().needs_parentheses_in(ExprPrecedence::Prefix); + + let replace_start = method_call.syntax().text_range().start(); + let replace_end = method_call + .arg_list() + .and_then(|it| it.l_paren_token()) + .map(|it| it.text_range().end()) + .unwrap_or_else(|| method_call.syntax().text_range().end()); + let replace_range = TextRange::new(replace_start, replace_end); + let macro_file = sema.hir_file_for(fn_name.syntax()); + let Some((replace_range, _)) = + InFile::new(macro_file, replace_range).original_node_file_range_opt(sema.db) + else { + continue; + }; + + let fn_container_path = match f.container(sema.db) { + hir::ItemContainer::Trait(trait_) => { + // FIXME: We always put it as `Trait::function`. Is it better to use `Type::function` (but + // that could conflict with an inherent method)? Or maybe `::function`? + // Or let the user decide? + let Some(path) = scope.module().find_path( + sema.db, + hir::ItemInNs::Types(trait_.into()), + import_path_config, + ) else { + continue; + }; + path.display(sema.db, replace_range.file_id.edition(sema.db)).to_string() + } + hir::ItemContainer::Impl(impl_) => { + let ty = impl_.self_ty(sema.db); + match ty.as_adt() { + Some(adt) => { + let Some(path) = scope.module().find_path( + sema.db, + hir::ItemInNs::Types(adt.into()), + import_path_config, + ) else { + continue; + }; + path.display(sema.db, replace_range.file_id.edition(sema.db)) + .to_string() + } + None => { + let Ok(mut ty) = + ty.display_source_code(sema.db, scope.module().into(), false) + else { + continue; + }; + ty.insert(0, '<'); + ty.push('>'); + ty + } + } + } + _ => continue, + }; + + let Some(macro_mapped_self) = sema.original_range_opt(self_arg.syntax()) else { + continue; + }; + let mut replacement = String::new(); + replacement.push_str(&fn_container_path); + replacement.push_str("::"); + format_to!(replacement, "{fn_name}"); + replacement.push('('); + replacement.push_str(match self_adjust { + CallReceiverAdjust::Deref => "*", + CallReceiverAdjust::Ref => "&", + CallReceiverAdjust::RefMut => "&mut ", + CallReceiverAdjust::None => "", + }); + if self_needs_parens { + replacement.push('('); + } + replacement.push_str(macro_mapped_self.text(sema.db)); + if self_needs_parens { + replacement.push(')'); + } + if needs_comma { + replacement.push_str(", "); + } + + source_change.insert_source_edit( + replace_range.file_id.file_id(sema.db), + TextEdit::replace(replace_range.range, replacement), + ); + } + } +} + fn rename_self_to_param( sema: &Semantics<'_, RootDatabase>, local: hir::Local, self_param: hir::SelfParam, new_name: &Name, identifier_kind: IdentifierKind, + import_path_config: ImportPathConfig, ) -> RenameResult { if identifier_kind == IdentifierKind::LowercaseSelf { // Let's do nothing rather than complain. @@ -527,6 +735,11 @@ fn rename_self_to_param( return Ok(SourceChange::default()); } + let fn_def = match local.parent(sema.db) { + hir::DefWithBody::Function(func) => func, + _ => bail!("Cannot rename local to self outside of function"), + }; + let InFile { file_id, value: self_param } = sema.source(self_param).ok_or_else(|| format_err!("cannot find function source"))?; @@ -554,6 +767,7 @@ fn rename_self_to_param( ), ) })); + transform_method_call_into_assoc_fn(sema, &mut source_change, fn_def, import_path_config); Ok(source_change) } @@ -587,7 +801,10 @@ mod tests { use crate::fixture; - use super::{RangeInfo, RenameError}; + use super::{RangeInfo, RenameConfig, RenameError}; + + const TEST_CONFIG: RenameConfig = + RenameConfig { prefer_no_std: false, prefer_prelude: true, prefer_absolute: false }; #[track_caller] fn check( @@ -603,7 +820,7 @@ mod tests { panic!("Prepare rename to '{new_name}' was failed: {err}") } let rename_result = analysis - .rename(position, new_name) + .rename(position, new_name, &TEST_CONFIG) .unwrap_or_else(|err| panic!("Rename to '{new_name}' was cancelled: {err}")); match rename_result { Ok(source_change) => { @@ -635,7 +852,7 @@ mod tests { #[track_caller] fn check_conflicts(new_name: &str, #[rust_analyzer::rust_fixture] ra_fixture: &str) { let (analysis, position, conflicts) = fixture::annotations(ra_fixture); - let source_change = analysis.rename(position, new_name).unwrap().unwrap(); + let source_change = analysis.rename(position, new_name, &TEST_CONFIG).unwrap().unwrap(); let expected_conflicts = conflicts .into_iter() .map(|(file_range, _)| (file_range.file_id, file_range.range)) @@ -662,8 +879,10 @@ mod tests { expect: Expect, ) { let (analysis, position) = fixture::position(ra_fixture); - let source_change = - analysis.rename(position, new_name).unwrap().expect("Expect returned a RenameError"); + let source_change = analysis + .rename(position, new_name, &TEST_CONFIG) + .unwrap() + .expect("Expect returned a RenameError"); expect.assert_eq(&filter_expect(source_change)) } @@ -3585,6 +3804,117 @@ impl Foo { fn bar(v: Foo) { v.foo(123); +} + "#, + ); + } + + #[test] + fn rename_to_self_callers_in_macro() { + check( + "self", + r#" +struct Foo; + +impl Foo { + fn foo(th$0is: &Self, v: i32) {} +} + +macro_rules! m { ($it:expr) => { $it } } +fn bar(v: Foo) { + m!(Foo::foo(&v, 123)); +} + "#, + r#" +struct Foo; + +impl Foo { + fn foo(&self, v: i32) {} +} + +macro_rules! m { ($it:expr) => { $it } } +fn bar(v: Foo) { + m!(v.foo( 123)); +} + "#, + ); + } + + #[test] + fn rename_from_self_callers() { + check( + "this", + r#" +//- minicore: add +struct Foo; +impl Foo { + fn foo(&sel$0f) {} +} +impl core::ops::Add for Foo { + type Output = Foo; + + fn add(self, _rhs: Self) -> Self::Output { + Foo + } +} + +fn bar(v: &Foo) { + v.foo(); + (Foo + Foo).foo(); +} + +mod baz { + fn baz(v: super::Foo) { + v.foo(); + } +} + "#, + r#" +struct Foo; +impl Foo { + fn foo(this: &Self) {} +} +impl core::ops::Add for Foo { + type Output = Foo; + + fn add(self, _rhs: Self) -> Self::Output { + Foo + } +} + +fn bar(v: &Foo) { + Foo::foo(v); + Foo::foo(&(Foo + Foo)); +} + +mod baz { + fn baz(v: super::Foo) { + crate::Foo::foo(&v); + } +} + "#, + ); + // Multiple args: + check( + "this", + r#" +struct Foo; +impl Foo { + fn foo(&sel$0f, _v: i32) {} +} + +fn bar() { + Foo.foo(1); +} + "#, + r#" +struct Foo; +impl Foo { + fn foo(this: &Self, _v: i32) {} +} + +fn bar() { + Foo::foo(&Foo, 1); } "#, ); diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index d4cd56dc55bf..3431e414af4e 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -11,8 +11,8 @@ use ide::{ AssistConfig, CallHierarchyConfig, CallableSnippets, CompletionConfig, CompletionFieldsToResolve, DiagnosticsConfig, GenericParameterHints, HighlightConfig, HighlightRelatedConfig, HoverConfig, HoverDocFormat, InlayFieldsToResolve, InlayHintsConfig, - JoinLinesConfig, MemoryLayoutHoverConfig, MemoryLayoutHoverRenderKind, Snippet, SnippetScope, - SourceRootId, + JoinLinesConfig, MemoryLayoutHoverConfig, MemoryLayoutHoverRenderKind, RenameConfig, Snippet, + SnippetScope, SourceRootId, }; use ide_db::{ SnippetCap, @@ -1671,6 +1671,14 @@ impl Config { } } + pub fn rename(&self, source_root: Option) -> RenameConfig { + RenameConfig { + prefer_no_std: self.imports_preferNoStd(source_root).to_owned(), + prefer_prelude: self.imports_preferPrelude(source_root).to_owned(), + prefer_absolute: self.imports_prefixExternPrelude(source_root).to_owned(), + } + } + pub fn call_hierarchy(&self) -> CallHierarchyConfig { CallHierarchyConfig { exclude_tests: self.references_excludeTests().to_owned() } } diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 6cb28aecf748..9081be08b8e4 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -1322,8 +1322,12 @@ pub(crate) fn handle_rename( let _p = tracing::info_span!("handle_rename").entered(); let position = try_default!(from_proto::file_position(&snap, params.text_document_position)?); - let mut change = - snap.analysis.rename(position, ¶ms.new_name)?.map_err(to_proto::rename_error)?; + let source_root = snap.analysis.source_root_id(position.file_id).ok(); + let config = snap.config.rename(source_root); + let mut change = snap + .analysis + .rename(position, ¶ms.new_name, &config)? + .map_err(to_proto::rename_error)?; // this is kind of a hack to prevent double edits from happening when moving files // When a module gets renamed by renaming the mod declaration this causes the file to move