From 7ae70a06ceb9b6a2619bb91f8d839417c3ee3f67 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 8 Jul 2023 02:39:03 +0200 Subject: [PATCH 1/5] Disallow renaming of non-local structs --- crates/ide-db/src/rename.rs | 11 ++++++++++- crates/ide/src/rename.rs | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index aa0bb7cce691..27add0ad3711 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -22,7 +22,7 @@ //! Our current behavior is ¯\_(ツ)_/¯. use std::fmt; -use base_db::{AnchoredPathBuf, FileId, FileRange}; +use base_db::{AnchoredPathBuf, CrateOrigin, FileId, FileRange}; use either::Either; use hir::{FieldSource, HasSource, InFile, ModuleSource, Semantics}; use stdx::never; @@ -77,6 +77,15 @@ impl Definition { bail!("Cannot rename builtin type") } Definition::SelfType(_) => bail!("Cannot rename `Self`"), + Definition::Adt(hir::Adt::Struct(strukt)) => { + if !matches!( + strukt.module(sema.db).krate().origin(sema.db), + CrateOrigin::Local { .. } + ) { + bail!("Cannot rename a non-local struct.") + } + rename_reference(sema, Definition::Adt(hir::Adt::Struct(strukt)), new_name) + } def => rename_reference(sema, def, new_name), } } diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index dae8e71e8a09..3017ca75366c 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -2529,6 +2529,7 @@ fn main() { ", ) } +<<<<<<< HEAD #[test] fn extern_crate() { @@ -2634,4 +2635,21 @@ use qux as frob; // ", // ); } +||||||| parent of 948d9f274 (Disallow renaming of non-local structs) +======= + + #[test] + fn disallow_renaming_for_non_local_struct() { + check( + "Baz", + r#" +//- /lib.rs crate:lib new_source_root:library +pub struct S$0; +//- /main.rs crate:main deps:lib new_source_root:local +use lib::S; +"#, + "error: Cannot rename a non-local struct.", + ); + } +>>>>>>> 948d9f274 (Disallow renaming of non-local structs) } From 43edb51b21e94662f61398079c8d30f19641561a Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sat, 8 Jul 2023 12:22:07 +0200 Subject: [PATCH 2/5] Generalize disallowing of definition renaming --- crates/ide-db/src/rename.rs | 18 +++++++++++------- crates/ide/src/rename.rs | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index 27add0ad3711..adb9e55613f9 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -71,20 +71,24 @@ impl Definition { sema: &Semantics<'_, RootDatabase>, new_name: &str, ) -> Result { + if let Some(krate) = self.krate(sema.db) { + if !matches!(krate.origin(sema.db), CrateOrigin::Local { .. }) { + bail!("Cannot rename a non-local definition.") + } + } + match *self { Definition::Module(module) => rename_mod(sema, module, new_name), Definition::BuiltinType(_) => { bail!("Cannot rename builtin type") } Definition::SelfType(_) => bail!("Cannot rename `Self`"), - Definition::Adt(hir::Adt::Struct(strukt)) => { - if !matches!( - strukt.module(sema.db).krate().origin(sema.db), - CrateOrigin::Local { .. } - ) { - bail!("Cannot rename a non-local struct.") + Definition::Macro(mac) => { + if mac.is_builtin_derive(sema.db) { + bail!("Cannot rename builtin macro") } - rename_reference(sema, Definition::Adt(hir::Adt::Struct(strukt)), new_name) + + rename_reference(sema, Definition::Macro(mac), new_name) } def => rename_reference(sema, def, new_name), } diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 3017ca75366c..5a6742d5c84b 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -2639,7 +2639,7 @@ use qux as frob; ======= #[test] - fn disallow_renaming_for_non_local_struct() { + fn disallow_renaming_for_non_local_definition() { check( "Baz", r#" @@ -2648,7 +2648,7 @@ pub struct S$0; //- /main.rs crate:main deps:lib new_source_root:local use lib::S; "#, - "error: Cannot rename a non-local struct.", + "error: Cannot rename a non-local definition.", ); } >>>>>>> 948d9f274 (Disallow renaming of non-local structs) From 6dc7fa94235f1b2ca9ecd8bf90a96c7c1871f111 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Thu, 24 Aug 2023 02:01:07 +0200 Subject: [PATCH 3/5] v3 --- crates/ide-db/src/rename.rs | 14 +++++++++++--- crates/ide/src/rename.rs | 23 +++++++++++++++++------ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index adb9e55613f9..a60b24235959 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -22,7 +22,7 @@ //! Our current behavior is ¯\_(ツ)_/¯. use std::fmt; -use base_db::{AnchoredPathBuf, CrateOrigin, FileId, FileRange}; +use base_db::{AnchoredPathBuf, FileId, FileRange}; use either::Either; use hir::{FieldSource, HasSource, InFile, ModuleSource, Semantics}; use stdx::never; @@ -71,21 +71,29 @@ impl Definition { sema: &Semantics<'_, RootDatabase>, new_name: &str, ) -> Result { + // self.krate() returns None if + // self is a built-in attr, built-in type or tool module. if let Some(krate) = self.krate(sema.db) { - if !matches!(krate.origin(sema.db), CrateOrigin::Local { .. }) { + if !krate.origin(sema.db).is_local() { bail!("Cannot rename a non-local definition.") } } match *self { Definition::Module(module) => rename_mod(sema, module, new_name), + Definition::ToolModule(_) => { + bail!("Cannot rename a tool module") + } Definition::BuiltinType(_) => { bail!("Cannot rename builtin type") } + Definition::BuiltinAttr(_) => { + bail!("Cannot rename a builtin attr.") + } Definition::SelfType(_) => bail!("Cannot rename `Self`"), Definition::Macro(mac) => { if mac.is_builtin_derive(sema.db) { - bail!("Cannot rename builtin macro") + bail!("Cannot rename builtin derive") } rename_reference(sema, Definition::Macro(mac), new_name) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 5a6742d5c84b..ac9df5ed6d1f 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -2529,7 +2529,6 @@ fn main() { ", ) } -<<<<<<< HEAD #[test] fn extern_crate() { @@ -2635,8 +2634,6 @@ use qux as frob; // ", // ); } -||||||| parent of 948d9f274 (Disallow renaming of non-local structs) -======= #[test] fn disallow_renaming_for_non_local_definition() { @@ -2644,12 +2641,26 @@ use qux as frob; "Baz", r#" //- /lib.rs crate:lib new_source_root:library -pub struct S$0; +pub struct S; //- /main.rs crate:main deps:lib new_source_root:local -use lib::S; +use lib::S$0; "#, "error: Cannot rename a non-local definition.", ); } ->>>>>>> 948d9f274 (Disallow renaming of non-local structs) + + #[test] + fn disallow_renaming_for_builtin_macros() { + check( + "Baz", + r#" +//- minicore: derive, hash +//- /main.rs crate:main +use core::hash::Hash; +#[derive(H$0ash)] +struct A; + "#, + "error: Cannot rename a non-local definition.", + ) + } } From a0c8bee35edcfb16c2c92f5c61bc574d6e555a44 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Thu, 24 Aug 2023 02:05:14 +0200 Subject: [PATCH 4/5] Add more comments as requested --- crates/ide-db/src/rename.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index a60b24235959..f6de3a888488 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -73,6 +73,8 @@ impl Definition { ) -> 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.") From 0118741632f9999da1db1071ae1a9b430d438d43 Mon Sep 17 00:00:00 2001 From: Ali Bektas Date: Sun, 10 Sep 2023 23:25:36 +0200 Subject: [PATCH 5/5] v4 --- crates/ide-db/src/rename.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index f6de3a888488..353a9749a37d 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -93,13 +93,7 @@ impl Definition { bail!("Cannot rename a builtin attr.") } Definition::SelfType(_) => bail!("Cannot rename `Self`"), - Definition::Macro(mac) => { - if mac.is_builtin_derive(sema.db) { - bail!("Cannot rename builtin derive") - } - - rename_reference(sema, Definition::Macro(mac), new_name) - } + Definition::Macro(mac) => rename_reference(sema, Definition::Macro(mac), new_name), def => rename_reference(sema, def, new_name), } }