Merge #6124

6124: Better normalized crate name usage r=jonas-schievink a=SomeoneToIgnore

Closes https://github.com/rust-analyzer/rust-analyzer/issues/5343 
Closes https://github.com/rust-analyzer/rust-analyzer/issues/5932

Uses normalized name for code snippets (to be able to test the fix), hover messages and documentation rewrite links (are there any tests for those?).
Also renamed the field to better resemble the semantics.

Co-authored-by: Kirill Bulatov <[email protected]>

10 files changed, 64 insertions, 31 deletions

diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs
index b7286fc7d..72f1fd667 100644
--- a/crates/base_db/src/fixture.rs
+++ b/crates/base_db/src/fixture.rs
@@ -154,19 +154,19 @@ impl ChangeFixture {
             assert!(meta.path.starts_with(&source_root_prefix));
 
             if let Some(krate) = meta.krate {
+                let crate_name = CrateName::normalize_dashes(&krate);
                 let crate_id = crate_graph.add_crate_root(
                     file_id,
                     meta.edition,
-                    Some(krate.clone()),
+                    Some(crate_name.clone()),
                     meta.cfg,
                     meta.env,
                     Default::default(),
                 );
-                let crate_name = CrateName::new(&krate).unwrap();
                 let prev = crates.insert(crate_name.clone(), crate_id);
                 assert!(prev.is_none());
                 for dep in meta.deps {
-                    let dep = CrateName::new(&dep).unwrap();
+                    let dep = CrateName::normalize_dashes(&dep);
                     crate_deps.push((crate_name.clone(), dep))
                 }
             } else if meta.path == "/main.rs" || meta.path == "/lib.rs" {
@@ -187,7 +187,7 @@ impl ChangeFixture {
             crate_graph.add_crate_root(
                 crate_root,
                 Edition::Edition2018,
-                Some("test".to_string()),
+                Some(CrateName::new("test").unwrap()),
                 default_cfg,
                 Env::default(),
                 Default::default(),
diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs
index 9a61f1d56..c330314d4 100644
--- a/crates/base_db/src/input.rs
+++ b/crates/base_db/src/input.rs
@@ -127,10 +127,11 @@ impl PartialEq for ProcMacro {
 pub struct CrateData {
     pub root_file_id: FileId,
     pub edition: Edition,
-    /// The name to display to the end user.
-    /// This actual crate name can be different in a particular dependent crate
-    /// or may even be missing for some cases, such as a dummy crate for the code snippet.
-    pub display_name: Option<String>,
+    /// A name used in the package's project declaration: for Cargo projects, it's [package].name,
+    /// can be different for other project types or even absent (a dummy crate for the code snippet, for example).
+    /// NOTE: The crate can be referenced as a dependency under a different name,
+    /// this one should be used when working with crate hierarchies.
+    pub declaration_name: Option<CrateName>,
     pub cfg_options: CfgOptions,
     pub env: Env,
     pub dependencies: Vec<Dependency>,
@@ -159,7 +160,7 @@ impl CrateGraph {
         &mut self,
         file_id: FileId,
         edition: Edition,
-        display_name: Option<String>,
+        declaration_name: Option<CrateName>,
         cfg_options: CfgOptions,
         env: Env,
         proc_macro: Vec<(SmolStr, Arc<dyn tt::TokenExpander>)>,
@@ -170,7 +171,7 @@ impl CrateGraph {
         let data = CrateData {
             root_file_id: file_id,
             edition,
-            display_name,
+            declaration_name,
             cfg_options,
             env,
             proc_macro,
diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs
index a445a97b3..c75d46bff 100644
--- a/crates/hir/src/code_model.rs
+++ b/crates/hir/src/code_model.rs
@@ -2,7 +2,7 @@
 use std::{iter, sync::Arc};
 
 use arrayvec::ArrayVec;
-use base_db::{CrateId, Edition, FileId};
+use base_db::{CrateId, CrateName, Edition, FileId};
 use either::Either;
 use hir_def::find_path::PrefixKind;
 use hir_def::{
@@ -99,8 +99,8 @@ impl Crate {
         db.crate_graph()[self.id].edition
     }
 
-    pub fn display_name(self, db: &dyn HirDatabase) -> Option<String> {
-        db.crate_graph()[self.id].display_name.clone()
+    pub fn declaration_name(self, db: &dyn HirDatabase) -> Option<CrateName> {
+        db.crate_graph()[self.id].declaration_name.clone()
     }
 
     pub fn query_external_importables(
diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs
index a442fb63a..44bfe1593 100644
--- a/crates/hir_def/src/import_map.rs
+++ b/crates/hir_def/src/import_map.rs
@@ -334,14 +334,14 @@ mod tests {
 
     use super::*;
 
-    fn check_search(ra_fixture: &str, krate_name: &str, query: Query, expect: Expect) {
+    fn check_search(ra_fixture: &str, crate_name: &str, query: Query, expect: Expect) {
         let db = TestDB::with_files(ra_fixture);
         let crate_graph = db.crate_graph();
         let krate = crate_graph
             .iter()
             .find(|krate| {
-                crate_graph[*krate].display_name.as_ref().map(|n| n.to_string())
-                    == Some(krate_name.to_string())
+                crate_graph[*krate].declaration_name.as_ref().map(|n| n.to_string())
+                    == Some(crate_name.to_string())
             })
             .unwrap();
 
@@ -359,7 +359,7 @@ mod tests {
                     let path = map.path_of(item).unwrap();
                     format!(
                         "{}::{} ({})\n",
-                        crate_graph[krate].display_name.as_ref().unwrap(),
+                        crate_graph[krate].declaration_name.as_ref().unwrap(),
                         path,
                         mark
                     )
@@ -400,7 +400,7 @@ mod tests {
             .iter()
             .filter_map(|krate| {
                 let cdata = &crate_graph[krate];
-                let name = cdata.display_name.as_ref()?;
+                let name = cdata.declaration_name.as_ref()?;
 
                 let map = db.import_map(krate);
 
diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs
index 5e4d73c1f..464ffef21 100644
--- a/crates/hir_def/src/nameres.rs
+++ b/crates/hir_def/src/nameres.rs
@@ -173,7 +173,7 @@ 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]
-                .display_name
+                .declaration_name
                 .as_ref()
                 .map(ToString::to_string)
                 .unwrap_or_default()
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 9cf02f0a3..4521d72cc 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -289,7 +289,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()].display_name.as_ref().map(ToString::to_string);
+        db.crate_graph()[module.krate().into()].declaration_name.as_ref().map(ToString::to_string);
     let module_path = module
         .path_to_root(db)
         .into_iter()
@@ -3163,4 +3163,34 @@ fn main() { let s<|>t = test().get(); }
             "#]],
         );
     }
+
+    #[test]
+    fn hover_displays_normalized_crate_names() {
+        check(
+            r#"
+//- /lib.rs crate:name-with-dashes
+pub mod wrapper {
+    pub struct Thing { x: u32 }
+
+    impl Thing {
+        pub fn new() -> Thing { Thing { x: 0 } }
+    }
+}
+
+//- /main.rs crate:main deps:name-with-dashes
+fn main() { let foo_test = name_with_dashes::wrapper::Thing::new<|>(); }
+"#,
+            expect![[r#"
+            *new*
+
+            ```rust
+            name_with_dashes::wrapper::Thing
+            ```
+
+            ```rust
+            pub fn new() -> Thing
+            ```
+            "#]],
+        )
+    }
 }
diff --git a/crates/ide/src/link_rewrite.rs b/crates/ide/src/link_rewrite.rs
index a16f90e17..c317a2379 100644
--- a/crates/ide/src/link_rewrite.rs
+++ b/crates/ide/src/link_rewrite.rs
@@ -107,7 +107,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.display_name(db)?))
+        .join(&format!("{}/", krate.declaration_name(db)?))
         .ok()?
         .join(&canonical_path.replace("::", "/"))
         .ok()?
@@ -127,7 +127,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.display_name(db)?, canonical_path.replace("::", "/"));
+    let base = format!("{}/{}", krate.declaration_name(db)?, canonical_path.replace("::", "/"));
 
     get_doc_url(db, &krate)
         .and_then(|url| url.join(&base).ok())
@@ -248,7 +248,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.display_name(db)?))
+            Some(format!("https://docs.rs/{}/*/", krate.declaration_name(db)?))
         })
         .and_then(|s| Url::parse(&s).ok())
 }
diff --git a/crates/ide/src/status.rs b/crates/ide/src/status.rs
index 0af84daa0..f67f10491 100644
--- a/crates/ide/src/status.rs
+++ b/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].display_name {
+                let display_crate = |krate: CrateId| match &crate_graph[krate].declaration_name {
                     Some(it) => format!("{}({:?})", it, krate),
                     None => format!("{:?}", krate),
                 };
diff --git a/crates/project_model/src/lib.rs b/crates/project_model/src/lib.rs
index 258f60e28..d1e7602fc 100644
--- a/crates/project_model/src/lib.rs
+++ b/crates/project_model/src/lib.rs
@@ -411,7 +411,7 @@ impl ProjectWorkspace {
                             let crate_id = crate_graph.add_crate_root(
                                 file_id,
                                 edition,
-                                Some(cargo[pkg].name.clone()),
+                                Some(CrateName::normalize_dashes(&cargo[pkg].name)),
                                 cfg_options,
                                 env,
                                 proc_macro.clone(),
@@ -546,7 +546,8 @@ fn sysroot_to_crate_graph(
 
             let env = Env::default();
             let proc_macro = vec![];
-            let name = sysroot[krate].name.clone();
+            let name = CrateName::new(&sysroot[krate].name)
+                .expect("Sysroot crates' names do not contain dashes");
             let crate_id = crate_graph.add_crate_root(
                 file_id,
                 Edition::Edition2018,
diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs
index f3b6c900e..d1d3b12f8 100644
--- a/crates/rust-analyzer/src/cli/diagnostics.rs
+++ b/crates/rust-analyzer/src/cli/diagnostics.rs
@@ -36,11 +36,12 @@ 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 = if let Some(name) = module.krate().display_name(db) {
-                format!("{}", name)
-            } else {
-                String::from("unknown")
-            };
+            let crate_name = module
+                .krate()
+                .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()
             {