diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 8dfdf0dbfb4..2137d633250 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -1,6 +1,7 @@ use crate::core::{Dependency, PackageId, SourceId}; use crate::util::interning::InternedString; use crate::util::CargoResult; +use crate::util_schemas::manifest::FeatureName; use crate::util_schemas::manifest::RustVersion; use anyhow::bail; use semver::Version; @@ -49,7 +50,7 @@ impl Summary { ) } } - let feature_map = build_feature_map(pkg_id, features, &dependencies)?; + let feature_map = build_feature_map(features, &dependencies)?; Ok(Summary { inner: Rc::new(Inner { package_id: pkg_id, @@ -140,7 +141,6 @@ impl Hash for Summary { /// Checks features for errors, bailing out a CargoResult:Err if invalid, /// and creates FeatureValues for each feature. fn build_feature_map( - pkg_id: PackageId, features: &BTreeMap<InternedString, Vec<InternedString>>, dependencies: &[Dependency], ) -> CargoResult<FeatureMap> { @@ -191,19 +191,7 @@ fn build_feature_map( // Validate features are listed properly. for (feature, fvs) in &map { - if feature.starts_with("dep:") { - bail!( - "feature named `{}` is not allowed to start with `dep:`", - feature - ); - } - if feature.contains('/') { - bail!( - "feature named `{}` is not allowed to contain slashes", - feature - ); - } - validate_feature_name(pkg_id, feature)?; + FeatureName::new(feature)?; for fv in fvs { // Find data for the referenced dependency... let dep_data = { @@ -429,68 +417,3 @@ impl fmt::Display for FeatureValue { } pub type FeatureMap = BTreeMap<InternedString, Vec<FeatureValue>>; - -fn validate_feature_name(pkg_id: PackageId, name: &str) -> CargoResult<()> { - if name.is_empty() { - bail!("feature name cannot be empty"); - } - let mut chars = name.chars(); - if let Some(ch) = chars.next() { - if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' || ch.is_digit(10)) { - bail!( - "invalid character `{}` in feature `{}` in package {}, \ - the first character must be a Unicode XID start character or digit \ - (most letters or `_` or `0` to `9`)", - ch, - name, - pkg_id - ); - } - } - for ch in chars { - if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' || ch == '+' || ch == '.') { - bail!( - "invalid character `{}` in feature `{}` in package {}, \ - characters must be Unicode XID characters, '-', `+`, or `.` \ - (numbers, `+`, `-`, `_`, `.`, or most letters)", - ch, - name, - pkg_id - ); - } - } - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::sources::CRATES_IO_INDEX; - use crate::util::into_url::IntoUrl; - - use crate::core::SourceId; - - #[test] - fn valid_feature_names() { - let loc = CRATES_IO_INDEX.into_url().unwrap(); - let source_id = SourceId::for_registry(&loc).unwrap(); - let pkg_id = PackageId::try_new("foo", "1.0.0", source_id).unwrap(); - - assert!(validate_feature_name(pkg_id, "c++17").is_ok()); - assert!(validate_feature_name(pkg_id, "128bit").is_ok()); - assert!(validate_feature_name(pkg_id, "_foo").is_ok()); - assert!(validate_feature_name(pkg_id, "feat-name").is_ok()); - assert!(validate_feature_name(pkg_id, "feat_name").is_ok()); - assert!(validate_feature_name(pkg_id, "foo.bar").is_ok()); - - assert!(validate_feature_name(pkg_id, "+foo").is_err()); - assert!(validate_feature_name(pkg_id, "-foo").is_err()); - assert!(validate_feature_name(pkg_id, ".foo").is_err()); - assert!(validate_feature_name(pkg_id, "foo:bar").is_err()); - assert!(validate_feature_name(pkg_id, "foo?").is_err()); - assert!(validate_feature_name(pkg_id, "?foo").is_err()); - assert!(validate_feature_name(pkg_id, "ⒶⒷⒸ").is_err()); - assert!(validate_feature_name(pkg_id, "a¼").is_err()); - assert!(validate_feature_name(pkg_id, "").is_err()); - } -} diff --git a/src/cargo/ops/cargo_add/crate_spec.rs b/src/cargo/ops/cargo_add/crate_spec.rs index 4cf6f484a32..65c58314f65 100644 --- a/src/cargo/ops/cargo_add/crate_spec.rs +++ b/src/cargo/ops/cargo_add/crate_spec.rs @@ -4,7 +4,7 @@ use anyhow::Context as _; use super::Dependency; use crate::util::toml_mut::dependency::RegistrySource; -use crate::util::validate_package_name; +use crate::util_schemas::manifest::PackageName; use crate::CargoResult; /// User-specified crate @@ -28,7 +28,7 @@ impl CrateSpec { .map(|(n, v)| (n, Some(v))) .unwrap_or((pkg_id, None)); - validate_package_name(name, "dependency name", "")?; + PackageName::new(name)?; if let Some(version) = version { semver::VersionReq::parse(version) diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index 1c06b5f8258..57c7e268e3a 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -4,6 +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 crate::util_schemas::manifest::PackageName; use anyhow::{anyhow, Context}; use cargo_util::paths::{self, write_atomic}; use serde::de; @@ -180,7 +181,7 @@ fn check_name( }; let bin_help = || { let mut help = String::from(name_help); - if has_bin { + if has_bin && !name.is_empty() { help.push_str(&format!( "\n\ If you need a binary with the name \"{name}\", use a valid package \ @@ -197,7 +198,10 @@ fn check_name( } help }; - restricted_names::validate_package_name(name, "package name", &bin_help())?; + PackageName::new(name).map_err(|err| { + let help = bin_help(); + anyhow::anyhow!("{err}{help}") + })?; if restricted_names::is_keyword(name) { anyhow::bail!( diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 3e236a6f735..24c8c098dde 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -11,6 +11,8 @@ use crate::util::{ print_available_benches, print_available_binaries, print_available_examples, print_available_packages, print_available_tests, }; +use crate::util_schemas::manifest::ProfileName; +use crate::util_schemas::manifest::RegistryName; use crate::util_schemas::manifest::StringOrVec; use crate::CargoResult; use anyhow::bail; @@ -605,7 +607,7 @@ Run `{cmd}` to see possible targets." bail!("profile `doc` is reserved and not allowed to be explicitly specified") } (_, _, Some(name)) => { - restricted_names::validate_profile_name(name)?; + ProfileName::new(name)?; name } }; @@ -833,7 +835,7 @@ 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) => { - restricted_names::validate_package_name(r, "registry name", "")?; + RegistryName::new(r)?; Some(RegistryOrIndex::Registry(r.to_string())) } (Some(_), Some(_)) => { @@ -848,7 +850,7 @@ Run `{cmd}` to see possible targets." match self._value_of("registry").map(|s| s.to_string()) { None => config.default_registry(), Some(registry) => { - restricted_names::validate_package_name(®istry, "registry name", "")?; + RegistryName::new(®istry)?; Ok(Some(registry)) } } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index c0dd42d39b8..1c1b949a783 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -77,9 +77,10 @@ use crate::sources::CRATES_IO_REGISTRY; use crate::util::errors::CargoResult; use crate::util::network::http::configure_http_handle; use crate::util::network::http::http_handle; +use crate::util::try_canonicalize; use crate::util::{internal, CanonicalUrl}; -use crate::util::{try_canonicalize, validate_package_name}; use crate::util::{Filesystem, IntoUrl, IntoUrlWithBase, Rustc}; +use crate::util_schemas::manifest::RegistryName; use anyhow::{anyhow, bail, format_err, Context as _}; use cargo_credential::Secret; use cargo_util::paths; @@ -1551,7 +1552,7 @@ impl Config { /// Gets the index for a registry. pub fn get_registry_index(&self, registry: &str) -> CargoResult<Url> { - validate_package_name(registry, "registry name", "")?; + RegistryName::new(registry)?; if let Some(index) = self.get_string(&format!("registries.{}.index", registry))? { self.resolve_registry_index(&index).with_context(|| { format!( diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 16c15770fe4..e5ecd077fd7 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -21,7 +21,6 @@ pub(crate) use self::io::LimitErrorReader; pub use self::lockserver::{LockServer, LockServerClient, LockServerStarted}; pub use self::progress::{Progress, ProgressStyle}; pub use self::queue::Queue; -pub use self::restricted_names::validate_package_name; pub use self::rustc::Rustc; pub use self::semver_ext::OptVersionReq; pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo}; diff --git a/src/cargo/util/restricted_names.rs b/src/cargo/util/restricted_names.rs index df0152ecb1e..f047b0d6656 100644 --- a/src/cargo/util/restricted_names.rs +++ b/src/cargo/util/restricted_names.rs @@ -1,7 +1,5 @@ //! Helpers for validating and checking names like package and crate names. -use crate::util::CargoResult; -use anyhow::bail; use std::path::Path; /// Returns `true` if the name contains non-ASCII characters. @@ -36,80 +34,6 @@ pub fn is_conflicting_artifact_name(name: &str) -> bool { ["deps", "examples", "build", "incremental"].contains(&name) } -/// Check the base requirements for a package name. -/// -/// This can be used for other things than package names, to enforce some -/// level of sanity. Note that package names have other restrictions -/// 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) { - // A specific error for a potentially common case. - bail!( - "the name `{}` cannot be used as a {}, \ - the name cannot start with a digit{}", - name, - what, - help - ); - } - if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') { - bail!( - "invalid character `{}` in {}: `{}`, \ - the first character must be a Unicode XID start character \ - (most letters or `_`){}", - ch, - what, - name, - help - ); - } - } - for ch in chars { - if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') { - bail!( - "invalid character `{}` in {}: `{}`, \ - characters must be Unicode XID characters \ - (numbers, `-`, `_`, or most letters){}", - ch, - what, - name, - help - ); - } - } - Ok(()) -} - -/// Ensure a package name is [valid][validate_package_name] -pub fn sanitize_package_name(name: &str, placeholder: char) -> String { - let mut slug = String::new(); - let mut chars = name.chars(); - while let Some(ch) = chars.next() { - if (unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') && !ch.is_digit(10) { - slug.push(ch); - break; - } - } - while let Some(ch) = chars.next() { - if unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' { - slug.push(ch); - } else { - slug.push(placeholder); - } - } - if slug.is_empty() { - slug.push_str("package"); - } - slug -} - /// Check the entire path for names reserved in Windows. pub fn is_windows_reserved_path(path: &Path) -> bool { path.iter() @@ -124,82 +48,3 @@ pub fn is_windows_reserved_path(path: &Path) -> bool { pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool { name.as_ref().contains(&['*', '?', '[', ']'][..]) } - -/// Validate dir-names and profile names according to RFC 2678. -pub fn validate_profile_name(name: &str) -> CargoResult<()> { - if let Some(ch) = name - .chars() - .find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-') - { - bail!( - "invalid character `{}` in profile name `{}`\n\ - Allowed characters are letters, numbers, underscore, and hyphen.", - ch, - name - ); - } - - const SEE_DOCS: &str = "See https://doc.rust-lang.org/cargo/reference/profiles.html \ - for more on configuring profiles."; - - let lower_name = name.to_lowercase(); - if lower_name == "debug" { - bail!( - "profile name `{}` is reserved\n\ - To configure the default development profile, use the name `dev` \ - as in [profile.dev]\n\ - {}", - name, - SEE_DOCS - ); - } - if lower_name == "build-override" { - bail!( - "profile name `{}` is reserved\n\ - To configure build dependency settings, use [profile.dev.build-override] \ - and [profile.release.build-override]\n\ - {}", - name, - SEE_DOCS - ); - } - - // These are some arbitrary reservations. We have no plans to use - // these, but it seems safer to reserve a few just in case we want to - // add more built-in profiles in the future. We can also uses special - // syntax like cargo:foo if needed. But it is unlikely these will ever - // be used. - if matches!( - lower_name.as_str(), - "build" - | "check" - | "clean" - | "config" - | "fetch" - | "fix" - | "install" - | "metadata" - | "package" - | "publish" - | "report" - | "root" - | "run" - | "rust" - | "rustc" - | "rustdoc" - | "target" - | "tmp" - | "uninstall" - ) || lower_name.starts_with("cargo") - { - bail!( - "profile name `{}` is reserved\n\ - Please choose a different name.\n\ - {}", - name, - SEE_DOCS - ); - } - - Ok(()) -} diff --git a/src/cargo/util/toml/embedded.rs b/src/cargo/util/toml/embedded.rs index 4c57195d4f4..cad7abc38fa 100644 --- a/src/cargo/util/toml/embedded.rs +++ b/src/cargo/util/toml/embedded.rs @@ -1,6 +1,7 @@ use anyhow::Context as _; use crate::util::restricted_names; +use crate::util_schemas::manifest::PackageName; use crate::CargoResult; use crate::Config; @@ -171,7 +172,7 @@ fn sanitize_name(name: &str) -> String { '-' }; - let mut name = restricted_names::sanitize_package_name(name, placeholder); + let mut name = PackageName::sanitize(name, placeholder).into_inner(); loop { if restricted_names::is_keyword(&name) { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9439ca9179e..8affc69a44b 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -23,10 +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::restricted_names; -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; @@ -310,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() @@ -480,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"); @@ -628,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); }; @@ -643,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", @@ -661,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()), ); } @@ -1467,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 @@ -2020,9 +2016,6 @@ pub fn validate_profile( } } - // Profile name validation - restricted_names::validate_profile_name(name)?; - if let Some(dir_name) = &root.dir_name { // This is disabled for now, as we would like to stabilize named // profiles without this, and then decide in the future if it is diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 3659fd74c37..7d0f1891e15 100644 --- a/src/cargo/util/toml/targets.rs +++ b/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 diff --git a/src/cargo/util_schemas/core/package_id_spec.rs b/src/cargo/util_schemas/core/package_id_spec.rs index 07d1fb5ee9b..015bfa928d7 100644 --- a/src/cargo/util_schemas/core/package_id_spec.rs +++ b/src/cargo/util_schemas/core/package_id_spec.rs @@ -6,9 +6,9 @@ use semver::Version; use serde::{de, ser}; use url::Url; -use crate::util::validate_package_name; use crate::util_schemas::core::GitReference; use crate::util_schemas::core::SourceKind; +use crate::util_schemas::manifest::PackageName; use crate::util_semver::PartialVersion; /// Some or all of the data required to identify a package: @@ -97,7 +97,7 @@ impl PackageIdSpec { Some(version) => Some(version.parse::<PartialVersion>()?), None => None, }; - validate_package_name(name, "pkgid", "")?; + PackageName::new(name)?; Ok(PackageIdSpec { name: String::from(name), version, @@ -182,7 +182,7 @@ impl PackageIdSpec { None => (String::from(path_name), None), } }; - validate_package_name(name.as_str(), "pkgid", "")?; + PackageName::new(&name)?; Ok(PackageIdSpec { name, version, diff --git a/src/cargo/util_schemas/manifest.rs b/src/cargo/util_schemas/manifest.rs index b0aa6393f6f..390658b0e46 100644 --- a/src/cargo/util_schemas/manifest.rs +++ b/src/cargo/util_schemas/manifest.rs @@ -10,12 +10,14 @@ use std::fmt::{self, Display, Write}; use std::path::PathBuf; use std::str; +use anyhow::Result; use serde::de::{self, IntoDeserializer as _, Unexpected}; use serde::ser; use serde::{Deserialize, Serialize}; use serde_untagged::UntaggedEnumVisitor; use crate::util_schemas::core::PackageIdSpec; +use crate::util_schemas::restricted_names; use crate::util_semver::PartialVersion; /// This type is used to deserialize `Cargo.toml` files. @@ -31,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 features: Option<BTreeMap<String, Vec<String>>>, + 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>, @@ -57,19 +59,19 @@ 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()) } - pub fn features(&self) -> Option<&BTreeMap<String, Vec<String>>> { + pub fn features(&self) -> Option<&BTreeMap<FeatureName, Vec<String>>> { self.features.as_ref() } } @@ -85,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>, } @@ -123,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>, @@ -570,7 +572,7 @@ impl<'de, P: Deserialize<'de> + Clone> de::Deserialize<'de> for TomlDependency<P #[serde(rename_all = "kebab-case")] pub struct TomlDetailedDependency<P: Clone = String> { pub version: Option<String>, - pub registry: Option<String>, + pub registry: Option<RegistryName>, /// The URL of the `registry` field. /// This is an internal implementation detail. When Cargo creates a /// package, it replaces `registry` with `registry-index` so that the @@ -590,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>`. @@ -639,10 +641,10 @@ impl<P: Clone> Default for TomlDetailedDependency<P> { } #[derive(Deserialize, Serialize, Clone, Debug, Default)] -pub struct TomlProfiles(pub BTreeMap<String, TomlProfile>); +pub struct TomlProfiles(pub BTreeMap<ProfileName, TomlProfile>); impl TomlProfiles { - pub fn get_all(&self) -> &BTreeMap<String, TomlProfile> { + pub fn get_all(&self) -> &BTreeMap<ProfileName, TomlProfile> { &self.0 } @@ -1108,27 +1110,138 @@ impl TomlTarget { } } +macro_rules! str_newtype { + ($name:ident) => { + /// Verified string newtype + #[derive(Serialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] + #[serde(transparent)] + pub struct $name<T: AsRef<str> = String>(T); + + impl<T: AsRef<str>> $name<T> { + pub fn into_inner(self) -> T { + self.0 + } + } + + impl<T: AsRef<str>> AsRef<str> for $name<T> { + fn as_ref(&self) -> &str { + self.0.as_ref() + } + } + + impl<T: AsRef<str>> std::ops::Deref for $name<T> { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl<T: AsRef<str>> std::borrow::Borrow<str> for $name<T> { + fn borrow(&self) -> &str { + self.0.as_ref() + } + } + + impl<'a> std::str::FromStr for $name<String> { + type Err = anyhow::Error; + + fn from_str(value: &str) -> Result<Self, Self::Err> { + Self::new(value.to_owned()) + } + } + + impl<'de, T: AsRef<str> + serde::Deserialize<'de>> serde::Deserialize<'de> for $name<T> { + fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> + where + D: serde::Deserializer<'de>, + { + let inner = T::deserialize(deserializer)?; + Self::new(inner).map_err(serde::de::Error::custom) + } + } + + impl<T: AsRef<str>> Display for $name<T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.as_ref().fmt(f) + } + } + }; +} + +str_newtype!(PackageName); + +impl<T: AsRef<str>> PackageName<T> { + /// Validated package name + pub fn new(name: T) -> Result<Self> { + restricted_names::validate_package_name(name.as_ref(), "package name", "")?; + Ok(Self(name)) + } +} + +impl PackageName { + /// Coerce a value to be a validate package name + /// + /// Replaces invalid values with `placeholder` + pub fn sanitize(name: impl AsRef<str>, placeholder: char) -> Self { + PackageName(restricted_names::sanitize_package_name( + name.as_ref(), + placeholder, + )) + } +} + +str_newtype!(RegistryName); + +impl<T: AsRef<str>> RegistryName<T> { + /// Validated registry name + pub fn new(name: T) -> Result<Self> { + restricted_names::validate_package_name(name.as_ref(), "registry name", "")?; + Ok(Self(name)) + } +} + +str_newtype!(ProfileName); + +impl<T: AsRef<str>> ProfileName<T> { + /// Validated profile name + pub fn new(name: T) -> Result<Self> { + restricted_names::validate_profile_name(name.as_ref())?; + Ok(Self(name)) + } +} + +str_newtype!(FeatureName); + +impl<T: AsRef<str>> FeatureName<T> { + /// Validated feature name + pub fn new(name: T) -> Result<Self> { + restricted_names::validate_feature_name(name.as_ref())?; + Ok(Self(name)) + } +} + /// Corresponds to a `target` entry, but `TomlTarget` is already used. #[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()) diff --git a/src/cargo/util_schemas/mod.rs b/src/cargo/util_schemas/mod.rs index a2d0a0736a8..84b6c39a89b 100644 --- a/src/cargo/util_schemas/mod.rs +++ b/src/cargo/util_schemas/mod.rs @@ -7,3 +7,5 @@ pub mod core; pub mod manifest; + +mod restricted_names; diff --git a/src/cargo/util_schemas/restricted_names.rs b/src/cargo/util_schemas/restricted_names.rs new file mode 100644 index 00000000000..2d22ce4f2f7 --- /dev/null +++ b/src/cargo/util_schemas/restricted_names.rs @@ -0,0 +1,218 @@ +//! Helpers for validating and checking names like package and crate names. + +use anyhow::bail; +use anyhow::Result; + +/// Check the base requirements for a package name. +/// +/// This can be used for other things than package names, to enforce some +/// level of sanity. Note that package names have other restrictions +/// 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) -> Result<()> { + 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) { + // A specific error for a potentially common case. + bail!( + "the name `{}` cannot be used as a {}, \ + the name cannot start with a digit{}", + name, + what, + help + ); + } + if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') { + bail!( + "invalid character `{}` in {}: `{}`, \ + the first character must be a Unicode XID start character \ + (most letters or `_`){}", + ch, + what, + name, + help + ); + } + } + for ch in chars { + if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-') { + bail!( + "invalid character `{}` in {}: `{}`, \ + characters must be Unicode XID characters \ + (numbers, `-`, `_`, or most letters){}", + ch, + what, + name, + help + ); + } + } + Ok(()) +} + +/// Ensure a package name is [valid][validate_package_name] +pub fn sanitize_package_name(name: &str, placeholder: char) -> String { + let mut slug = String::new(); + let mut chars = name.chars(); + while let Some(ch) = chars.next() { + if (unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_') && !ch.is_digit(10) { + slug.push(ch); + break; + } + } + while let Some(ch) = chars.next() { + if unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' { + slug.push(ch); + } else { + slug.push(placeholder); + } + } + if slug.is_empty() { + slug.push_str("package"); + } + slug +} + +/// Validate dir-names and profile names according to RFC 2678. +pub fn validate_profile_name(name: &str) -> Result<()> { + if let Some(ch) = name + .chars() + .find(|ch| !ch.is_alphanumeric() && *ch != '_' && *ch != '-') + { + bail!( + "invalid character `{}` in profile name `{}`\n\ + Allowed characters are letters, numbers, underscore, and hyphen.", + ch, + name + ); + } + + const SEE_DOCS: &str = "See https://doc.rust-lang.org/cargo/reference/profiles.html \ + for more on configuring profiles."; + + let lower_name = name.to_lowercase(); + if lower_name == "debug" { + bail!( + "profile name `{}` is reserved\n\ + To configure the default development profile, use the name `dev` \ + as in [profile.dev]\n\ + {}", + name, + SEE_DOCS + ); + } + if lower_name == "build-override" { + bail!( + "profile name `{}` is reserved\n\ + To configure build dependency settings, use [profile.dev.build-override] \ + and [profile.release.build-override]\n\ + {}", + name, + SEE_DOCS + ); + } + + // These are some arbitrary reservations. We have no plans to use + // these, but it seems safer to reserve a few just in case we want to + // add more built-in profiles in the future. We can also uses special + // syntax like cargo:foo if needed. But it is unlikely these will ever + // be used. + if matches!( + lower_name.as_str(), + "build" + | "check" + | "clean" + | "config" + | "fetch" + | "fix" + | "install" + | "metadata" + | "package" + | "publish" + | "report" + | "root" + | "run" + | "rust" + | "rustc" + | "rustdoc" + | "target" + | "tmp" + | "uninstall" + ) || lower_name.starts_with("cargo") + { + bail!( + "profile name `{}` is reserved\n\ + Please choose a different name.\n\ + {}", + name, + SEE_DOCS + ); + } + + Ok(()) +} + +pub fn validate_feature_name(name: &str) -> Result<()> { + if name.is_empty() { + bail!("feature name cannot be empty"); + } + + if name.starts_with("dep:") { + bail!("feature named `{name}` is not allowed to start with `dep:`",); + } + if name.contains('/') { + bail!("feature named `{name}` is not allowed to contain slashes",); + } + let mut chars = name.chars(); + if let Some(ch) = chars.next() { + if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' || ch.is_digit(10)) { + bail!( + "invalid character `{ch}` in feature `{name}`, \ + the first character must be a Unicode XID start character or digit \ + (most letters or `_` or `0` to `9`)", + ); + } + } + for ch in chars { + if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' || ch == '+' || ch == '.') { + bail!( + "invalid character `{ch}` in feature `{name}`, \ + characters must be Unicode XID characters, '-', `+`, or `.` \ + (numbers, `+`, `-`, `_`, `.`, or most letters)", + ); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn valid_feature_names() { + assert!(validate_feature_name("c++17").is_ok()); + assert!(validate_feature_name("128bit").is_ok()); + assert!(validate_feature_name("_foo").is_ok()); + assert!(validate_feature_name("feat-name").is_ok()); + assert!(validate_feature_name("feat_name").is_ok()); + assert!(validate_feature_name("foo.bar").is_ok()); + + assert!(validate_feature_name("").is_err()); + assert!(validate_feature_name("+foo").is_err()); + assert!(validate_feature_name("-foo").is_err()); + assert!(validate_feature_name(".foo").is_err()); + assert!(validate_feature_name("dep:bar").is_err()); + assert!(validate_feature_name("foo/bar").is_err()); + assert!(validate_feature_name("foo:bar").is_err()); + assert!(validate_feature_name("foo?").is_err()); + assert!(validate_feature_name("?foo").is_err()); + assert!(validate_feature_name("ⒶⒷⒸ").is_err()); + assert!(validate_feature_name("a¼").is_err()); + assert!(validate_feature_name("").is_err()); + } +} diff --git a/tests/testsuite/alt_registry.rs b/tests/testsuite/alt_registry.rs index 8982b1941a7..e347af1c708 100644 --- a/tests/testsuite/alt_registry.rs +++ b/tests/testsuite/alt_registry.rs @@ -715,7 +715,14 @@ fn bad_registry_name() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - invalid character ` ` in registry name: `bad name`, [..]", + TOML parse error at line 7, column 17 + | + 7 | [dependencies.bar] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + invalid character ` ` in registry name: `bad name`, [..] + + +", ) .run(); @@ -1618,7 +1625,14 @@ fn empty_dependency_registry() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - registry name cannot be empty", + TOML parse error at line 7, column 23 + | + 7 | bar = { version = \"0.1.0\", registry = \"\" } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + registry name cannot be empty + + +", ) .run(); } diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 13553671e3c..dd67161d6b1 100644 --- a/tests/testsuite/build.rs +++ b/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(); diff --git a/tests/testsuite/cargo_add/empty_dep_name/stderr.log b/tests/testsuite/cargo_add/empty_dep_name/stderr.log index 1bb057cd14d..d9547a42acf 100644 --- a/tests/testsuite/cargo_add/empty_dep_name/stderr.log +++ b/tests/testsuite/cargo_add/empty_dep_name/stderr.log @@ -1 +1 @@ -error: dependency name cannot be empty +error: package name cannot be empty diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index 7086c3a037a..febdf52fed8 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -60,6 +60,10 @@ fn empty_feature_name() { [ERROR] failed to parse manifest at `[..]` Caused by: + TOML parse error at line 8, column 17 + | + 8 | \"\" = [] + | ^^ feature name cannot be empty ", ) @@ -2055,7 +2059,11 @@ fn invalid_feature_names_error() { error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: - invalid character `+` in feature `+foo` in package foo v0.1.0 ([ROOT]/foo), \ + TOML parse error at line 8, column 17 + | + 8 | \"+foo\" = [] + | ^^^^^^ + invalid character `+` in feature `+foo`, \ the first character must be a Unicode XID start character or digit \ (most letters or `_` or `0` to `9`) ", @@ -2082,7 +2090,11 @@ Caused by: error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: - invalid character `&` in feature `a&b` in package foo v0.1.0 ([ROOT]/foo), \ + TOML parse error at line 8, column 13 + | + 8 | \"a&b\" = [] + | ^^^^^ + invalid character `&` in feature `a&b`, \ characters must be Unicode XID characters, '-', `+`, or `.` \ (numbers, `+`, `-`, `_`, `.`, or most letters) ", @@ -2115,6 +2127,10 @@ fn invalid_feature_name_slash_error() { error: failed to parse manifest at `[CWD]/Cargo.toml` Caused by: + TOML parse error at line 7, column 17 + | + 7 | \"foo/bar\" = [] + | ^^^^^^^^^ feature named `foo/bar` is not allowed to contain slashes ", ) diff --git a/tests/testsuite/features_namespaced.rs b/tests/testsuite/features_namespaced.rs index f24186c1591..b79be55e83c 100644 --- a/tests/testsuite/features_namespaced.rs +++ b/tests/testsuite/features_namespaced.rs @@ -439,6 +439,10 @@ fn crate_syntax_bad_name() { [ERROR] failed to parse manifest at [..]/foo/Cargo.toml` Caused by: + TOML parse error at line 10, column 17 + | + 10 | \"dep:bar\" = [] + | ^^^^^^^^^ feature named `dep:bar` is not allowed to start with `dep:` ", ) diff --git a/tests/testsuite/profile_custom.rs b/tests/testsuite/profile_custom.rs index f7139e55267..cf9828d3794 100644 --- a/tests/testsuite/profile_custom.rs +++ b/tests/testsuite/profile_custom.rs @@ -86,6 +86,10 @@ fn invalid_profile_name() { [ERROR] failed to parse manifest at [..] Caused by: + TOML parse error at line 7, column 26 + | + 7 | [profile.'.release-lto'] + | ^^^^^^^^^^^^^^ invalid character `.` in profile name `.release-lto` Allowed characters are letters, numbers, underscore, and hyphen. ", @@ -626,6 +630,7 @@ See https://doc.rust-lang.org/cargo/reference/profiles.html for more on configur ), ); + let highlight = "^".repeat(name.len()); p.cargo("build") .with_status(101) .with_stderr(&format!( @@ -633,11 +638,14 @@ See https://doc.rust-lang.org/cargo/reference/profiles.html for more on configur error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: - profile name `{}` is reserved + TOML parse error at line 6, column 30 + | + 6 | [profile.{name}] + | {highlight} + profile name `{name}` is reserved Please choose a different name. See https://doc.rust-lang.org/cargo/reference/profiles.html for more on configuring profiles. ", - name )) .run(); } @@ -663,6 +671,10 @@ Caused by: error: failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: + TOML parse error at line 7, column 25 + | + 7 | [profile.debug] + | ^^^^^ profile name `debug` is reserved To configure the default development profile, use the name `dev` as in [profile.dev] See https://doc.rust-lang.org/cargo/reference/profiles.html for more on configuring profiles.