Merge #6295

6295: More type safety around names r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/assists/src/utils.rs

@@ -406,7 +406,7 @@ pub use prelude::*;
         let std_crate = path.next()?;
         let std_crate = if self
             .1
-            .declaration_name(db)
+            .display_name(db)
             .map(|name| name.to_string() == std_crate)
             .unwrap_or(false)
         {

crates/base_db/src/fixture.rs

@@ -158,7 +158,7 @@ impl ChangeFixture {
                 let crate_id = crate_graph.add_crate_root(
                     file_id,
                     meta.edition,
-                    Some(crate_name.clone()),
+                    Some(crate_name.clone().into()),
                     meta.cfg,
                     meta.env,
                     Default::default(),
@@ -187,7 +187,7 @@ impl ChangeFixture {
             crate_graph.add_crate_root(
                 crate_root,
                 Edition::Edition2018,
-                Some(CrateName::new("test").unwrap()),
+                Some(CrateName::new("test").unwrap().into()),
                 default_cfg,
                 Env::default(),
                 Default::default(),

crates/base_db/src/input.rs

@@ -102,11 +102,46 @@ impl fmt::Display for CrateName {
 
 impl ops::Deref for CrateName {
     type Target = str;
-    fn deref(&self) -> &Self::Target {
+    fn deref(&self) -> &str {
         &*self.0
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct CrateDisplayName {
+    // The name we use to display various paths (with `_`).
+    crate_name: CrateName,
+    // The name as specified in Cargo.toml (with `-`).
+    canonical_name: String,
+}
+
+impl From<CrateName> for CrateDisplayName {
+    fn from(crate_name: CrateName) -> CrateDisplayName {
+        let canonical_name = crate_name.to_string();
+        CrateDisplayName { crate_name, canonical_name }
+    }
+}
+
+impl fmt::Display for CrateDisplayName {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.crate_name)
+    }
+}
+
+impl ops::Deref for CrateDisplayName {
+    type Target = str;
+    fn deref(&self) -> &str {
+        &*self.crate_name
+    }
+}
+
+impl CrateDisplayName {
+    pub fn from_canonical_name(canonical_name: String) -> CrateDisplayName {
+        let crate_name = CrateName::normalize_dashes(&canonical_name);
+        CrateDisplayName { crate_name, canonical_name }
+    }
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct ProcMacroId(pub u32);
 
@@ -127,11 +162,13 @@ impl PartialEq for ProcMacro {
 pub struct CrateData {
     pub root_file_id: FileId,
     pub edition: Edition,
-    /// A name used in the package's project declaration: for Cargo projects, it's [package].name,
+    /// A name used in the package's project declaration: for Cargo projects,
-    /// can be different for other project types or even absent (a dummy crate for the code snippet, for example).
+    /// it's [package].name, can be different for other project types or even
-    /// NOTE: The crate can be referenced as a dependency under a different name,
+    /// absent (a dummy crate for the code snippet, for example).
-    /// this one should be used when working with crate hierarchies.
+    ///
-    pub declaration_name: Option<CrateName>,
+    /// For purposes of analysis, crates are anonymous (only names in
+    /// `Dependency` matters), this name should only be used for UI.
+    pub display_name: Option<CrateDisplayName>,
     pub cfg_options: CfgOptions,
     pub env: Env,
     pub dependencies: Vec<Dependency>,
@@ -160,7 +197,7 @@ impl CrateGraph {
         &mut self,
         file_id: FileId,
         edition: Edition,
-        declaration_name: Option<CrateName>,
+        display_name: Option<CrateDisplayName>,
         cfg_options: CfgOptions,
         env: Env,
         proc_macro: Vec<(SmolStr, Arc<dyn tt::TokenExpander>)>,
@@ -171,7 +208,7 @@ impl CrateGraph {
         let data = CrateData {
             root_file_id: file_id,
             edition,
-            declaration_name,
+            display_name,
             cfg_options,
             env,
             proc_macro,
@@ -310,8 +347,8 @@ impl CrateGraph {
         }
     }
 
-    fn hacky_find_crate(&self, declaration_name: &str) -> Option<CrateId> {
+    fn hacky_find_crate(&self, display_name: &str) -> Option<CrateId> {
-        self.iter().find(|it| self[*it].declaration_name.as_deref() == Some(declaration_name))
+        self.iter().find(|it| self[*it].display_name.as_deref() == Some(display_name))
     }
 }
 

crates/base_db/src/lib.rs

@@ -13,8 +13,8 @@ pub use crate::{
     cancellation::Canceled,
     change::Change,
     input::{
-        CrateData, CrateGraph, CrateId, CrateName, Dependency, Edition, Env, FileId, ProcMacroId,
+        CrateData, CrateDisplayName, CrateGraph, CrateId, CrateName, Dependency, Edition, Env,
-        SourceRoot, SourceRootId,
+        FileId, ProcMacroId, SourceRoot, SourceRootId,
     },
 };
 pub use salsa;

crates/hir/src/code_model.rs

@@ -2,7 +2,7 @@
 use std::{iter, sync::Arc};
 
 use arrayvec::ArrayVec;
-use base_db::{CrateId, CrateName, Edition, FileId};
+use base_db::{CrateDisplayName, CrateId, Edition, FileId};
 use either::Either;
 use hir_def::find_path::PrefixKind;
 use hir_def::{
@@ -103,8 +103,8 @@ impl Crate {
         db.crate_graph()[self.id].edition
     }
 
-    pub fn declaration_name(self, db: &dyn HirDatabase) -> Option<CrateName> {
+    pub fn display_name(self, db: &dyn HirDatabase) -> Option<CrateDisplayName> {
-        db.crate_graph()[self.id].declaration_name.clone()
+        db.crate_graph()[self.id].display_name.clone()
     }
 
     pub fn query_external_importables(

crates/hir_def/src/import_map.rs

@@ -356,7 +356,7 @@ mod tests {
         let krate = crate_graph
             .iter()
             .find(|krate| {
-                crate_graph[*krate].declaration_name.as_ref().map(|n| n.to_string())
+                crate_graph[*krate].display_name.as_ref().map(|n| n.to_string())
                     == Some(crate_name.to_string())
             })
             .unwrap();
@@ -375,7 +375,7 @@ mod tests {
                     let path = map.path_of(item).unwrap();
                     format!(
                         "{}::{} ({})\n",
-                        crate_graph[krate].declaration_name.as_ref().unwrap(),
+                        crate_graph[krate].display_name.as_ref().unwrap(),
                         path,
                         mark
                     )
@@ -416,7 +416,7 @@ mod tests {
             .iter()
             .filter_map(|krate| {
                 let cdata = &crate_graph[krate];
-                let name = cdata.declaration_name.as_ref()?;
+                let name = cdata.display_name.as_ref()?;
 
                 let map = db.import_map(krate);
 

crates/hir_def/src/nameres.rs

@@ -172,11 +172,7 @@ pub struct ModuleData {
 impl CrateDefMap {
     pub(crate) fn crate_def_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc<CrateDefMap> {
         let _p = profile::span("crate_def_map_query").detail(|| {
-            db.crate_graph()[krate]
+            db.crate_graph()[krate].display_name.as_deref().unwrap_or_default().to_string()
-                .declaration_name
-                .as_ref()
-                .map(ToString::to_string)
-                .unwrap_or_default()
         });
         let def_map = {
             let edition = db.crate_graph()[krate].edition;

crates/ide/src/doc_links.rs

@@ -130,7 +130,7 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option<String> {
     let module = definition.module(db)?;
     let krate = module.krate();
     let import_map = db.import_map(krate.into());
-    let base = once(krate.declaration_name(db)?.to_string())
+    let base = once(krate.display_name(db)?.to_string())
         .chain(import_map.path_of(ns)?.segments.iter().map(|name| name.to_string()))
         .join("/");
 
@@ -188,7 +188,7 @@ fn rewrite_intra_doc_link(
     let krate = resolved.module(db)?.krate();
     let canonical_path = resolved.canonical_path(db)?;
     let new_target = get_doc_url(db, &krate)?
-        .join(&format!("{}/", krate.declaration_name(db)?))
+        .join(&format!("{}/", krate.display_name(db)?))
         .ok()?
         .join(&canonical_path.replace("::", "/"))
         .ok()?
@@ -208,7 +208,7 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option<S
     let module = def.module(db)?;
     let krate = module.krate();
     let canonical_path = def.canonical_path(db)?;
-    let base = format!("{}/{}", krate.declaration_name(db)?, canonical_path.replace("::", "/"));
+    let base = format!("{}/{}", krate.display_name(db)?, canonical_path.replace("::", "/"));
 
     get_doc_url(db, &krate)
         .and_then(|url| url.join(&base).ok())
@@ -357,7 +357,7 @@ fn get_doc_url(db: &RootDatabase, krate: &Crate) -> Option<Url> {
             //
             // FIXME: clicking on the link should just open the file in the editor,
             // instead of falling back to external urls.
-            Some(format!("https://docs.rs/{}/*/", krate.declaration_name(db)?))
+            Some(format!("https://docs.rs/{}/*/", krate.display_name(db)?))
         })
         .and_then(|s| Url::parse(&s).ok())
 }

crates/ide/src/hover.rs

@@ -300,7 +300,7 @@ fn definition_owner_name(db: &RootDatabase, def: &Definition) -> Option<String>
 
 fn render_path(db: &RootDatabase, module: Module, item_name: Option<String>) -> String {
     let crate_name =
-        db.crate_graph()[module.krate().into()].declaration_name.as_ref().map(ToString::to_string);
+        db.crate_graph()[module.krate().into()].display_name.as_ref().map(|it| it.to_string());
     let module_path = module
         .path_to_root(db)
         .into_iter()

crates/ide/src/inlay_hints.rs

@@ -1,15 +1,14 @@
 use assists::utils::FamousDefs;
+use either::Either;
 use hir::{known, HirDisplay, Semantics};
 use ide_db::RootDatabase;
 use stdx::to_lower_snake_case;
 use syntax::{
-    ast::{self, ArgListOwner, AstNode},
+    ast::{self, ArgListOwner, AstNode, NameOwner},
     match_ast, Direction, NodeOrToken, SmolStr, SyntaxKind, TextRange, T,
 };
 
 use crate::FileId;
-use ast::NameOwner;
-use either::Either;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct InlayHintsConfig {
@@ -215,7 +214,7 @@ fn hint_iterator(
         .last()
         .and_then(|strukt| strukt.as_adt())?;
     let krate = strukt.krate(db)?;
-    if krate.declaration_name(db).as_deref() != Some("core") {
+    if krate.display_name(db).as_deref() != Some("core") {
         return None;
     }
     let iter_trait = FamousDefs(sema, krate).core_iter_Iterator()?;

crates/ide/src/prime_caches.rs

@@ -32,8 +32,7 @@ pub(crate) fn prime_caches(db: &RootDatabase, cb: &(dyn Fn(PrimeCachesProgress)
     // Unfortunately rayon prevents panics from propagation out of a `scope`, which breaks
     // cancellation, so we cannot use rayon.
     for (i, krate) in topo.iter().enumerate() {
-        let crate_name =
+        let crate_name = graph[*krate].display_name.as_deref().unwrap_or_default().to_string();
-            graph[*krate].declaration_name.as_ref().map(ToString::to_string).unwrap_or_default();
 
         cb(PrimeCachesProgress::StartedOnCrate {
             on_crate: crate_name,

crates/ide/src/status.rs

@@ -45,7 +45,7 @@ pub(crate) fn status(db: &RootDatabase, file_id: Option<FileId>) -> String {
         match krate {
             Some(krate) => {
                 let crate_graph = db.crate_graph();
-                let display_crate = |krate: CrateId| match &crate_graph[krate].declaration_name {
+                let display_crate = |krate: CrateId| match &crate_graph[krate].display_name {
                     Some(it) => format!("{}({:?})", it, krate),
                     None => format!("{:?}", krate),
                 };

crates/project_model/src/lib.rs

@@ -13,7 +13,7 @@ use std::{
 };
 
 use anyhow::{bail, Context, Result};
-use base_db::{CrateGraph, CrateId, CrateName, Edition, Env, FileId};
+use base_db::{CrateDisplayName, CrateGraph, CrateId, CrateName, Edition, Env, FileId};
 use cfg::CfgOptions;
 use paths::{AbsPath, AbsPathBuf};
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -408,10 +408,12 @@ impl ProjectWorkspace {
                                 .map(|it| proc_macro_client.by_dylib_path(&it))
                                 .unwrap_or_default();
 
+                            let display_name =
+                                CrateDisplayName::from_canonical_name(cargo[pkg].name.clone());
                             let crate_id = crate_graph.add_crate_root(
                                 file_id,
                                 edition,
-                                Some(CrateName::normalize_dashes(&cargo[pkg].name)),
+                                Some(display_name),
                                 cfg_options,
                                 env,
                                 proc_macro.clone(),
@@ -556,7 +558,7 @@ fn sysroot_to_crate_graph(
             let crate_id = crate_graph.add_crate_root(
                 file_id,
                 Edition::Edition2018,
-                Some(name),
+                Some(name.into()),
                 cfg_options.clone(),
                 env,
                 proc_macro,

crates/rust-analyzer/src/cli/diagnostics.rs

@@ -36,12 +36,8 @@ pub fn diagnostics(path: &Path, load_output_dirs: bool, with_proc_macro: bool) -
     for module in work {
         let file_id = module.definition_source(db).file_id.original_file(db);
         if !visited_files.contains(&file_id) {
-            let crate_name = module
+            let crate_name =
-                .krate()
+                module.krate().display_name(db).as_deref().unwrap_or("unknown").to_string();
-                .declaration_name(db)
-                .as_ref()
-                .map(ToString::to_string)
-                .unwrap_or_else(|| "unknown".to_string());
             println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id));
             for diagnostic in analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap()
             {