From a383185c38a9c076796ce4e6f922776053fae056 Mon Sep 17 00:00:00 2001 From: Ed Page <eopage@gmail.com> Date: Wed, 13 Dec 2023 09:36:14 -0600 Subject: [PATCH] refactor: Centralize empty name check --- src/cargo/ops/cargo_add/crate_spec.rs | 4 ---- src/cargo/ops/cargo_new.rs | 5 +---- src/cargo/util/command_prelude.rs | 6 ------ src/cargo/util/config/mod.rs | 3 --- src/cargo/util/restricted_names.rs | 4 ++++ src/cargo/util/toml/mod.rs | 4 ---- src/cargo/util_schemas/core/package_id_spec.rs | 6 ------ tests/testsuite/build.rs | 2 +- 8 files changed, 6 insertions(+), 28 deletions(-) diff --git a/src/cargo/ops/cargo_add/crate_spec.rs b/src/cargo/ops/cargo_add/crate_spec.rs index 4d0f067f6cf..4cf6f484a32 100644 --- a/src/cargo/ops/cargo_add/crate_spec.rs +++ b/src/cargo/ops/cargo_add/crate_spec.rs @@ -1,6 +1,5 @@ //! Crate name parsing. -use anyhow::bail; use anyhow::Context as _; use super::Dependency; @@ -29,9 +28,6 @@ impl CrateSpec { .map(|(n, v)| (n, Some(v))) .unwrap_or((pkg_id, None)); - if name.is_empty() { - bail!("dependency name cannot be empty"); - } validate_package_name(name, "dependency name", "")?; if let Some(version) = version { diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index b68baaed4a6..1c06b5f8258 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -4,7 +4,7 @@ use crate::util::important_paths::find_root_manifest_for_wd; use crate::util::toml_mut::is_sorted; use crate::util::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo}; use crate::util::{restricted_names, Config}; -use anyhow::{anyhow, bail, Context}; +use anyhow::{anyhow, Context}; use cargo_util::paths::{self, write_atomic}; use serde::de; use serde::Deserialize; @@ -197,9 +197,6 @@ fn check_name( } help }; - if name.is_empty() { - bail!("package name cannot be empty"); - } restricted_names::validate_package_name(name, "package name", &bin_help())?; if restricted_names::is_keyword(name) { diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 6ce6161be40..3e236a6f735 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -833,9 +833,6 @@ Run `{cmd}` to see possible targets." (None, None) => config.default_registry()?.map(RegistryOrIndex::Registry), (None, Some(i)) => Some(RegistryOrIndex::Index(i.into_url()?)), (Some(r), None) => { - if r.is_empty() { - bail!("registry name cannot be empty"); - } restricted_names::validate_package_name(r, "registry name", "")?; Some(RegistryOrIndex::Registry(r.to_string())) } @@ -851,9 +848,6 @@ Run `{cmd}` to see possible targets." match self._value_of("registry").map(|s| s.to_string()) { None => config.default_registry(), Some(registry) => { - if registry.is_empty() { - bail!("registry name cannot be empty"); - } restricted_names::validate_package_name(®istry, "registry name", "")?; Ok(Some(registry)) } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index cf70eda91ee..c0dd42d39b8 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -1551,9 +1551,6 @@ impl Config { /// Gets the index for a registry. pub fn get_registry_index(&self, registry: &str) -> CargoResult<Url> { - if registry.is_empty() { - bail!("registry name cannot be empty"); - } validate_package_name(registry, "registry name", "")?; if let Some(index) = self.get_string(&format!("registries.{}.index", registry))? { self.resolve_registry_index(&index).with_context(|| { diff --git a/src/cargo/util/restricted_names.rs b/src/cargo/util/restricted_names.rs index f61249775d3..df0152ecb1e 100644 --- a/src/cargo/util/restricted_names.rs +++ b/src/cargo/util/restricted_names.rs @@ -43,6 +43,10 @@ pub fn is_conflicting_artifact_name(name: &str) -> bool { /// elsewhere. `cargo new` has a few restrictions, such as checking for /// reserved names. crates.io has even more restrictions. pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<()> { + if name.is_empty() { + bail!("{what} cannot be empty"); + } + let mut chars = name.chars(); if let Some(ch) = chars.next() { if ch.is_digit(10) { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 6824cb5f5c7..9439ca9179e 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -480,10 +480,6 @@ pub fn to_real_manifest( }; let package_name = package.name.trim(); - if package_name.is_empty() { - bail!("package name cannot be an empty string") - } - validate_package_name(package_name, "package name", "")?; let resolved_path = package_root.join("Cargo.toml"); diff --git a/src/cargo/util_schemas/core/package_id_spec.rs b/src/cargo/util_schemas/core/package_id_spec.rs index b5d2a3db223..07d1fb5ee9b 100644 --- a/src/cargo/util_schemas/core/package_id_spec.rs +++ b/src/cargo/util_schemas/core/package_id_spec.rs @@ -97,9 +97,6 @@ impl PackageIdSpec { Some(version) => Some(version.parse::<PartialVersion>()?), None => None, }; - if name.is_empty() { - bail!("package ID specification must have a name: `{spec}`"); - } validate_package_name(name, "pkgid", "")?; Ok(PackageIdSpec { name: String::from(name), @@ -185,9 +182,6 @@ impl PackageIdSpec { None => (String::from(path_name), None), } }; - if name.is_empty() { - bail!("package ID specification must have a name: `{url}`"); - } validate_package_name(name.as_str(), "pkgid", "")?; Ok(PackageIdSpec { name, diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 250aeb08512..13553671e3c 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -460,7 +460,7 @@ fn cargo_compile_with_empty_package_name() { [ERROR] failed to parse manifest at `[..]` Caused by: - package name cannot be an empty string + package name cannot be empty ", ) .run();