diff --git a/crates/nargo_cli/src/cli/init_cmd.rs b/crates/nargo_cli/src/cli/init_cmd.rs index d6ad172ce26..e6020e3cfd9 100644 --- a/crates/nargo_cli/src/cli/init_cmd.rs +++ b/crates/nargo_cli/src/cli/init_cmd.rs @@ -6,6 +6,7 @@ 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. @@ -13,7 +14,7 @@ use std::path::PathBuf; pub(crate) struct InitCommand { /// Name of the package [default: current directory name] #[clap(long)] - name: Option, + name: Option, /// Use a library template #[arg(long, conflicts_with = "bin", conflicts_with = "contract")] @@ -67,9 +68,13 @@ pub(crate) fn run( args: InitCommand, config: NargoConfig, ) -> Result<(), CliError> { - 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( } 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); diff --git a/crates/nargo_cli/src/cli/new_cmd.rs b/crates/nargo_cli/src/cli/new_cmd.rs index dbdeddef712..d6a9d00257b 100644 --- a/crates/nargo_cli/src/cli/new_cmd.rs +++ b/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, + name: Option, /// Use a library template #[arg(long, conflicts_with = "bin", conflicts_with = "contract")] @@ -41,8 +42,13 @@ pub(crate) fn run( 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 { @@ -50,6 +56,6 @@ pub(crate) fn run( } else { PackageType::Binary }; - initialize_project(package_dir, &package_name, package_type); + initialize_project(package_dir, package_name, package_type); Ok(()) } diff --git a/crates/nargo_cli/src/errors.rs b/crates/nargo_cli/src/errors.rs index d801a56d3f5..dade4262431 100644 --- a/crates/nargo_cli/src/errors.rs +++ b/crates/nargo_cli/src/errors.rs @@ -41,6 +41,9 @@ pub(crate) enum CliError { #[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), diff --git a/crates/nargo_cli/tests/compile_failure/invalid_dependency_name/Nargo.toml b/crates/nargo_cli/tests/compile_failure/invalid_dependency_name/Nargo.toml new file mode 100644 index 00000000000..69b56a2e7d0 --- /dev/null +++ b/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" } diff --git a/crates/nargo_cli/tests/compile_failure/invalid_dependency_name/src/main.nr b/crates/nargo_cli/tests/compile_failure/invalid_dependency_name/src/main.nr new file mode 100644 index 00000000000..faf1ba0045a --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/invalid_dependency_name/src/main.nr @@ -0,0 +1 @@ +fn main(x: Field) { } diff --git a/crates/nargo_cli/tests/compile_failure/package_name_empty/Nargo.toml b/crates/nargo_cli/tests/compile_failure/package_name_empty/Nargo.toml new file mode 100644 index 00000000000..7c4dbd0c994 --- /dev/null +++ b/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] diff --git a/crates/nargo_cli/tests/compile_failure/package_name_empty/src/main.nr b/crates/nargo_cli/tests/compile_failure/package_name_empty/src/main.nr new file mode 100644 index 00000000000..faf1ba0045a --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/package_name_empty/src/main.nr @@ -0,0 +1 @@ +fn main(x: Field) { } diff --git a/crates/nargo_cli/tests/compile_failure/package_name_hyphen/Nargo.toml b/crates/nargo_cli/tests/compile_failure/package_name_hyphen/Nargo.toml new file mode 100644 index 00000000000..f8d0a85db4a --- /dev/null +++ b/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] diff --git a/crates/nargo_cli/tests/compile_failure/package_name_hyphen/src/main.nr b/crates/nargo_cli/tests/compile_failure/package_name_hyphen/src/main.nr new file mode 100644 index 00000000000..faf1ba0045a --- /dev/null +++ b/crates/nargo_cli/tests/compile_failure/package_name_hyphen/src/main.nr @@ -0,0 +1 @@ +fn main(x: Field) { } diff --git a/crates/nargo_cli/tests/test_libraries/bad_name/Nargo.toml b/crates/nargo_cli/tests/test_libraries/bad_name/Nargo.toml new file mode 100644 index 00000000000..313e2411e83 --- /dev/null +++ b/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] diff --git a/crates/nargo_cli/tests/test_libraries/bad_name/src/lib.nr b/crates/nargo_cli/tests/test_libraries/bad_name/src/lib.nr new file mode 100644 index 00000000000..e69de29bb2d diff --git a/crates/nargo_toml/src/errors.rs b/crates/nargo_toml/src/errors.rs index 42c5a59cba6..e986a181e43 100644 --- a/crates/nargo_toml/src/errors.rs +++ b/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}")] diff --git a/crates/nargo_toml/src/lib.rs b/crates/nargo_toml/src/lib.rs index 4a9238ee021..0e05a0a7901 100644 --- a/crates/nargo_toml/src/lib.rs +++ b/crates/nargo_toml/src/lib.rs @@ -72,14 +72,20 @@ struct PackageConfig { impl PackageConfig { fn resolve_to_package(&self, root_dir: &Path) -> Result { 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 = 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); diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index 2c251432e9b..7a1daef45fd 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/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 { - 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)); } } }