Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #14138 - lowr:fix/rename-raw-ident-mod, r=Veykril
fix: don't include `r#` prefix in filesystem changes
Fixes #14131
In addition to fix for #14131, this PR adds raw ident validity checks in rename functionality that we've been missing.
| -rw-r--r-- | crates/hir-expand/src/name.rs | 7 | ||||
| -rw-r--r-- | crates/ide-db/src/rename.rs | 18 | ||||
| -rw-r--r-- | crates/ide/src/rename.rs | 154 | ||||
| -rw-r--r-- | crates/syntax/src/ast/make.rs | 5 | ||||
| -rw-r--r-- | crates/syntax/src/utils.rs | 7 |
5 files changed, 176 insertions, 15 deletions
diff --git a/crates/hir-expand/src/name.rs b/crates/hir-expand/src/name.rs index 0e4d20c07e..c3462beac7 100644 --- a/crates/hir-expand/src/name.rs +++ b/crates/hir-expand/src/name.rs @@ -2,7 +2,7 @@ use std::fmt; -use syntax::{ast, SmolStr, SyntaxKind}; +use syntax::{ast, utils::is_raw_identifier, SmolStr}; /// `Name` is a wrapper around string, which is used in hir for both references /// and declarations. In theory, names should also carry hygiene info, but we are @@ -33,11 +33,6 @@ impl fmt::Display for Name { } } -fn is_raw_identifier(name: &str) -> bool { - let is_keyword = SyntaxKind::from_keyword(name).is_some(); - is_keyword && !matches!(name, "self" | "crate" | "super" | "Self") -} - impl<'a> fmt::Display for UnescapedName<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.0 .0 { diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index 6da650aeb6..84d70b258f 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -190,6 +190,7 @@ fn rename_mod( let InFile { file_id, value: def_source } = module.definition_source(sema.db); if let ModuleSource::SourceFile(..) = def_source { + let new_name = new_name.trim_start_matches("r#"); let anchor = file_id.original_file(sema.db); let is_mod_rs = module.is_mod_rs(sema.db); @@ -207,9 +208,13 @@ fn rename_mod( // - Module has submodules defined in separate files let dir_paths = match (is_mod_rs, has_detached_child, module.name(sema.db)) { // Go up one level since the anchor is inside the dir we're trying to rename - (true, _, Some(mod_name)) => Some((format!("../{mod_name}"), format!("../{new_name}"))), + (true, _, Some(mod_name)) => { + Some((format!("../{}", mod_name.unescaped()), format!("../{new_name}"))) + } // The anchor is on the same level as target dir - (false, true, Some(mod_name)) => Some((mod_name.to_string(), new_name.to_string())), + (false, true, Some(mod_name)) => { + Some((mod_name.unescaped().to_string(), new_name.to_string())) + } _ => None, }; @@ -532,7 +537,14 @@ impl IdentifierKind { pub fn classify(new_name: &str) -> Result<IdentifierKind> { match parser::LexedStr::single_token(new_name) { Some(res) => match res { - (SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident), + (SyntaxKind::IDENT, _) => { + if let Some(inner) = new_name.strip_prefix("r#") { + if matches!(inner, "self" | "crate" | "super" | "Self") { + bail!("Invalid name: `{}` cannot be a raw identifier", inner); + } + } + Ok(IdentifierKind::Ident) + } (T![_], _) => Ok(IdentifierKind::Underscore), (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => { Ok(IdentifierKind::Lifetime) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 25d165d111..8e89160ef5 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -13,7 +13,7 @@ use ide_db::{ }; use itertools::Itertools; use stdx::{always, never}; -use syntax::{ast, AstNode, SyntaxNode, TextRange, TextSize}; +use syntax::{ast, utils::is_raw_identifier, AstNode, SmolStr, SyntaxNode, TextRange, TextSize}; use text_edit::TextEdit; @@ -122,7 +122,11 @@ pub(crate) fn will_rename_file( let sema = Semantics::new(db); let module = sema.to_module_def(file_id)?; let def = Definition::Module(module); - let mut change = def.rename(&sema, new_name_stem).ok()?; + let mut change = if is_raw_identifier(new_name_stem) { + def.rename(&sema, &SmolStr::from_iter(["r#", new_name_stem])).ok()? + } else { + def.rename(&sema, new_name_stem).ok()? + }; change.file_system_edits.clear(); Some(change) } @@ -559,6 +563,15 @@ impl Foo { } #[test] + fn test_rename_mod_invalid_raw_ident() { + check( + "r#self", + r#"mod foo$0 {}"#, + "error: Invalid name: `self` cannot be a raw identifier", + ); + } + + #[test] fn test_rename_for_local() { check( "k", @@ -1287,6 +1300,143 @@ mod bar$0; } #[test] + fn test_rename_mod_to_raw_ident() { + check_expect( + "r#fn", + r#" +//- /lib.rs +mod foo$0; + +fn main() { foo::bar::baz(); } + +//- /foo.rs +pub mod bar; + +//- /foo/bar.rs +pub fn baz() {} +"#, + expect![[r#" + SourceChange { + source_file_edits: { + FileId( + 0, + ): TextEdit { + indels: [ + Indel { + insert: "r#fn", + delete: 4..7, + }, + Indel { + insert: "r#fn", + delete: 22..25, + }, + ], + }, + }, + file_system_edits: [ + MoveFile { + src: FileId( + 1, + ), + dst: AnchoredPathBuf { + anchor: FileId( + 1, + ), + path: "fn.rs", + }, + }, + MoveDir { + src: AnchoredPathBuf { + anchor: FileId( + 1, + ), + path: "foo", + }, + src_id: FileId( + 1, + ), + dst: AnchoredPathBuf { + anchor: FileId( + 1, + ), + path: "fn", + }, + }, + ], + is_snippet: false, + } + "#]], + ); + } + + #[test] + fn test_rename_mod_from_raw_ident() { + // FIXME: `r#fn` in path expression is not renamed. + check_expect( + "foo", + r#" +//- /lib.rs +mod r#fn$0; + +fn main() { r#fn::bar::baz(); } + +//- /fn.rs +pub mod bar; + +//- /fn/bar.rs +pub fn baz() {} +"#, + expect![[r#" + SourceChange { + source_file_edits: { + FileId( + 0, + ): TextEdit { + indels: [ + Indel { + insert: "foo", + delete: 4..8, + }, + ], + }, + }, + file_system_edits: [ + MoveFile { + src: FileId( + 1, + ), + dst: AnchoredPathBuf { + anchor: FileId( + 1, + ), + path: "foo.rs", + }, + }, + MoveDir { + src: AnchoredPathBuf { + anchor: FileId( + 1, + ), + path: "fn", + }, + src_id: FileId( + 1, + ), + dst: AnchoredPathBuf { + anchor: FileId( + 1, + ), + path: "foo", + }, + }, + ], + is_snippet: false, + } + "#]], + ); + } + + #[test] fn test_enum_variant_from_module_1() { cov_mark::check!(rename_non_local); check( diff --git a/crates/syntax/src/ast/make.rs b/crates/syntax/src/ast/make.rs index 78ed2a73e5..5aebe4cd9f 100644 --- a/crates/syntax/src/ast/make.rs +++ b/crates/syntax/src/ast/make.rs @@ -12,7 +12,7 @@ use itertools::Itertools; use stdx::{format_to, never}; -use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxToken}; +use crate::{ast, utils::is_raw_identifier, AstNode, SourceFile, SyntaxKind, SyntaxToken}; /// While the parent module defines basic atomic "constructors", the `ext` /// module defines shortcuts for common things. @@ -111,8 +111,7 @@ pub fn name_ref(name_ref: &str) -> ast::NameRef { ast_from_text(&format!("fn f() {{ {raw_escape}{name_ref}; }}")) } fn raw_ident_esc(ident: &str) -> &'static str { - let is_keyword = parser::SyntaxKind::from_keyword(ident).is_some(); - if is_keyword && !matches!(ident, "self" | "crate" | "super" | "Self") { + if is_raw_identifier(ident) { "r#" } else { "" diff --git a/crates/syntax/src/utils.rs b/crates/syntax/src/utils.rs index f4c02518b4..25f34ea9d3 100644 --- a/crates/syntax/src/utils.rs +++ b/crates/syntax/src/utils.rs @@ -2,7 +2,7 @@ use itertools::Itertools; -use crate::{ast, match_ast, AstNode}; +use crate::{ast, match_ast, AstNode, SyntaxKind}; pub fn path_to_string_stripping_turbo_fish(path: &ast::Path) -> String { path.syntax() @@ -23,6 +23,11 @@ pub fn path_to_string_stripping_turbo_fish(path: &ast::Path) -> String { .join("::") } +pub fn is_raw_identifier(name: &str) -> bool { + let is_keyword = SyntaxKind::from_keyword(name).is_some(); + is_keyword && !matches!(name, "self" | "crate" | "super" | "Self") +} + #[cfg(test)] mod tests { use super::path_to_string_stripping_turbo_fish; |