Merge #5158

5158: Use CrateName correctly r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

8 files changed, 45 insertions, 29 deletions

diff --git a/crates/ra_db/src/fixture.rs b/crates/ra_db/src/fixture.rs
index 4f4fb4494..209713987 100644
--- a/crates/ra_db/src/fixture.rs
+++ b/crates/ra_db/src/fixture.rs
@@ -149,15 +149,17 @@ fn with_files(
             let crate_id = crate_graph.add_crate_root(
                 file_id,
                 meta.edition,
-                Some(CrateName::new(&krate).unwrap()),
+                Some(krate.clone()),
                 meta.cfg,
                 meta.env,
                 Default::default(),
             );
-            let prev = crates.insert(krate.clone(), crate_id);
+            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 {
-                crate_deps.push((krate.clone(), dep))
+                let dep = CrateName::new(&dep).unwrap();
+                crate_deps.push((crate_name.clone(), dep))
             }
         } else if meta.path == "/main.rs" || meta.path == "/lib.rs" {
             assert!(default_crate_root.is_none());
diff --git a/crates/ra_db/src/input.rs b/crates/ra_db/src/input.rs
index 7f3660118..445a1ee48 100644
--- a/crates/ra_db/src/input.rs
+++ b/crates/ra_db/src/input.rs
@@ -67,7 +67,7 @@ pub struct CrateGraph {
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct CrateId(pub u32);
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct CrateName(SmolStr);
 
 impl CrateName {
@@ -94,6 +94,13 @@ impl fmt::Display for CrateName {
     }
 }
 
+impl ops::Deref for CrateName {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        &*self.0
+    }
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct ProcMacroId(pub u32);
 
@@ -117,7 +124,7 @@ pub struct CrateData {
     /// 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<CrateName>,
+    pub display_name: Option<String>,
     pub cfg_options: CfgOptions,
     pub env: Env,
     pub dependencies: Vec<Dependency>,
@@ -138,7 +145,7 @@ pub struct Env {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Dependency {
     pub crate_id: CrateId,
-    pub name: SmolStr,
+    pub name: CrateName,
 }
 
 impl CrateGraph {
@@ -146,7 +153,7 @@ impl CrateGraph {
         &mut self,
         file_id: FileId,
         edition: Edition,
-        display_name: Option<CrateName>,
+        display_name: Option<String>,
         cfg_options: CfgOptions,
         env: Env,
         proc_macro: Vec<(SmolStr, Arc<dyn ra_tt::TokenExpander>)>,
@@ -178,7 +185,7 @@ impl CrateGraph {
         if self.dfs_find(from, to, &mut FxHashSet::default()) {
             return Err(CyclicDependenciesError);
         }
-        self.arena.get_mut(&from).unwrap().add_dep(name.0, to);
+        self.arena.get_mut(&from).unwrap().add_dep(name, to);
         Ok(())
     }
 
@@ -247,7 +254,7 @@ impl CrateId {
 }
 
 impl CrateData {
-    fn add_dep(&mut self, name: SmolStr, crate_id: CrateId) {
+    fn add_dep(&mut self, name: CrateName, crate_id: CrateId) {
         self.dependencies.push(Dependency { name, crate_id })
     }
 }
@@ -429,7 +436,10 @@ mod tests {
             .is_ok());
         assert_eq!(
             graph[crate1].dependencies,
-            vec![Dependency { crate_id: crate2, name: "crate_name_with_dashes".into() }]
+            vec![Dependency {
+                crate_id: crate2,
+                name: CrateName::new("crate_name_with_dashes").unwrap()
+            }]
         );
     }
 }
diff --git a/crates/ra_hir/src/code_model.rs b/crates/ra_hir/src/code_model.rs
index e86077dd6..e09eb77c2 100644
--- a/crates/ra_hir/src/code_model.rs
+++ b/crates/ra_hir/src/code_model.rs
@@ -31,7 +31,7 @@ use hir_ty::{
     ApplicationTy, Canonical, GenericPredicate, InEnvironment, Substs, TraitEnvironment, Ty,
     TyDefId, TypeCtor,
 };
-use ra_db::{CrateId, CrateName, Edition, FileId};
+use ra_db::{CrateId, Edition, FileId};
 use ra_prof::profile;
 use ra_syntax::ast::{self, AttrsOwner, NameOwner};
 use rustc_hash::FxHashSet;
@@ -94,8 +94,8 @@ impl Crate {
         db.crate_graph()[self.id].edition
     }
 
-    pub fn display_name(self, db: &dyn HirDatabase) -> Option<CrateName> {
-        db.crate_graph()[self.id].display_name.as_ref().cloned()
+    pub fn display_name(self, db: &dyn HirDatabase) -> Option<String> {
+        db.crate_graph()[self.id].display_name.clone()
     }
 
     pub fn query_external_importables(
diff --git a/crates/ra_hir_expand/src/builtin_derive.rs b/crates/ra_hir_expand/src/builtin_derive.rs
index 26b667b55..f2d664863 100644
--- a/crates/ra_hir_expand/src/builtin_derive.rs
+++ b/crates/ra_hir_expand/src/builtin_derive.rs
@@ -161,7 +161,7 @@ fn find_builtin_crate(db: &dyn AstDatabase, id: LazyMacroId) -> tt::TokenTree {
     // XXX
     //  All crates except core itself should have a dependency on core,
     //  We detect `core` by seeing whether it doesn't have such a dependency.
-    let tt = if cg[krate].dependencies.iter().any(|dep| dep.name == "core") {
+    let tt = if cg[krate].dependencies.iter().any(|dep| &*dep.name == "core") {
         quote! { core }
     } else {
         quote! { crate }
diff --git a/crates/ra_hir_expand/src/name.rs b/crates/ra_hir_expand/src/name.rs
index 1b0303685..969a2e5b8 100644
--- a/crates/ra_hir_expand/src/name.rs
+++ b/crates/ra_hir_expand/src/name.rs
@@ -117,7 +117,7 @@ impl AsName for ast::FieldKind {
 
 impl AsName for ra_db::Dependency {
     fn as_name(&self) -> Name {
-        Name::new_text(self.name.clone())
+        Name::new_text(SmolStr::new(&*self.name))
     }
 }
 
diff --git a/crates/ra_ide/src/mock_analysis.rs b/crates/ra_ide/src/mock_analysis.rs
index db6d50694..a393d3dba 100644
--- a/crates/ra_ide/src/mock_analysis.rs
+++ b/crates/ra_ide/src/mock_analysis.rs
@@ -130,7 +130,7 @@ impl MockAnalysis {
                 let other_crate = crate_graph.add_crate_root(
                     file_id,
                     edition,
-                    Some(CrateName::new(crate_name).unwrap()),
+                    Some(crate_name.to_string()),
                     cfg,
                     env,
                     Default::default(),
diff --git a/crates/ra_project_model/src/lib.rs b/crates/ra_project_model/src/lib.rs
index 8b85b4831..8dbf4e6ea 100644
--- a/crates/ra_project_model/src/lib.rs
+++ b/crates/ra_project_model/src/lib.rs
@@ -288,10 +288,7 @@ impl ProjectWorkspace {
                         if let (Some(&from), Some(&to)) =
                             (crates.get(&from_crate_id), crates.get(&to_crate_id))
                         {
-                            if crate_graph
-                                .add_dep(from, CrateName::new(&dep.name).unwrap(), to)
-                                .is_err()
-                            {
+                            if crate_graph.add_dep(from, dep.name.clone(), to).is_err() {
                                 log::error!(
                                     "cyclic dependency {:?} -> {:?}",
                                     from_crate_id,
@@ -312,13 +309,11 @@ impl ProjectWorkspace {
 
                         let env = Env::default();
                         let proc_macro = vec![];
-                        let crate_name = CrateName::new(&sysroot[krate].name)
-                            .expect("Sysroot crate names should not contain dashes");
-
+                        let name = sysroot[krate].name.clone();
                         let crate_id = crate_graph.add_crate_root(
                             file_id,
                             Edition::Edition2018,
-                            Some(crate_name),
+                            Some(name),
                             cfg_options.clone(),
                             env,
                             proc_macro,
@@ -392,7 +387,7 @@ impl ProjectWorkspace {
                             let crate_id = crate_graph.add_crate_root(
                                 file_id,
                                 edition,
-                                Some(CrateName::normalize_dashes(&cargo[pkg].name)),
+                                Some(cargo[pkg].name.clone()),
                                 cfg_options,
                                 env,
                                 proc_macro.clone(),
diff --git a/crates/ra_project_model/src/project_json.rs b/crates/ra_project_model/src/project_json.rs
index 4b5dcd634..9fe1e2dcb 100644
--- a/crates/ra_project_model/src/project_json.rs
+++ b/crates/ra_project_model/src/project_json.rs
@@ -4,9 +4,9 @@ use std::path::PathBuf;
 
 use paths::{AbsPath, AbsPathBuf};
 use ra_cfg::CfgOptions;
-use ra_db::{CrateId, Dependency, Edition};
+use ra_db::{CrateId, CrateName, Dependency, Edition};
 use rustc_hash::FxHashSet;
-use serde::Deserialize;
+use serde::{de, Deserialize};
 use stdx::split_delim;
 
 /// Roots and crates that compose this Rust project.
@@ -50,7 +50,7 @@ impl ProjectJson {
                         .into_iter()
                         .map(|dep_data| Dependency {
                             crate_id: CrateId(dep_data.krate as u32),
-                            name: dep_data.name.into(),
+                            name: dep_data.name,
                         })
                         .collect::<Vec<_>>(),
                     cfg: {
@@ -113,5 +113,14 @@ struct DepData {
     /// Identifies a crate by position in the crates array.
     #[serde(rename = "crate")]
     krate: usize,
-    name: String,
+    #[serde(deserialize_with = "deserialize_crate_name")]
+    name: CrateName,
+}
+
+fn deserialize_crate_name<'de, D>(de: D) -> Result<CrateName, D::Error>
+where
+    D: de::Deserializer<'de>,
+{
+    let name = String::deserialize(de)?;
+    CrateName::new(&name).map_err(|err| de::Error::custom(format!("invalid crate name: {:?}", err)))
 }