Auto merge of #12993 - lowr:patch/swap-name-and-escaped-name, r=Veykril

Make `Name` hold escaped name

Resolves #12787
Resolves rust-lang/rust#99361

This PR effectively swaps `Name` and `EscapedName` in hir. In other words, it makes `Name` hold and print escaped raw identifiers and introduces another struct `UnescapedName` for cases where you need to print names without "r#" prefix.

My rationale is that it makes it easier for us to format an escaped name into string, which is what we want when we serialize names in general. This is because we format a name into string usually when we are presenting it to the users and arguably they expect its escaped form as that's what they see and write in the source code.

I split the change for `Name` into 3 commits to make it easier to follow but it also made some tests fail in the intermediate commits. I can squash them into a commit after the review if desired. I've also made similar changes for `ModPath` and `EscapedModPath` as it makes them consistent with `Name`.

For reference, there was a brief discussion on this in [a zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Frust-analyzer/topic/escaping.20.60Name.60s).

crates/hir-def/src/nameres/mod_resolution.rs

@@ -73,10 +73,12 @@ impl ModDir {
                 candidate_files.push(self.dir_path.join_attr(attr_path, self.root_non_dir_owner))
             }
             None if file_id.is_include_macro(db.upcast()) => {
+                let name = name.unescaped();
                 candidate_files.push(format!("{}.rs", name));
                 candidate_files.push(format!("{}/mod.rs", name));
             }
             None => {
+                let name = name.unescaped();
                 candidate_files.push(format!("{}{}.rs", self.dir_path.0, name));
                 candidate_files.push(format!("{}{}/mod.rs", self.dir_path.0, name));
             }

crates/hir-def/src/nameres/tests/mod_resolution.rs

@@ -132,9 +132,9 @@ pub struct Bar;
         expect![[r#"
             crate
             Bar: t v
-            async: t
+            r#async: t
 
-            crate::async
+            crate::r#async
             Bar: t v
         "#]],
     );

crates/hir-expand/src/mod_path.rs

@@ -22,7 +22,7 @@ pub struct ModPath {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct EscapedModPath<'a>(&'a ModPath);
+pub struct UnescapedModPath<'a>(&'a ModPath);
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum PathKind {
@@ -102,8 +102,8 @@ impl ModPath {
         }
     }
 
-    pub fn escaped(&self) -> EscapedModPath<'_> {
+    pub fn unescaped(&self) -> UnescapedModPath<'_> {
-        EscapedModPath(self)
+        UnescapedModPath(self)
     }
 
     fn _fmt(&self, f: &mut fmt::Formatter<'_>, escaped: bool) -> fmt::Result {
@@ -134,9 +134,9 @@ impl ModPath {
             }
             first_segment = false;
             if escaped {
-                segment.escaped().fmt(f)?
-            } else {
                 segment.fmt(f)?
+            } else {
+                segment.unescaped().fmt(f)?
             };
         }
         Ok(())
@@ -145,13 +145,13 @@ impl ModPath {
 
 impl Display for ModPath {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self._fmt(f, false)
+        self._fmt(f, true)
     }
 }
 
-impl<'a> Display for EscapedModPath<'a> {
+impl<'a> Display for UnescapedModPath<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0._fmt(f, true)
+        self.0._fmt(f, false)
     }
 }
 

crates/hir-expand/src/name.rs

@@ -7,12 +7,16 @@ use syntax::{ast, SmolStr, SyntaxKind};
 /// `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
 /// not there yet!
+///
+/// Note that `Name` holds and prints escaped name i.e. prefixed with "r#" when it
+/// is a raw identifier. Use [`unescaped()`][Name::unescaped] when you need the
+/// name without "r#".
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct Name(Repr);
 
