Merge #6124

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

Closes #5343 
Closes #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 <mail4score@gmail.com>

Author: bors[bot]

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(),

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,

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(

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);
 

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()

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
+            ```
+            "#]],
+        )
+    }
 }

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())
 }

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),
                 };

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,

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()
             {