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.

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 {

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)

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)
 }
@@ -558,6 +562,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(
@@ -1286,6 +1299,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);

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_raw_identifier(ident) {
-    if is_keyword && !matches!(ident, "self" | "crate" | "super" | "Self") {
         "r#"
     } else {
         ""

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;