chore: Implement FromStr and Into<String> for CrateName (#2061)

crates/lsp/src/lib_hacky.rs

@@ -22,7 +22,7 @@ use lsp_types::{
 use noirc_driver::{check_crate, create_local_crate, create_non_local_crate, propagate_dep};
 use noirc_errors::{DiagnosticKind, FileDiagnostic};
 use noirc_frontend::{
-    graph::{CrateGraph, CrateId, CrateName, CrateType},
+    graph::{CrateGraph, CrateId, CrateType},
     hir::Context,
 };
 
@@ -299,7 +299,7 @@ fn create_context_at_path(
                     .join(PathBuf::from(&dependency_path).join("src").join("lib.nr"));
                 let library_crate =
                     create_non_local_crate(&mut context, &path_to_lib, CrateType::Library);
-                propagate_dep(&mut context, library_crate, &CrateName::new(crate_name).unwrap());
+                propagate_dep(&mut context, library_crate, &crate_name.parse().unwrap());
             }
         }
     }

crates/nargo_cli/src/resolver.rs

@@ -163,7 +163,7 @@ fn resolve_package_manifest(
 }
 
 fn crate_name(name: Option<CrateName>) -> String {
-    name.map(|name| name.as_string()).unwrap_or_else(|| "[unnamed]".to_string())
+    name.map(|name| name.into()).unwrap_or_else(|| "[unnamed]".to_string())
 }
 
 fn resolve_workspace_manifest(
@@ -191,8 +191,7 @@ fn resolve_workspace_manifest(
                     .package
                     .name
                     .map(|name| {
-                        CrateName::new(&name)
-                            .map_err(|_name| DependencyResolutionError::InvalidPackageName)
+                        name.parse().map_err(|_name| DependencyResolutionError::InvalidPackageName)
                     })
                     .transpose()?;
 
@@ -201,7 +200,7 @@ fn resolve_workspace_manifest(
                 }
 
                 if local_package.is_none() && workspace.default_member.as_ref() == Some(member) {
-                    local_package = name.as_ref().map(CrateName::as_string);
+                    local_package = name.map(Into::into);
                 }
             }
             Manifest::Workspace(_) => {
@@ -214,7 +213,8 @@ fn resolve_workspace_manifest(
     }
 
     let local_package = match local_package {
-        Some(local_package) => CrateName::new(&local_package)
+        Some(local_package) => local_package
+            .parse::<CrateName>()
             .map_err(|_| DependencyResolutionError::InvalidPackageName)?
             .into(),
         None => packages.keys().last().expect("non-empty packages").clone(),

crates/noirc_driver/src/lib.rs

@@ -88,8 +88,8 @@ pub fn create_non_local_crate(
 
 /// Adds a edge in the crate graph for two crates
 pub fn add_dep(context: &mut Context, this_crate: CrateId, depends_on: CrateId, crate_name: &str) {
-    let crate_name = CrateName::new(crate_name)
-        .expect("crate name contains blacklisted characters, please remove");
+    let crate_name =
+        crate_name.parse().expect("crate name contains blacklisted characters, please remove");
 
     // Cannot depend on a binary
     if context.crate_graph.crate_type(depends_on) == CrateType::Binary {
@@ -139,7 +139,7 @@ pub fn check_crate(
     // You can add any crate type to the crate graph
     // but you cannot depend on Binaries
     let std_crate = context.crate_graph.add_stdlib(CrateType::Library, root_file_id);
-    propagate_dep(context, std_crate, &CrateName::new(std_crate_name).unwrap());
+    propagate_dep(context, std_crate, &std_crate_name.parse().unwrap());
 
     let mut errors = vec![];
     match context.crate_graph.crate_type(crate_id) {

crates/noirc_frontend/src/graph/mod.rs

@@ -4,6 +4,8 @@
 // This version is also simpler due to not having macro_defs or proc_macros
 // XXX: Edition may be reintroduced or some sort of versioning
 
+use std::str::FromStr;
+
 use fm::FileId;
 use rustc_hash::{FxHashMap, FxHashSet};
 use smol_str::SmolStr;
@@ -27,23 +29,27 @@ impl CrateId {
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct CrateName(SmolStr);
 
-impl CrateName {
-    /// Creates a new CrateName rejecting any crate name that
-    /// has a character on the blacklist.
-    /// The difference between RA and this implementation is that
-    /// characters on the blacklist are never allowed; there is no normalization.
-    pub fn new(name: &str) -> Result<CrateName, &str> {
+impl From<CrateName> for String {
+    fn from(crate_name: CrateName) -> Self {
+        crate_name.0.into()
+    }
+}
+
+/// Creates a new CrateName rejecting any crate name that
+/// has a character on the blacklist.
+/// The difference between RA and this implementation is that
+/// characters on the blacklist are never allowed; there is no normalization.
+impl FromStr for CrateName {
+    type Err = String;
+
+    fn from_str(name: &str) -> Result<Self, Self::Err> {
         let is_invalid = name.chars().any(|n| CHARACTER_BLACK_LIST.contains(&n));
         if is_invalid {
-            Err(name)
+            Err(name.into())
         } else {
             Ok(Self(SmolStr::new(name)))
         }
     }
-
-    pub fn as_string(&self) -> String {
-        self.0.clone().into()
-    }
 }
 
 #[derive(Debug, Clone, Default, PartialEq, Eq)]
@@ -80,7 +86,7 @@ pub struct Dependency {
 
 impl Dependency {
     pub fn as_name(&self) -> String {
-        self.name.as_string()
+        self.name.clone().into()
     }
 }
 
@@ -212,7 +218,7 @@ pub struct CyclicDependenciesError {
 mod tests {
     use std::path::PathBuf;
 
-    use super::{CrateGraph, CrateName, CrateType, FileId};
+    use super::{CrateGraph, CrateType, FileId};
 
     fn dummy_file_ids(n: usize) -> Vec<FileId> {
         use fm::{FileMap, FILE_EXTENSION};
@@ -238,9 +244,9 @@ mod tests {
         let crate2 = graph.add_crate_root(CrateType::Library, file_ids[1]);
         let crate3 = graph.add_crate_root(CrateType::Library, file_ids[2]);
 
-        assert!(graph.add_dep(crate1, CrateName::new("crate2").unwrap(), crate2).is_ok());
-        assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok());
-        assert!(graph.add_dep(crate3, CrateName::new("crate1").unwrap(), crate1).is_err());
+        assert!(graph.add_dep(crate1, "crate2".parse().unwrap(), crate2).is_ok());
+        assert!(graph.add_dep(crate2, "crate3".parse().unwrap(), crate3).is_ok());
+        assert!(graph.add_dep(crate3, "crate1".parse().unwrap(), crate1).is_err());
     }
 
     #[test]
@@ -253,8 +259,8 @@ mod tests {
         let crate1 = graph.add_crate_root(CrateType::Library, file_id_0);
         let crate2 = graph.add_crate_root(CrateType::Library, file_id_1);
         let crate3 = graph.add_crate_root(CrateType::Library, file_id_2);
-        assert!(graph.add_dep(crate1, CrateName::new("crate2").unwrap(), crate2).is_ok());
-        assert!(graph.add_dep(crate2, CrateName::new("crate3").unwrap(), crate3).is_ok());
+        assert!(graph.add_dep(crate1, "crate2".parse().unwrap(), crate2).is_ok());
+        assert!(graph.add_dep(crate2, "crate3".parse().unwrap(), crate3).is_ok());
     }
     #[test]
     fn it_works2() {

crates/wasm/src/compile.rs

@@ -7,7 +7,7 @@ use noirc_driver::{
     propagate_dep, CompileOptions, CompiledContract,
 };
 use noirc_frontend::{
-    graph::{CrateGraph, CrateName, CrateType},
+    graph::{CrateGraph, CrateType},
     hir::Context,
 };
 use serde::{Deserialize, Serialize};
@@ -65,7 +65,7 @@ fn add_noir_lib(context: &mut Context, crate_name: &str) {
     let path_to_lib = Path::new(&crate_name).join("lib.nr");
     let library_crate = create_non_local_crate(context, &path_to_lib, CrateType::Library);
 
-    propagate_dep(context, library_crate, &CrateName::new(crate_name).unwrap());
+    propagate_dep(context, library_crate, &crate_name.parse().unwrap());
 }
 
 #[wasm_bindgen]