Skip to content

Commit c97aae3

Browse files
borsbors
committed
Auto merge of rust-lang#14138 - lowr:fix/rename-raw-ident-mod, r=Veykril
fix: don't include `r#` prefix in filesystem changes Fixes rust-lang#14131 In addition to fix for rust-lang#14131, this PR adds raw ident validity checks in rename functionality that we've been missing.
2 parents 23871f9 + 57f0e9c commit c97aae3

File tree

5 files changed

+176
-15
lines changed

5 files changed

+176
-15
lines changed

‎crates/hir-expand/src/name.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use std::fmt;
44

5-
use syntax::{ast, SmolStr, SyntaxKind};
5+
use syntax::{ast, utils::is_raw_identifier, SmolStr};
66

77
/// `Name` is a wrapper around string, which is used in hir for both references
88
/// and declarations. In theory, names should also carry hygiene info, but we are
@@ -33,11 +33,6 @@ impl fmt::Display for Name {
3333
}
3434
}
3535

36-
fn is_raw_identifier(name: &str) -> bool {
37-
let is_keyword = SyntaxKind::from_keyword(name).is_some();
38-
is_keyword && !matches!(name, "self" | "crate" | "super" | "Self")
39-
}
40-
4136
impl<'a> fmt::Display for UnescapedName<'a> {
4237
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
4338
match &self.0 .0 {

‎crates/ide-db/src/rename.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ fn rename_mod(
190190

191191
let InFile { file_id, value: def_source } = module.definition_source(sema.db);
192192
if let ModuleSource::SourceFile(..) = def_source {
193+
let new_name = new_name.trim_start_matches("r#");
193194
let anchor = file_id.original_file(sema.db);
194195

195196
let is_mod_rs = module.is_mod_rs(sema.db);
@@ -207,9 +208,13 @@ fn rename_mod(
207208
// - Module has submodules defined in separate files
208209
let dir_paths = match (is_mod_rs, has_detached_child, module.name(sema.db)) {
209210
// Go up one level since the anchor is inside the dir we're trying to rename
210-
(true, _, Some(mod_name)) => Some((format!("../{mod_name}"), format!("../{new_name}"))),
211+
(true, _, Some(mod_name)) => {
212+
Some((format!("../{}", mod_name.unescaped()), format!("../{new_name}")))
213+
}
211214
// The anchor is on the same level as target dir
212-
(false, true, Some(mod_name)) => Some((mod_name.to_string(), new_name.to_string())),
215+
(false, true, Some(mod_name)) => {
216+
Some((mod_name.unescaped().to_string(), new_name.to_string()))
217+
}
213218
_ => None,
214219
};
215220

@@ -532,7 +537,14 @@ impl IdentifierKind {
532537
pub fn classify(new_name: &str) -> Result<IdentifierKind> {
533538
match parser::LexedStr::single_token(new_name) {
534539
Some(res) => match res {
535-
(SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident),
540+
(SyntaxKind::IDENT, _) => {
541+
if let Some(inner) = new_name.strip_prefix("r#") {
542+
if matches!(inner, "self" | "crate" | "super" | "Self") {
543+
bail!("Invalid name: `{}` cannot be a raw identifier", inner);
544+
}
545+
}
546+
Ok(IdentifierKind::Ident)
547+
}
536548
(T![_], _) => Ok(IdentifierKind::Underscore),
537549
(SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => {
538550
Ok(IdentifierKind::Lifetime)

‎crates/ide/src/rename.rs

+152-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use ide_db::{
1313
};
1414
use itertools::Itertools;
1515
use stdx::{always, never};
16-
use syntax::{ast, AstNode, SyntaxNode, TextRange, TextSize};
16+
use syntax::{ast, utils::is_raw_identifier, AstNode, SmolStr, SyntaxNode, TextRange, TextSize};
1717

1818
use text_edit::TextEdit;
1919

@@ -122,7 +122,11 @@ pub(crate) fn will_rename_file(
122122
let sema = Semantics::new(db);
123123
let module = sema.to_module_def(file_id)?;
124124
let def = Definition::Module(module);
125-
let mut change = def.rename(&sema, new_name_stem).ok()?;
125+
let mut change = if is_raw_identifier(new_name_stem) {
126+
def.rename(&sema, &SmolStr::from_iter(["r#", new_name_stem])).ok()?
127+
} else {
128+
def.rename(&sema, new_name_stem).ok()?
129+
};
126130
change.file_system_edits.clear();
127131
Some(change)
128132
}
@@ -558,6 +562,15 @@ impl Foo {
558562
);
559563
}
560564

565+
#[test]
566+
fn test_rename_mod_invalid_raw_ident() {
567+
check(
568+
"r#self",
569+
r#"mod foo$0 {}"#,
570+
"error: Invalid name: `self` cannot be a raw identifier",
571+
);
572+
}
573+
561574
#[test]
562575
fn test_rename_for_local() {
563576
check(
@@ -1286,6 +1299,143 @@ mod bar$0;
12861299
)
12871300
}
12881301

1302+
#[test]
1303+
fn test_rename_mod_to_raw_ident() {
1304+
check_expect(
1305+
"r#fn",
1306+
r#"
1307+
//- /lib.rs
1308+
mod foo$0;
1309+
1310+
fn main() { foo::bar::baz(); }
1311+
1312+
//- /foo.rs
1313+
pub mod bar;
1314+
1315+
//- /foo/bar.rs
1316+
pub fn baz() {}
1317+
"#,
1318+
expect![[r#"
1319+
SourceChange {
1320+
source_file_edits: {
1321+
FileId(
1322+
0,
1323+
): TextEdit {
1324+
indels: [
1325+
Indel {
1326+
insert: "r#fn",
1327+
delete: 4..7,
1328+
},
1329+
Indel {
1330+
insert: "r#fn",
1331+
delete: 22..25,
1332+
},
1333+
],
1334+
},
1335+
},
1336+
file_system_edits: [
1337+
MoveFile {
1338+
src: FileId(
1339+
1,
1340+
),
1341+
dst: AnchoredPathBuf {
1342+
anchor: FileId(
1343+
1,
1344+
),
1345+
path: "fn.rs",
1346+
},
1347+
},
1348+
MoveDir {
1349+
src: AnchoredPathBuf {
1350+
anchor: FileId(
1351+
1,
1352+
),
1353+
path: "foo",
1354+
},
1355+
src_id: FileId(
1356+
1,
1357+
),
1358+
dst: AnchoredPathBuf {
1359+
anchor: FileId(
1360+
1,
1361+
),
1362+
path: "fn",
1363+
},
1364+
},
1365+
],
1366+
is_snippet: false,
1367+
}
1368+
"#]],
1369+
);
1370+
}
1371+
1372+
#[test]
1373+
fn test_rename_mod_from_raw_ident() {
1374+
// FIXME: `r#fn` in path expression is not renamed.
1375+
check_expect(
1376+
"foo",
1377+
r#"
1378+
//- /lib.rs
1379+
mod r#fn$0;
1380+
1381+
fn main() { r#fn::bar::baz(); }
1382+
1383+
//- /fn.rs
1384+
pub mod bar;
1385+
1386+
//- /fn/bar.rs
1387+
pub fn baz() {}
1388+
"#,
1389+
expect![[r#"
1390+
SourceChange {
1391+
source_file_edits: {
1392+
FileId(
1393+
0,
1394+
): TextEdit {
1395+
indels: [
1396+
Indel {
1397+
insert: "foo",
1398+
delete: 4..8,
1399+
},
1400+
],
1401+
},
1402+
},
1403+
file_system_edits: [
1404+
MoveFile {
1405+
src: FileId(
1406+
1,
1407+
),
1408+
dst: AnchoredPathBuf {
1409+
anchor: FileId(
1410+
1,
1411+
),
1412+
path: "foo.rs",
1413+
},
1414+
},
1415+
MoveDir {
1416+
src: AnchoredPathBuf {
1417+
anchor: FileId(
1418+
1,
1419+
),
1420+
path: "fn",
1421+
},
1422+
src_id: FileId(
1423+
1,
1424+
),
1425+
dst: AnchoredPathBuf {
1426+
anchor: FileId(
1427+
1,
1428+
),
1429+
path: "foo",
1430+
},
1431+
},
1432+
],
1433+
is_snippet: false,
1434+
}
1435+
"#]],
1436+
);
1437+
}
1438+
12891439
#[test]
12901440
fn test_enum_variant_from_module_1() {
12911441
cov_mark::check!(rename_non_local);

‎crates/syntax/src/ast/make.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
use itertools::Itertools;
1313
use stdx::{format_to, never};
1414

15-
use crate::{ast, AstNode, SourceFile, SyntaxKind, SyntaxToken};
15+
use crate::{ast, utils::is_raw_identifier, AstNode, SourceFile, SyntaxKind, SyntaxToken};
1616

1717
/// While the parent module defines basic atomic "constructors", the `ext`
1818
/// module defines shortcuts for common things.
@@ -111,8 +111,7 @@ pub fn name_ref(name_ref: &str) -> ast::NameRef {
111111
ast_from_text(&format!("fn f() {{ {raw_escape}{name_ref}; }}"))
112112
}
113113
fn raw_ident_esc(ident: &str) -> &'static str {
114-
let is_keyword = parser::SyntaxKind::from_keyword(ident).is_some();
115-
if is_keyword && !matches!(ident, "self" | "crate" | "super" | "Self") {
114+
if is_raw_identifier(ident) {
116115
"r#"
117116
} else {
118117
""

‎crates/syntax/src/utils.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use itertools::Itertools;
44

5-
use crate::{ast, match_ast, AstNode};
5+
use crate::{ast, match_ast, AstNode, SyntaxKind};
66

77
pub fn path_to_string_stripping_turbo_fish(path: &ast::Path) -> String {
88
path.syntax()
@@ -23,6 +23,11 @@ pub fn path_to_string_stripping_turbo_fish(path: &ast::Path) -> String {
2323
.join("::")
2424
}
2525

26+
pub fn is_raw_identifier(name: &str) -> bool {
27+
let is_keyword = SyntaxKind::from_keyword(name).is_some();
28+
is_keyword && !matches!(name, "self" | "crate" | "super" | "Self")
29+
}
30+
2631
#[cfg(test)]
2732
mod tests {
2833
use super::path_to_string_stripping_turbo_fish;

0 commit comments

Comments
 (0)