-/// `EscapedName` will add a prefix "r#" to the wrapped `Name` when it is a raw identifier
+/// Wrapper of `Name` to print the name without "r#" even when it is a raw identifier.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
-pub struct EscapedName<'a>(&'a Name);
+pub struct UnescapedName<'a>(&'a Name);
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 enum Repr {
@@ -34,37 +38,26 @@ fn is_raw_identifier(name: &str) -> bool {
     is_keyword && !matches!(name, "self" | "crate" | "super" | "Self")
 }
 
-impl<'a> fmt::Display for EscapedName<'a> {
+impl<'a> fmt::Display for UnescapedName<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match &self.0 .0 {
             Repr::Text(text) => {
-                if is_raw_identifier(text) {
+                let text = text.strip_prefix("r#").unwrap_or(text);
-                    write!(f, "r#{}", &text)
+                fmt::Display::fmt(&text, f)
-                } else {
-                    fmt::Display::fmt(&text, f)
-                }
             }
             Repr::TupleField(idx) => fmt::Display::fmt(&idx, f),
         }
     }
 }
 
-impl<'a> EscapedName<'a> {
+impl<'a> UnescapedName<'a> {
-    pub fn is_escaped(&self) -> bool {
+    /// Returns the textual representation of this name as a [`SmolStr`]. Prefer using this over
-        match &self.0 .0 {
+    /// [`ToString::to_string`] if possible as this conversion is cheaper in the general case.
-            Repr::Text(it) => is_raw_identifier(&it),
-            Repr::TupleField(_) => false,
-        }
-    }
-
-    /// Returns the textual representation of this name as a [`SmolStr`].
-    /// Prefer using this over [`ToString::to_string`] if possible as this conversion is cheaper in
-    /// the general case.
     pub fn to_smol_str(&self) -> SmolStr {
         match &self.0 .0 {
             Repr::Text(it) => {
-                if is_raw_identifier(&it) {
+                if let Some(stripped) = it.strip_prefix("r#") {
-                    SmolStr::from_iter(["r#", &it])
+                    SmolStr::new(stripped)
                 } else {
                     it.clone()
                 }
@@ -97,9 +90,11 @@ impl Name {
 
     /// Resolve a name from the text of token.
     fn resolve(raw_text: &str) -> Name {
+        // When `raw_text` starts with "r#" but the name does not coincide with any
+        // keyword, we never need the prefix so we strip it.
         match raw_text.strip_prefix("r#") {
-            Some(text) => Name::new_text(SmolStr::new(text)),
+            Some(text) if !is_raw_identifier(text) => Name::new_text(SmolStr::new(text)),
-            None => Name::new_text(raw_text.into()),
+            _ => Name::new_text(raw_text.into()),
         }
     }
 
@@ -142,8 +137,15 @@ impl Name {
         }
     }
 
-    pub fn escaped(&self) -> EscapedName<'_> {
+    pub fn unescaped(&self) -> UnescapedName<'_> {
-        EscapedName(self)
+        UnescapedName(self)
+    }
+
+    pub fn is_escaped(&self) -> bool {
+        match &self.0 {
+            Repr::Text(it) => it.starts_with("r#"),
+            Repr::TupleField(_) => false,
+        }
     }
 }
 

crates/ide-completion/src/completions/item_list/trait_impl.rs

@@ -233,7 +233,8 @@ fn add_type_alias_impl(
     type_alias: hir::TypeAlias,
 ) {
     let alias_name = type_alias.name(ctx.db);
-    let (alias_name, escaped_name) = (alias_name.to_smol_str(), alias_name.escaped().to_smol_str());
+    let (alias_name, escaped_name) =
+        (alias_name.unescaped().to_smol_str(), alias_name.to_smol_str());
 
     let label = format!("type {} =", alias_name);
     let replacement = format!("type {} = ", escaped_name);

crates/ide-completion/src/render.rs

@@ -117,7 +117,7 @@ pub(crate) fn render_field(
 ) -> CompletionItem {
     let is_deprecated = ctx.is_deprecated(field);
     let name = field.name(ctx.db());
-    let (name, escaped_name) = (name.to_smol_str(), name.escaped().to_smol_str());
+    let (name, escaped_name) = (name.unescaped().to_smol_str(), name.to_smol_str());
     let mut item = CompletionItem::new(
         SymbolKind::Field,
         ctx.source_range(),
@@ -283,8 +283,8 @@ fn render_resolution_path(
 
     let name = local_name.to_smol_str();
     let mut item = render_resolution_simple_(ctx, &local_name, import_to_add, resolution);
-    if local_name.escaped().is_escaped() {
+    if local_name.is_escaped() {
-        item.insert_text(local_name.escaped().to_smol_str());
+        item.insert_text(local_name.to_smol_str());
     }
     // Add `<>` for generic types
     let type_path_no_ty_args = matches!(
@@ -306,7 +306,7 @@ fn render_resolution_path(
                 item.lookup_by(name.clone())
                     .label(SmolStr::from_iter([&name, "<…>"]))
                     .trigger_call_info()
-                    .insert_snippet(cap, format!("{}<$0>", local_name.escaped()));
+                    .insert_snippet(cap, format!("{}<$0>", local_name));
             }
         }
     }
@@ -342,7 +342,8 @@ fn render_resolution_simple_(
     let ctx = ctx.import_to_add(import_to_add);
     let kind = res_to_kind(resolution);
 
-    let mut item = CompletionItem::new(kind, ctx.source_range(), local_name.to_smol_str());
+    let mut item =
+        CompletionItem::new(kind, ctx.source_range(), local_name.unescaped().to_smol_str());
     item.set_relevance(ctx.completion_relevance())
         .set_documentation(scope_def_docs(db, resolution))
         .set_deprecated(scope_def_is_deprecated(&ctx, resolution));

crates/ide-completion/src/render/const_.rs

@@ -13,7 +13,7 @@ pub(crate) fn render_const(ctx: RenderContext<'_>, const_: hir::Const) -> Option
 fn render(ctx: RenderContext<'_>, const_: hir::Const) -> Option<CompletionItem> {
     let db = ctx.db();
     let name = const_.name(db)?;
-    let (name, escaped_name) = (name.to_smol_str(), name.escaped().to_smol_str());
+    let (name, escaped_name) = (name.unescaped().to_smol_str(), name.to_smol_str());
     let detail = const_.display(db).to_string();
 
     let mut item = CompletionItem::new(SymbolKind::Const, ctx.source_range(), name.clone());

crates/ide-completion/src/render/function.rs

@@ -52,10 +52,10 @@ fn render(
 
     let (call, escaped_call) = match &func_kind {
         FuncKind::Method(_, Some(receiver)) => (
-            format!("{}.{}", receiver, &name).into(),
+            format!("{}.{}", receiver.unescaped(), name.unescaped()).into(),
-            format!("{}.{}", receiver.escaped(), name.escaped()).into(),
+            format!("{}.{}", receiver, name).into(),
         ),
-        _ => (name.to_smol_str(), name.escaped().to_smol_str()),
+        _ => (name.unescaped().to_smol_str(), name.to_smol_str()),
     };
     let mut item = CompletionItem::new(
         if func.self_param(db).is_some() {
@@ -96,7 +96,7 @@ fn render(
     item.set_documentation(ctx.docs(func))
         .set_deprecated(ctx.is_deprecated(func) || ctx.is_deprecated_assoc_item(func))
         .detail(detail(db, func))
-        .lookup_by(name.to_smol_str());
+        .lookup_by(name.unescaped().to_smol_str());
 
     match ctx.completion.config.snippet_cap {
         Some(cap) => {

crates/ide-completion/src/render/literal.rs

@@ -73,7 +73,7 @@ fn render(
         None => (name.clone().into(), name.into(), false),
     };
     let (qualified_name, escaped_qualified_name) =
-        (qualified_name.to_string(), qualified_name.escaped().to_string());
+        (qualified_name.unescaped().to_string(), qualified_name.to_string());
     let snippet_cap = ctx.snippet_cap();
 
     let mut rendered = match kind {

crates/ide-completion/src/render/macro_.rs

@@ -46,7 +46,7 @@ fn render(
         ctx.source_range()
     };
 
-    let (name, escaped_name) = (name.to_smol_str(), name.escaped().to_smol_str());
+    let (name, escaped_name) = (name.unescaped().to_smol_str(), name.to_smol_str());
     let docs = ctx.docs(macro_);
     let docs_str = docs.as_ref().map(Documentation::as_str).unwrap_or_default();
     let is_fn_like = macro_.is_fn_like(completion.db);

crates/ide-completion/src/render/pattern.rs

@@ -31,7 +31,7 @@ pub(crate) fn render_struct_pat(
     }
 
     let name = local_name.unwrap_or_else(|| strukt.name(ctx.db()));
-    let (name, escaped_name) = (name.to_smol_str(), name.escaped().to_smol_str());
+    let (name, escaped_name) = (name.unescaped().to_smol_str(), name.to_smol_str());
     let kind = strukt.kind(ctx.db());
     let label = format_literal_label(name.as_str(), kind);
     let pat = render_pat(&ctx, pattern_ctx, &escaped_name, kind, &visible_fields, fields_omitted)?;
@@ -53,10 +53,10 @@ pub(crate) fn render_variant_pat(
     let (visible_fields, fields_omitted) = visible_fields(ctx.completion, &fields, variant)?;
 
     let (name, escaped_name) = match path {
-        Some(path) => (path.to_string().into(), path.escaped().to_string().into()),
+        Some(path) => (path.unescaped().to_string().into(), path.to_string().into()),
         None => {
             let name = local_name.unwrap_or_else(|| variant.name(ctx.db()));
-            (name.to_smol_str(), name.escaped().to_smol_str())
+            (name.unescaped().to_smol_str(), name.to_smol_str())
         }
     };
 
@@ -146,7 +146,7 @@ fn render_record_as_pat(
             format!(
                 "{name} {{ {}{} }}",
                 fields.enumerate().format_with(", ", |(idx, field), f| {
-                    f(&format_args!("{}${}", field.name(db).escaped(), idx + 1))
+                    f(&format_args!("{}${}", field.name(db), idx + 1))
                 }),
                 if fields_omitted { ", .." } else { "" },
                 name = name
@@ -155,7 +155,7 @@ fn render_record_as_pat(
         None => {
             format!(
                 "{name} {{ {}{} }}",
-                fields.map(|field| field.name(db).escaped().to_smol_str()).format(", "),
+                fields.map(|field| field.name(db).to_smol_str()).format(", "),
                 if fields_omitted { ", .." } else { "" },
                 name = name
             )

crates/ide-completion/src/render/type_alias.rs

@@ -32,11 +32,11 @@ fn render(
     let name = type_alias.name(db);
     let (name, escaped_name) = if with_eq {
         (
+            SmolStr::from_iter([&name.unescaped().to_smol_str(), " = "]),
             SmolStr::from_iter([&name.to_smol_str(), " = "]),
-            SmolStr::from_iter([&name.escaped().to_smol_str(), " = "]),
         )
     } else {
-        (name.to_smol_str(), name.escaped().to_smol_str())
+        (name.unescaped().to_smol_str(), name.to_smol_str())
     };
     let detail = type_alias.display(db).to_string();
 

crates/ide-completion/src/render/union_literal.rs

@@ -21,8 +21,8 @@ pub(crate) fn render_union_literal(
     let name = local_name.unwrap_or_else(|| un.name(ctx.db()));
 
     let (qualified_name, escaped_qualified_name) = match path {
-        Some(p) => (p.to_string(), p.escaped().to_string()),
+        Some(p) => (p.unescaped().to_string(), p.to_string()),
-        None => (name.to_string(), name.escaped().to_string()),
+        None => (name.unescaped().to_string(), name.to_string()),
     };
 
     let mut item = CompletionItem::new(
@@ -42,15 +42,15 @@ pub(crate) fn render_union_literal(
         format!(
             "{} {{ ${{1|{}|}}: ${{2:()}} }}$0",
             escaped_qualified_name,
-            fields.iter().map(|field| field.name(ctx.db()).escaped().to_smol_str()).format(",")
+            fields.iter().map(|field| field.name(ctx.db()).to_smol_str()).format(",")
         )
     } else {
         format!(
             "{} {{ {} }}",
             escaped_qualified_name,
-            fields.iter().format_with(", ", |field, f| {
+            fields
-                f(&format_args!("{}: ()", field.name(ctx.db()).escaped()))
+                .iter()
-            })
+                .format_with(", ", |field, f| { f(&format_args!("{}: ()", field.name(ctx.db()))) })
         )
     };
 

crates/ide-completion/src/render/variant.rs

@@ -24,9 +24,9 @@ pub(crate) fn render_record_lit(
 ) -> RenderedLiteral {
     let completions = fields.iter().enumerate().format_with(", ", |(idx, field), f| {
         if snippet_cap.is_some() {
-            f(&format_args!("{}: ${{{}:()}}", field.name(db).escaped(), idx + 1))
+            f(&format_args!("{}: ${{{}:()}}", field.name(db), idx + 1))
         } else {
-            f(&format_args!("{}: ()", field.name(db).escaped()))
+            f(&format_args!("{}: ()", field.name(db)))
         }
     });
 

crates/ide/src/runnables.rs

@@ -2225,4 +2225,190 @@ macro_rules! foo {
             "#]],
         );
     }
+
+    #[test]
+    fn test_paths_with_raw_ident() {
+        check(
+            r#"
+//- /lib.rs
+$0
+mod r#mod {
+    #[test]
+    fn r#fn() {}
+
+    /// ```
+    /// ```
+    fn r#for() {}
+
+    /// ```
+    /// ```
+    struct r#struct<r#type>(r#type);
+
+    /// ```
+    /// ```
+    impl<r#type> r#struct<r#type> {
+        /// ```
+        /// ```
+        fn r#fn() {}
+    }
+
+    enum r#enum {}
+    impl r#struct<r#enum> {
+        /// ```
+        /// ```
+        fn r#fn() {}
+    }
+
+    trait r#trait {}
+
+    /// ```
+    /// ```
+    impl<T> r#trait for r#struct<T> {}
+}
+"#,
+            &[TestMod, Test, DocTest, DocTest, DocTest, DocTest, DocTest, DocTest],
+            expect![[r#"
+                [
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 1..461,
+                            focus_range: 5..10,
+                            name: "r#mod",
+                            kind: Module,
+                            description: "mod r#mod",
+                        },
+                        kind: TestMod {
+                            path: "r#mod",
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 17..41,
+                            focus_range: 32..36,
+                            name: "r#fn",
+                            kind: Function,
+                        },
+                        kind: Test {
+                            test_id: Path(
+                                "r#mod::r#fn",
+                            ),
+                            attr: TestAttr {
+                                ignore: false,
+                            },
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 47..84,
+                            name: "r#for",
+                        },
+                        kind: DocTest {
+                            test_id: Path(
+                                "r#mod::r#for",
+                            ),
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 90..146,
+                            name: "r#struct",
+                        },
+                        kind: DocTest {
+                            test_id: Path(
+                                "r#mod::r#struct",
+                            ),
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 152..266,
+                            focus_range: 189..205,
+                            name: "impl",
+                            kind: Impl,
+                        },
+                        kind: DocTest {
+                            test_id: Path(
+                                "r#struct<r#type>",
+                            ),
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 216..260,
+                            name: "r#fn",
+                        },
+                        kind: DocTest {
+                            test_id: Path(
+                                "r#mod::r#struct<r#type>::r#fn",
+                            ),
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 323..367,
+                            name: "r#fn",
+                        },
+                        kind: DocTest {
+                            test_id: Path(
+                                "r#mod::r#struct<r#enum>::r#fn",
+                            ),
+                        },
+                        cfg: None,
+                    },
+                    Runnable {
+                        use_name_in_title: false,
+                        nav: NavigationTarget {
+                            file_id: FileId(
+                                0,
+                            ),
+                            full_range: 401..459,
+                            focus_range: 445..456,
+                            name: "impl",
+                            kind: Impl,
+                        },
+                        kind: DocTest {
+                            test_id: Path(
+                                "r#struct<T>",
+                            ),
+                        },
+                        cfg: None,
+                    },
+                ]
+            "#]],
+        )
+    }
 }