fix: Improve package name errors

src/cargo/util/toml/mod.rs

@@ -23,9 +23,7 @@ use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, Worksp
 use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY};
 use crate::util::errors::{CargoResult, ManifestError};
 use crate::util::interning::InternedString;
-use crate::util::{
-    self, config::ConfigRelativePath, validate_package_name, Config, IntoUrl, OptVersionReq,
-};
+use crate::util::{self, config::ConfigRelativePath, Config, IntoUrl, OptVersionReq};
 use crate::util_schemas::manifest;
 use crate::util_schemas::manifest::RustVersion;
 
@@ -309,9 +307,9 @@ pub fn prepare_for_publish(
 
     fn map_deps(
         config: &Config,
-        deps: Option<&BTreeMap<String, manifest::InheritableDependency>>,
+        deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
         filter: impl Fn(&manifest::TomlDependency) -> bool,
-    ) -> CargoResult<Option<BTreeMap<String, manifest::InheritableDependency>>> {
+    ) -> CargoResult<Option<BTreeMap<manifest::PackageName, manifest::InheritableDependency>>> {
         let Some(deps) = deps else { return Ok(None) };
         let deps = deps
             .iter()
@@ -479,7 +477,6 @@ pub fn to_real_manifest(
     };
 
     let package_name = package.name.trim();
-    validate_package_name(package_name, "package name", "")?;
 
     let resolved_path = package_root.join("Cargo.toml");
 
@@ -627,11 +624,11 @@ pub fn to_real_manifest(
 
     fn process_dependencies(
         cx: &mut Context<'_, '_>,
-        new_deps: Option<&BTreeMap<String, manifest::InheritableDependency>>,
+        new_deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
         kind: Option<DepKind>,
         workspace_config: &WorkspaceConfig,
         inherit_cell: &LazyCell<InheritableFields>,
-    ) -> CargoResult<Option<BTreeMap<String, manifest::InheritableDependency>>> {
+    ) -> CargoResult<Option<BTreeMap<manifest::PackageName, manifest::InheritableDependency>>> {
         let Some(dependencies) = new_deps else {
             return Ok(None);
         };
@@ -642,12 +639,12 @@ pub fn to_real_manifest(
             })
         };
 
-        let mut deps: BTreeMap<String, manifest::InheritableDependency> = BTreeMap::new();
+        let mut deps: BTreeMap<manifest::PackageName, manifest::InheritableDependency> =
+            BTreeMap::new();
         for (n, v) in dependencies.iter() {
             let resolved = dependency_inherit_with(v.clone(), n, inheritable, cx)?;
             let dep = dep_to_dependency(&resolved, n, cx, kind)?;
             let name_in_toml = dep.name_in_toml().as_str();
-            validate_package_name(name_in_toml, "dependency name", "")?;
             let kind_name = match kind {
                 Some(k) => k.kind_table(),
                 None => "dependencies",
@@ -660,7 +657,7 @@ pub fn to_real_manifest(
             unused_dep_keys(name_in_toml, &table_in_toml, v.unused_keys(), cx.warnings);
             cx.deps.push(dep);
             deps.insert(
-                n.to_string(),
+                n.clone(),
                 manifest::InheritableDependency::Value(resolved.clone()),
             );
         }
@@ -1466,7 +1463,7 @@ macro_rules! package_field_getter {
 #[derive(Clone, Debug, Default)]
 pub struct InheritableFields {
     package: Option<manifest::InheritablePackage>,
-    dependencies: Option<BTreeMap<String, manifest::TomlDependency>>,
+    dependencies: Option<BTreeMap<manifest::PackageName, manifest::TomlDependency>>,
     lints: Option<manifest::TomlLints>,
 
     // Bookkeeping to help when resolving values from above

src/cargo/util/toml/targets.rs

@@ -127,7 +127,7 @@ pub(super) fn targets(
         // Verify names match available build deps.
         let bdeps = manifest.build_dependencies.as_ref();
         for name in &metabuild.0 {
-            if !bdeps.map_or(false, |bd| bd.contains_key(name)) {
+            if !bdeps.map_or(false, |bd| bd.contains_key(name.as_str())) {
                 anyhow::bail!(
                     "metabuild package `{}` must be specified in `build-dependencies`",
                     name

src/cargo/util_schemas/manifest.rs

@@ -33,17 +33,17 @@ pub struct TomlManifest {
     pub example: Option<Vec<TomlExampleTarget>>,
     pub test: Option<Vec<TomlTestTarget>>,
     pub bench: Option<Vec<TomlTestTarget>>,
-    pub dependencies: Option<BTreeMap<String, InheritableDependency>>,
-    pub dev_dependencies: Option<BTreeMap<String, InheritableDependency>>,
+    pub dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
+    pub dev_dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
     #[serde(rename = "dev_dependencies")]
-    pub dev_dependencies2: Option<BTreeMap<String, InheritableDependency>>,
-    pub build_dependencies: Option<BTreeMap<String, InheritableDependency>>,
+    pub dev_dependencies2: Option<BTreeMap<PackageName, InheritableDependency>>,
+    pub build_dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
     #[serde(rename = "build_dependencies")]
-    pub build_dependencies2: Option<BTreeMap<String, InheritableDependency>>,
+    pub build_dependencies2: Option<BTreeMap<PackageName, InheritableDependency>>,
     pub features: Option<BTreeMap<FeatureName, Vec<String>>>,
     pub target: Option<BTreeMap<String, TomlPlatform>>,
     pub replace: Option<BTreeMap<String, TomlDependency>>,
-    pub patch: Option<BTreeMap<String, BTreeMap<String, TomlDependency>>>,
+    pub patch: Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
     pub workspace: Option<TomlWorkspace>,
     pub badges: Option<InheritableBtreeMap>,
     pub lints: Option<InheritableLints>,
@@ -59,13 +59,13 @@ impl TomlManifest {
         self.package.as_ref().or(self.project.as_ref())
     }
 
-    pub fn dev_dependencies(&self) -> Option<&BTreeMap<String, InheritableDependency>> {
+    pub fn dev_dependencies(&self) -> Option<&BTreeMap<PackageName, InheritableDependency>> {
         self.dev_dependencies
             .as_ref()
             .or(self.dev_dependencies2.as_ref())
     }
 
-    pub fn build_dependencies(&self) -> Option<&BTreeMap<String, InheritableDependency>> {
+    pub fn build_dependencies(&self) -> Option<&BTreeMap<PackageName, InheritableDependency>> {
         self.build_dependencies
             .as_ref()
             .or(self.build_dependencies2.as_ref())
@@ -87,7 +87,7 @@ pub struct TomlWorkspace {
 
     // Properties that can be inherited by members.
     pub package: Option<InheritablePackage>,
-    pub dependencies: Option<BTreeMap<String, TomlDependency>>,
+    pub dependencies: Option<BTreeMap<PackageName, TomlDependency>>,
     pub lints: Option<TomlLints>,
 }
 
@@ -125,7 +125,7 @@ pub struct InheritablePackage {
 pub struct TomlPackage {
     pub edition: Option<InheritableString>,
     pub rust_version: Option<InheritableRustVersion>,
-    pub name: String,
+    pub name: PackageName,
     pub version: Option<InheritableSemverVersion>,
     pub authors: Option<InheritableVecString>,
     pub build: Option<StringOrBool>,
@@ -592,7 +592,7 @@ pub struct TomlDetailedDependency<P: Clone = String> {
     pub default_features: Option<bool>,
     #[serde(rename = "default_features")]
     pub default_features2: Option<bool>,
-    pub package: Option<String>,
+    pub package: Option<PackageName>,
     pub public: Option<bool>,
 
     /// One or more of `bin`, `cdylib`, `staticlib`, `bin:<name>`.
@@ -1168,6 +1168,15 @@ macro_rules! str_newtype {
     };
 }
 
+str_newtype!(PackageName);
+
+impl<T: AsRef<str>> PackageName<T> {
+    pub fn new(name: T) -> Result<Self> {
+        restricted_names::validate_package_name(name.as_ref(), "package name", "")?;
+        Ok(Self(name))
+    }
+}
+
 str_newtype!(ProfileName);
 
 impl<T: AsRef<str>> ProfileName<T> {
@@ -1190,23 +1199,23 @@ impl<T: AsRef<str>> FeatureName<T> {
 #[derive(Serialize, Deserialize, Debug, Clone)]
 #[serde(rename_all = "kebab-case")]
 pub struct TomlPlatform {
-    pub dependencies: Option<BTreeMap<String, InheritableDependency>>,
-    pub build_dependencies: Option<BTreeMap<String, InheritableDependency>>,
+    pub dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
+    pub build_dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
     #[serde(rename = "build_dependencies")]
-    pub build_dependencies2: Option<BTreeMap<String, InheritableDependency>>,
-    pub dev_dependencies: Option<BTreeMap<String, InheritableDependency>>,
+    pub build_dependencies2: Option<BTreeMap<PackageName, InheritableDependency>>,
+    pub dev_dependencies: Option<BTreeMap<PackageName, InheritableDependency>>,
     #[serde(rename = "dev_dependencies")]
-    pub dev_dependencies2: Option<BTreeMap<String, InheritableDependency>>,
+    pub dev_dependencies2: Option<BTreeMap<PackageName, InheritableDependency>>,
 }
 
 impl TomlPlatform {
-    pub fn dev_dependencies(&self) -> Option<&BTreeMap<String, InheritableDependency>> {
+    pub fn dev_dependencies(&self) -> Option<&BTreeMap<PackageName, InheritableDependency>> {
         self.dev_dependencies
             .as_ref()
             .or(self.dev_dependencies2.as_ref())
     }
 
-    pub fn build_dependencies(&self) -> Option<&BTreeMap<String, InheritableDependency>> {
+    pub fn build_dependencies(&self) -> Option<&BTreeMap<PackageName, InheritableDependency>> {
         self.build_dependencies
             .as_ref()
             .or(self.build_dependencies2.as_ref())

tests/testsuite/build.rs

@@ -460,6 +460,10 @@ fn cargo_compile_with_empty_package_name() {
 [ERROR] failed to parse manifest at `[..]`
 
 Caused by:
+  TOML parse error at line 3, column 16
+    |
+  3 |         name = \"\"
+    |                ^^
   package name cannot be empty
 ",
         )
@@ -479,6 +483,10 @@ fn cargo_compile_with_invalid_package_name() {
 [ERROR] failed to parse manifest at `[..]`
 
 Caused by:
+  TOML parse error at line 3, column 16
+    |
+  3 |         name = \"foo::bar\"
+    |                ^^^^^^^^^^
   invalid character `:` in package name: `foo::bar`, [..]
 ",
         )
@@ -1182,7 +1190,11 @@ fn cargo_compile_with_invalid_dep_rename() {
 error: failed to parse manifest at `[..]`
 
 Caused by:
-  invalid character ` ` in dependency name: `haha this isn't a valid name 🐛`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)
+  TOML parse error at line 7, column 17
+    |
+  7 |                 \"haha this isn't a valid name 🐛\" = { package = \"libc\", version = \"0.1\" }
+    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  invalid character ` ` in package name: `haha this isn't a valid name 🐛`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)
 ",
         )
         .run();