From 9bd9a17ce56ae7be8ab9b2eb83e805514794e36c Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Wed, 17 Jan 2024 18:02:41 +0100 Subject: [PATCH] Add a new config to allow renaming of non-local items With #15656 we started disallowing renaming of non-local items. Although this makes sense there are some false positives that impacted users' workflows. So this config aims to mitigate this by giving users the liberty to disable this feature. --- crates/ide-db/src/rename.rs | 7 ++- .../src/handlers/incorrect_case.rs | 2 +- crates/ide/src/lib.rs | 3 +- crates/ide/src/rename.rs | 47 ++++++++++++++----- crates/rust-analyzer/src/config.rs | 7 +++ crates/rust-analyzer/src/handlers/request.rs | 6 ++- docs/user/generated_config.adoc | 5 ++ editors/code/package.json | 5 ++ 8 files changed, 65 insertions(+), 17 deletions(-) diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index f694f7160dea..a6058b57f1ea 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -71,14 +71,17 @@ impl Definition { &self, sema: &Semantics<'_, RootDatabase>, new_name: &str, + rename_external: bool, ) -> Result { // self.krate() returns None if // self is a built-in attr, built-in type or tool module. // it is not allowed for these defs to be renamed. // cases where self.krate() is None is handled below. if let Some(krate) = self.krate(sema.db) { - if !krate.origin(sema.db).is_local() { - bail!("Cannot rename a non-local definition.") + // Can we not rename non-local items? + // Then bail if non-local + if !rename_external && !krate.origin(sema.db).is_local() { + bail!("Cannot rename a non-local definition. Set `renameExternalItems` to `true` to allow renaming for this item.") } } diff --git a/crates/ide-diagnostics/src/handlers/incorrect_case.rs b/crates/ide-diagnostics/src/handlers/incorrect_case.rs index 0f12e814ba39..25a5cdde26b9 100644 --- a/crates/ide-diagnostics/src/handlers/incorrect_case.rs +++ b/crates/ide-diagnostics/src/handlers/incorrect_case.rs @@ -43,7 +43,7 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::IncorrectCase) -> Option Cancellable> { - self.with_db(|db| rename::rename(db, position, new_name)) + self.with_db(|db| rename::rename(db, position, new_name, rename_external)) } pub fn prepare_rename( diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 3bf41defe343..e678d43487e2 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -84,6 +84,7 @@ pub(crate) fn rename( db: &RootDatabase, position: FilePosition, new_name: &str, + rename_external: bool, ) -> RenameResult { let sema = Semantics::new(db); let source_file = sema.parse(position.file_id); @@ -103,7 +104,7 @@ pub(crate) fn rename( return rename_to_self(&sema, local); } } - def.rename(&sema, new_name) + def.rename(&sema, new_name, rename_external) }) .collect(); @@ -122,9 +123,9 @@ pub(crate) fn will_rename_file( let module = sema.to_module_def(file_id)?; let def = Definition::Module(module); let mut change = if is_raw_identifier(new_name_stem) { - def.rename(&sema, &SmolStr::from_iter(["r#", new_name_stem])).ok()? + def.rename(&sema, &SmolStr::from_iter(["r#", new_name_stem]), true).ok()? } else { - def.rename(&sema, new_name_stem).ok()? + def.rename(&sema, new_name_stem, true).ok()? }; change.file_system_edits.clear(); Some(change) @@ -375,8 +376,17 @@ mod tests { use super::{RangeInfo, RenameError}; - #[track_caller] fn check(new_name: &str, ra_fixture_before: &str, ra_fixture_after: &str) { + check_with_rename_config(new_name, ra_fixture_before, ra_fixture_after, true); + } + + #[track_caller] + fn check_with_rename_config( + new_name: &str, + ra_fixture_before: &str, + ra_fixture_after: &str, + rename_external: bool, + ) { let ra_fixture_after = &trim_indent(ra_fixture_after); let (analysis, position) = fixture::position(ra_fixture_before); if !ra_fixture_after.starts_with("error: ") { @@ -385,7 +395,7 @@ mod tests { } } let rename_result = analysis - .rename(position, new_name) + .rename(position, new_name, rename_external) .unwrap_or_else(|err| panic!("Rename to '{new_name}' was cancelled: {err}")); match rename_result { Ok(source_change) => { @@ -417,8 +427,10 @@ mod tests { fn check_expect(new_name: &str, ra_fixture: &str, 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, true) + .unwrap() + .expect("Expect returned a RenameError"); expect.assert_eq(&filter_expect(source_change)) } @@ -2617,6 +2629,18 @@ use qux as frob; #[test] fn disallow_renaming_for_non_local_definition() { + check_with_rename_config( + "Baz", + r#" +//- /lib.rs crate:lib new_source_root:library +pub struct S; +//- /main.rs crate:main deps:lib new_source_root:local +use lib::S$0; +"#, + "error: Cannot rename a non-local definition. Set `renameExternalItems` to `true` to allow renaming for this item.", + false, + ); + check( "Baz", r#" @@ -2625,13 +2649,13 @@ pub struct S; //- /main.rs crate:main deps:lib new_source_root:local use lib::S$0; "#, - "error: Cannot rename a non-local definition.", + "use lib::Baz;", ); } #[test] fn disallow_renaming_for_builtin_macros() { - check( + check_with_rename_config( "Baz", r#" //- minicore: derive, hash @@ -2640,8 +2664,9 @@ use core::hash::Hash; #[derive(H$0ash)] struct A; "#, - "error: Cannot rename a non-local definition.", - ) + "error: Cannot rename a non-local definition. Set `renameExternalItems` to `true` to allow renaming for this item.", + false, + ); } #[test] diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 3ec5d8696615..b8c9ae3d11a1 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -494,6 +494,9 @@ config_data! { /// Exclude imports from find-all-references. references_excludeImports: bool = "false", + /// Allow renaming of items not belonging to any workspace crates. + renameExternalItems_enable : bool = "false", + /// Command to be executed instead of 'cargo' for runnables. runnables_command: Option = "null", /// Additional arguments to be passed to cargo for runnables such as @@ -1739,6 +1742,10 @@ impl Config { self.data.typing_autoClosingAngleBrackets_enable } + pub fn rename(&self) -> bool { + self.data.renameExternalItems_enable + } + // FIXME: VSCode seems to work wrong sometimes, see https://github.com/microsoft/vscode/issues/193124 // hence, distinguish it for now. pub fn is_visual_studio_code(&self) -> bool { diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 22c7e9b0503c..349e6e50239b 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -1017,8 +1017,10 @@ pub(crate) fn handle_rename( let _p = profile::span("handle_rename"); let position = 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 mut change = snap + .analysis + .rename(position, ¶ms.new_name, snap.config.rename())? + .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 diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 1a2791954e62..3eb65166b959 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -777,6 +777,11 @@ Internal config, path to proc-macro server executable. -- Exclude imports from find-all-references. -- +[[rust-analyzer.renameExternalItems.enable]]rust-analyzer.renameExternalItems.enable (default: `false`):: ++ +-- +Allow renaming of items not belonging to any workspace crates. +-- [[rust-analyzer.runnables.command]]rust-analyzer.runnables.command (default: `null`):: + -- diff --git a/editors/code/package.json b/editors/code/package.json index e7ceee165cdc..2ace7569996d 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -1503,6 +1503,11 @@ "default": false, "type": "boolean" }, + "rust-analyzer.renameExternalItems.enable": { + "markdownDescription": "Allow renaming of items not belonging to any workspace crates.", + "default": false, + "type": "boolean" + }, "rust-analyzer.runnables.command": { "markdownDescription": "Command to be executed instead of 'cargo' for runnables.", "default": null,