fix: Require package names to be non-empty (#2293)

Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>

crates/nargo_cli/src/cli/init_cmd.rs

@@ -6,14 +6,15 @@ use acvm::Backend;
 use clap::Args;
 use nargo::constants::{PKG_FILE, SRC_DIR};
 use nargo::package::PackageType;
+use noirc_frontend::graph::CrateName;
 use std::path::PathBuf;
 
 /// Create a Noir project in the current directory.
 #[derive(Debug, Clone, Args)]
 pub(crate) struct InitCommand {
     /// Name of the package [default: current directory name]
     #[clap(long)]
-    name: Option<String>,
+    name: Option<CrateName>,
 
     /// Use a library template
     #[arg(long, conflicts_with = "bin", conflicts_with = "contract")]
@@ -67,9 +68,13 @@ pub(crate) fn run<B: Backend>(
     args: InitCommand,
     config: NargoConfig,
 ) -> Result<(), CliError<B>> {
-    let package_name = args
-        .name
-        .unwrap_or_else(|| config.program_dir.file_name().unwrap().to_str().unwrap().to_owned());
+    let package_name = match args.name {
+        Some(name) => name,
+        None => {
+            let name = config.program_dir.file_name().unwrap().to_str().unwrap();
+            name.parse().map_err(|_| CliError::InvalidPackageName(name.into()))?
+        }
+    };
 
     let package_type = if args.lib {
         PackageType::Library
@@ -78,14 +83,14 @@ pub(crate) fn run<B: Backend>(
     } else {
         PackageType::Binary
     };
-    initialize_project(config.program_dir, &package_name, package_type);
+    initialize_project(config.program_dir, package_name, package_type);
     Ok(())
 }
 
 /// Initializes a new Noir project in `package_dir`.
 pub(crate) fn initialize_project(
     package_dir: PathBuf,
-    package_name: &str,
+    package_name: CrateName,
     package_type: PackageType,
 ) {
     let src_dir = package_dir.join(SRC_DIR);

crates/nargo_cli/src/cli/new_cmd.rs

@@ -4,6 +4,7 @@ use super::{init_cmd::initialize_project, NargoConfig};
 use acvm::Backend;
 use clap::Args;
 use nargo::package::PackageType;
+use noirc_frontend::graph::CrateName;
 use std::path::PathBuf;
 
 /// Create a Noir project in a new directory.
@@ -14,7 +15,7 @@ pub(crate) struct NewCommand {
 
     /// Name of the package [default: package directory name]
     #[clap(long)]
-    name: Option<String>,
+    name: Option<CrateName>,
 
     /// Use a library template
     #[arg(long, conflicts_with = "bin", conflicts_with = "contract")]
@@ -41,15 +42,20 @@ pub(crate) fn run<B: Backend>(
         return Err(CliError::DestinationAlreadyExists(package_dir));
     }
 
-    let package_name =
-        args.name.unwrap_or_else(|| args.path.file_name().unwrap().to_str().unwrap().to_owned());
+    let package_name = match args.name {
+        Some(name) => name,
+        None => {
+            let name = args.path.file_name().unwrap().to_str().unwrap();
+            name.parse().map_err(|_| CliError::InvalidPackageName(name.into()))?
+        }
+    };
     let package_type = if args.lib {
         PackageType::Library
     } else if args.contract {
         PackageType::Contract
     } else {
         PackageType::Binary
     };
-    initialize_project(package_dir, &package_name, package_type);
+    initialize_project(package_dir, package_name, package_type);
     Ok(())
 }

crates/nargo_cli/src/errors.rs

@@ -41,6 +41,9 @@ pub(crate) enum CliError<B: Backend> {
     #[error("Failed to verify proof {}", .0.display())]
     InvalidProof(PathBuf),
 
+    #[error("Invalid package name {0}. Did you mean to use `--name`?")]
+    InvalidPackageName(String),
+
     /// ABI encoding/decoding error
     #[error(transparent)]
     AbiError(#[from] AbiError),

crates/nargo_cli/tests/compile_failure/invalid_dependency_name/Nargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "invalid_dependency_name"
+type = "bin"
+authors = [""]
+compiler_version = "0.9.0"
+
+[dependencies]
+bad_name = { path = "../../test_libraries/bad_name" }

crates/nargo_cli/tests/compile_failure/invalid_dependency_name/src/main.nr

@@ -0,0 +1 @@
+fn main(x: Field) { }

crates/nargo_cli/tests/compile_failure/package_name_empty/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = ""
+type = "bin"
+authors = [""]
+compiler_version = "0.9.0"
+
+[dependencies]

crates/nargo_cli/tests/compile_failure/package_name_empty/src/main.nr

@@ -0,0 +1 @@
+fn main(x: Field) { }

crates/nargo_cli/tests/compile_failure/package_name_hyphen/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "hyphenated-name"
+type = "bin"
+authors = [""]
+compiler_version = "0.9.0"
+
+[dependencies]

crates/nargo_cli/tests/compile_failure/package_name_hyphen/src/main.nr

@@ -0,0 +1 @@
+fn main(x: Field) { }

crates/nargo_cli/tests/test_libraries/bad_name/Nargo.toml

@@ -0,0 +1,7 @@
+[package]
+name = "bad-name"
+type = "lib"
+authors = [""]
+compiler_version = "0.7.1"
+
+[dependencies]

crates/nargo_toml/src/errors.rs

@@ -38,9 +38,11 @@ pub enum ManifestError {
     )]
     MissingDefaultEntryFile { toml: PathBuf, entry: PathBuf, package_type: PackageType },
 
-    /// Invalid character `-` in package name
-    #[error("invalid character `-` in package name")]
-    InvalidPackageName,
+    #[error("{} found in {toml}", if name.is_empty() { "Empty package name".into() } else { format!("Invalid package name `{name}`") })]
+    InvalidPackageName { toml: PathBuf, name: String },
+
+    #[error("{} found in {toml}", if name.is_empty() { "Empty dependency name".into() } else { format!("Invalid dependency name `{name}`") })]
+    InvalidDependencyName { toml: PathBuf, name: String },
 
     /// Encountered error while downloading git repository.
     #[error("{0}")]

crates/nargo_toml/src/lib.rs

@@ -72,14 +72,20 @@ struct PackageConfig {
 impl PackageConfig {
     fn resolve_to_package(&self, root_dir: &Path) -> Result<Package, ManifestError> {
         let name = if let Some(name) = &self.package.name {
-            name.parse().map_err(|_| ManifestError::InvalidPackageName)?
+            name.parse().map_err(|_| ManifestError::InvalidPackageName {
+                toml: root_dir.join("Nargo.toml"),
+                name: name.into(),
+            })?
         } else {
             return Err(ManifestError::MissingNameField { toml: root_dir.join("Nargo.toml") });
         };
 
         let mut dependencies: BTreeMap<CrateName, Dependency> = BTreeMap::new();
         for (name, dep_config) in self.dependencies.iter() {
-            let name = name.parse().map_err(|_| ManifestError::InvalidPackageName)?;
+            let name = name.parse().map_err(|_| ManifestError::InvalidDependencyName {
+                toml: root_dir.join("Nargo.toml"),
+                name: name.into(),
+            })?;
             let resolved_dep = dep_config.resolve_to_dependency(root_dir)?;
 
             dependencies.insert(name, resolved_dep);

crates/noirc_frontend/src/graph/mod.rs

@@ -29,6 +29,12 @@ impl CrateId {
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Ord, PartialOrd)]
 pub struct CrateName(SmolStr);
 
+impl CrateName {
+   fn is_valid_name(name: &str) -> bool {
+        !name.is_empty() && name.chars().all(|n| !CHARACTER_BLACK_LIST.contains(&n))
+    }
+}
+
 impl Display for CrateName {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         self.0.fmt(f)
@@ -54,11 +60,28 @@ 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.into())
-        } else {
+        if Self::is_valid_name(name) {
             Ok(Self(SmolStr::new(name)))
+        } else {
+            Err("Package names must be non-empty and cannot contain hyphens".into())
+        }
+    }
+}
+
+#[cfg(test)]
+mod crate_name {
+    use super::{CrateName, CHARACTER_BLACK_LIST};
+
+    #[test]
+    fn it_rejects_empty_string() {
+        assert!(!CrateName::is_valid_name(""));
+    }
+
+    #[test]
+    fn it_rejects_blacklisted_chars() {
+        for bad_char in CHARACTER_BLACK_LIST {
+            let bad_char_string = bad_char.to_string();
+            assert!(!CrateName::is_valid_name(&bad_char_string));
         }
     }
 }