diff --git a/crates/distribution-filename/src/source_distribution.rs b/crates/distribution-filename/src/source_distribution.rs index 1bd76774e..443a7e12a 100644 --- a/crates/distribution-filename/src/source_distribution.rs +++ b/crates/distribution-filename/src/source_distribution.rs @@ -4,7 +4,7 @@ use std::str::FromStr; use thiserror::Error; use pep440_rs::Version; -use puffin_normalize::PackageName; +use puffin_normalize::{InvalidNameError, PackageName}; #[derive(Clone, Debug, PartialEq, Eq)] pub enum SourceDistributionExtension { @@ -71,8 +71,12 @@ impl SourceDistributionFilename { )); }; + let actual_package_name = PackageName::from_str(&stem[..package_name.as_ref().len()]) + .map_err(|err| { + SourceDistributionFilenameError::InvalidPackageName(filename.to_string(), err) + })?; if stem.len() <= package_name.as_ref().len() + "-".len() - || &PackageName::new(&stem[..package_name.as_ref().len()]) != package_name + || &actual_package_name != package_name { return Err(SourceDistributionFilenameError::InvalidFilename { filename: filename.to_string(), @@ -109,11 +113,14 @@ pub enum SourceDistributionFilenameError { InvalidExtension(String), #[error("Source distribution filename version section is invalid: {0}")] InvalidVersion(String), + #[error("Source distribution filename has an invalid package name: {0}")] + InvalidPackageName(String, #[source] InvalidNameError), } #[cfg(test)] mod tests { use puffin_normalize::PackageName; + use std::str::FromStr; use crate::SourceDistributionFilename; @@ -126,9 +133,12 @@ mod tests { "foo-lib-1.2.3.tar.gz", ] { assert_eq!( - SourceDistributionFilename::parse(normalized, &PackageName::new("foo_lib")) - .unwrap() - .to_string(), + SourceDistributionFilename::parse( + normalized, + &PackageName::from_str("foo_lib").unwrap() + ) + .unwrap() + .to_string(), normalized ); } @@ -137,7 +147,11 @@ mod tests { #[test] fn errors() { for invalid in ["b-1.2.3.zip", "a-1.2.3-gamma.3.zip", "a-1.2.3.tar.zstd"] { - assert!(SourceDistributionFilename::parse(invalid, &PackageName::new("a")).is_err()); + assert!(SourceDistributionFilename::parse( + invalid, + &PackageName::from_str("a").unwrap() + ) + .is_err()); } } } diff --git a/crates/distribution-filename/src/wheel.rs b/crates/distribution-filename/src/wheel.rs index eee7946b2..f683a9ca8 100644 --- a/crates/distribution-filename/src/wheel.rs +++ b/crates/distribution-filename/src/wheel.rs @@ -6,7 +6,7 @@ use url::Url; use pep440_rs::Version; use platform_tags::Tags; -use puffin_normalize::PackageName; +use puffin_normalize::{InvalidNameError, PackageName}; #[derive(Debug, Clone, Eq, PartialEq)] pub struct WheelFilename { @@ -87,10 +87,12 @@ impl FromStr for WheelFilename { ) }; + let distribution = PackageName::from_str(distribution) + .map_err(|err| WheelFilenameError::InvalidPackageName(filename.to_string(), err))?; let version = Version::from_str(version) .map_err(|err| WheelFilenameError::InvalidVersion(filename.to_string(), err))?; Ok(WheelFilename { - distribution: PackageName::new(distribution), + distribution, version, python_tag: python_tag.split('.').map(String::from).collect(), abi_tag: abi_tag.split('.').map(String::from).collect(), @@ -165,4 +167,6 @@ pub enum WheelFilenameError { InvalidWheelFileName(String, String), #[error("The wheel filename \"{0}\" has an invalid version part: {1}")] InvalidVersion(String, String), + #[error("The wheel filename \"{0}\" has an invalid package name")] + InvalidPackageName(String, InvalidNameError), } diff --git a/crates/pep508-rs/src/lib.rs b/crates/pep508-rs/src/lib.rs index 4482e9654..a1ad7c3d2 100644 --- a/crates/pep508-rs/src/lib.rs +++ b/crates/pep508-rs/src/lib.rs @@ -11,7 +11,7 @@ //! let marker = r#"requests [security,tests] >= 2.8.1, == 2.8.* ; python_version > "3.8""#; //! let dependency_specification = Requirement::from_str(marker).unwrap(); //! assert_eq!(dependency_specification.name.as_ref(), "requests"); -//! assert_eq!(dependency_specification.extras, Some(vec![ExtraName::new("security"), ExtraName::new("tests")])); +//! assert_eq!(dependency_specification.extras, Some(vec![ExtraName::from_str("security").unwrap(), ExtraName::from_str("tests").unwrap()])); //! ``` #![deny(missing_docs)] @@ -63,21 +63,13 @@ pub struct Pep508Error { #[derive(Debug, Error, Clone, Eq, PartialEq)] pub enum Pep508ErrorSource { /// An error from our parser + #[error("{0}")] String(String), /// A url parsing error #[error(transparent)] UrlError(#[from] url::ParseError), } -impl Display for Pep508ErrorSource { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - Pep508ErrorSource::String(string) => string.fmt(f), - Pep508ErrorSource::UrlError(parse_err) => parse_err.fmt(f), - } - } -} - impl Display for Pep508Error { /// Pretty formatting with underline fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { @@ -568,7 +560,10 @@ fn parse_name(chars: &mut CharIter) -> Result { }); } } - Some(_) | None => return Ok(PackageName::new(name)), + Some(_) | None => { + return Ok(PackageName::new(name) + .expect("`PackageName` validation should match PEP 508 parsing")); + } } } } @@ -641,10 +636,16 @@ fn parse_extras(chars: &mut CharIter) -> Result>, Pep508Er // end or next identifier? match chars.next() { Some((_, ',')) => { - extras.push(ExtraName::new(buffer)); + extras.push( + ExtraName::new(buffer) + .expect("`ExtraName` validation should match PEP 508 parsing"), + ); } Some((_, ']')) => { - extras.push(ExtraName::new(buffer)); + extras.push( + ExtraName::new(buffer) + .expect("`ExtraName` validation should match PEP 508 parsing"), + ); break; } Some((pos, other)) => { @@ -944,8 +945,11 @@ mod tests { let requests = Requirement::from_str(input).unwrap(); assert_eq!(input, requests.to_string()); let expected = Requirement { - name: PackageName::new("requests"), - extras: Some(vec![ExtraName::new("security"), ExtraName::new("tests")]), + name: PackageName::from_str("requests").unwrap(), + extras: Some(vec![ + ExtraName::from_str("security").unwrap(), + ExtraName::from_str("tests").unwrap(), + ]), version_or_url: Some(VersionOrUrl::VersionSpecifier( [ VersionSpecifier::new( @@ -1086,7 +1090,7 @@ mod tests { #[test] fn error_extras1() { let numpy = Requirement::from_str("black[d]").unwrap(); - assert_eq!(numpy.extras, Some(vec![ExtraName::new("d")])); + assert_eq!(numpy.extras, Some(vec![ExtraName::from_str("d").unwrap()])); } #[test] @@ -1094,7 +1098,10 @@ mod tests { let numpy = Requirement::from_str("black[d,jupyter]").unwrap(); assert_eq!( numpy.extras, - Some(vec![ExtraName::new("d"), ExtraName::new("jupyter")]) + Some(vec![ + ExtraName::from_str("d").unwrap(), + ExtraName::from_str("jupyter").unwrap() + ]) ); } @@ -1141,7 +1148,7 @@ mod tests { .unwrap(); let url = "https://github.com/pypa/pip/archive/1.3.1.zip#sha1=da9234ee9982d4bbb3c72346a6de940a148ea686"; let expected = Requirement { - name: PackageName::new("pip"), + name: PackageName::from_str("pip").unwrap(), extras: None, marker: None, version_or_url: Some(VersionOrUrl::Url(Url::parse(url).unwrap())), diff --git a/crates/puffin-cli/src/commands/mod.rs b/crates/puffin-cli/src/commands/mod.rs index 073a09b98..36ab674c5 100644 --- a/crates/puffin-cli/src/commands/mod.rs +++ b/crates/puffin-cli/src/commands/mod.rs @@ -4,7 +4,7 @@ use std::time::Duration; pub(crate) use add::add; pub(crate) use clean::clean; pub(crate) use freeze::freeze; -pub(crate) use pip_compile::pip_compile; +pub(crate) use pip_compile::{extra_name_with_clap_error, pip_compile}; pub(crate) use pip_sync::pip_sync; pub(crate) use pip_uninstall::pip_uninstall; pub(crate) use remove::remove; diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index f4a961b77..5c876500d 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -16,7 +16,9 @@ use platform_tags::Tags; use puffin_client::RegistryClientBuilder; use puffin_dispatch::BuildDispatch; use puffin_interpreter::Virtualenv; +use puffin_normalize::ExtraName; use puffin_resolver::{Manifest, PreReleaseMode, ResolutionFailureReporter, ResolutionMode}; +use std::str::FromStr; use crate::commands::reporters::ResolverReporter; use crate::commands::{elapsed, ExitStatus}; @@ -229,3 +231,12 @@ impl From for UpgradeMode { } } } + +pub(crate) fn extra_name_with_clap_error(arg: &str) -> Result { + ExtraName::from_str(arg).map_err(|_err| { + anyhow!( + "Extra names must start and end with a letter or digit and may only \ + contain -, _, ., and alphanumeric characters" + ) + }) +} diff --git a/crates/puffin-cli/src/commands/remove.rs b/crates/puffin-cli/src/commands/remove.rs index d020baf45..9ad421d6b 100644 --- a/crates/puffin-cli/src/commands/remove.rs +++ b/crates/puffin-cli/src/commands/remove.rs @@ -2,6 +2,7 @@ use std::path::PathBuf; use anyhow::Result; use miette::{Diagnostic, IntoDiagnostic}; +use puffin_normalize::PackageName; use thiserror::Error; use tracing::info; @@ -12,7 +13,7 @@ use crate::printer::Printer; /// Remove a dependency from the workspace. #[allow(clippy::unnecessary_wraps)] -pub(crate) fn remove(name: &str, _printer: Printer) -> Result { +pub(crate) fn remove(name: &PackageName, _printer: Printer) -> Result { match remove_impl(name) { Ok(status) => Ok(status), Err(err) => { @@ -46,7 +47,7 @@ enum RemoveError { RemovalError(String, #[source] WorkspaceError), } -fn remove_impl(name: &str) -> miette::Result { +fn remove_impl(name: &PackageName) -> miette::Result { // Locate the workspace. let cwd = std::env::current_dir().into_diagnostic()?; let Some(workspace_root) = puffin_workspace::find_pyproject_toml(cwd) else { diff --git a/crates/puffin-cli/src/main.rs b/crates/puffin-cli/src/main.rs index 64465bfae..c9b09c2b0 100644 --- a/crates/puffin-cli/src/main.rs +++ b/crates/puffin-cli/src/main.rs @@ -9,11 +9,11 @@ use directories::ProjectDirs; use tempfile::tempdir; use url::Url; -use puffin_normalize::ExtraName; +use puffin_normalize::{ExtraName, PackageName}; use puffin_resolver::{PreReleaseMode, ResolutionMode}; use requirements::ExtrasSpecification; -use crate::commands::ExitStatus; +use crate::commands::{extra_name_with_clap_error, ExitStatus}; use crate::index_urls::IndexUrls; use crate::requirements::RequirementsSource; @@ -78,7 +78,7 @@ struct PipCompileArgs { constraint: Vec, /// Include optional dependencies in the given extra group name; may be provided more than once. - #[clap(long, conflicts_with = "all_extras")] + #[clap(long, conflicts_with = "all_extras", value_parser = extra_name_with_clap_error)] extra: Vec, /// Include all optional dependencies. @@ -169,7 +169,7 @@ struct AddArgs { #[derive(Args)] struct RemoveArgs { /// The name of the package to remove (e.g., `Django`). - name: String, + name: PackageName, } async fn inner() -> Result { diff --git a/crates/puffin-cli/src/requirements.rs b/crates/puffin-cli/src/requirements.rs index b950d5444..af066d460 100644 --- a/crates/puffin-cli/src/requirements.rs +++ b/crates/puffin-cli/src/requirements.rs @@ -112,7 +112,9 @@ impl RequirementsSpecification { for (name, optional_requirements) in project.optional_dependencies.unwrap_or_default() { - let normalized_name = ExtraName::new(name); + // TODO(konstin): It's not ideal that pyproject-toml doesn't use + // `ExtraName` + let normalized_name = ExtraName::new(name)?; if extras.contains(&normalized_name) { used_extras.insert(normalized_name); requirements.extend(optional_requirements); @@ -120,7 +122,9 @@ impl RequirementsSpecification { } } // Parse the project name - project_name = Some(PackageName::new(project.name)); + project_name = Some(PackageName::new(project.name).with_context(|| { + format!("Invalid `project.name` in {}", path.display()) + })?); } Self { diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap b/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap index 8546596bb..4fcc9ba7c 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap @@ -8,9 +8,9 @@ info: - "--extra" - invalid name! - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpXIuamZ + - /tmp/.tmpiOHlGv env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp0LGEEX/.venv + VIRTUAL_ENV: /tmp/.tmpA05wES/.venv --- success: false exit_code: 2 diff --git a/crates/puffin-distribution/src/lib.rs b/crates/puffin-distribution/src/lib.rs index 475933ce9..b348d6ddf 100644 --- a/crates/puffin-distribution/src/lib.rs +++ b/crates/puffin-distribution/src/lib.rs @@ -172,7 +172,7 @@ impl CachedDistribution { return Ok(None); }; - let name = PackageName::new(name); + let name = PackageName::from_str(name)?; let version = Version::from_str(version).map_err(|err| anyhow!(err))?; let path = path.to_path_buf(); @@ -251,7 +251,7 @@ impl InstalledDistribution { return Ok(None); }; - let name = PackageName::new(name); + let name = PackageName::from_str(name)?; let version = Version::from_str(version).map_err(|err| anyhow!(err))?; let path = path.to_path_buf(); diff --git a/crates/puffin-normalize/src/extra_name.rs b/crates/puffin-normalize/src/extra_name.rs index 52715f9e8..d26dda2f2 100644 --- a/crates/puffin-normalize/src/extra_name.rs +++ b/crates/puffin-normalize/src/extra_name.rs @@ -1,10 +1,9 @@ +use serde::{Deserialize, Deserializer, Serialize}; use std::fmt; use std::fmt::{Display, Formatter}; use std::str::FromStr; -use anyhow::{anyhow, Error, Result}; -use once_cell::sync::Lazy; -use regex::Regex; +use crate::{validate_and_normalize_owned, validate_and_normalize_ref, InvalidNameError}; /// The normalized name of an extra dependency group. /// @@ -14,91 +13,42 @@ use regex::Regex; /// See: /// - /// - -#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize)] pub struct ExtraName(String); +impl ExtraName { + /// Create a validated, normalized extra name. + pub fn new(name: String) -> Result { + validate_and_normalize_owned(name).map(Self) + } +} + +impl FromStr for ExtraName { + type Err = InvalidNameError; + + fn from_str(name: &str) -> Result { + validate_and_normalize_ref(name).map(Self) + } +} + +impl<'de> Deserialize<'de> for ExtraName { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + Self::from_str(&s).map_err(serde::de::Error::custom) + } +} + impl Display for ExtraName { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { self.0.fmt(f) } } -static NAME_NORMALIZE: Lazy = Lazy::new(|| Regex::new(r"[-_.]+").unwrap()); -static NAME_VALIDATE: Lazy = - Lazy::new(|| Regex::new(r"(?i)^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$").unwrap()); - -impl ExtraName { - pub fn new(name: impl AsRef) -> Self { - // TODO(charlie): Avoid allocating in the common case (when no normalization is required). - let mut normalized = NAME_NORMALIZE.replace_all(name.as_ref(), "-").to_string(); - normalized.make_ascii_lowercase(); - Self(normalized) - } - - /// Create a validated, normalized extra name. - pub fn validate(name: impl AsRef) -> Result { - if NAME_VALIDATE.is_match(name.as_ref()) { - Ok(Self::new(name)) - } else { - Err(anyhow!( - "Extra names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters" - )) - } - } -} - impl AsRef for ExtraName { fn as_ref(&self) -> &str { - self.0.as_ref() - } -} - -impl FromStr for ExtraName { - type Err = Error; - - fn from_str(name: &str) -> Result { - Self::validate(name) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn normalize() { - assert_eq!(ExtraName::new("friendly-bard").as_ref(), "friendly-bard"); - assert_eq!(ExtraName::new("Friendly-Bard").as_ref(), "friendly-bard"); - assert_eq!(ExtraName::new("FRIENDLY-BARD").as_ref(), "friendly-bard"); - assert_eq!(ExtraName::new("friendly.bard").as_ref(), "friendly-bard"); - assert_eq!(ExtraName::new("friendly_bard").as_ref(), "friendly-bard"); - assert_eq!(ExtraName::new("friendly--bard").as_ref(), "friendly-bard"); - assert_eq!( - ExtraName::new("FrIeNdLy-._.-bArD").as_ref(), - "friendly-bard" - ); - } - - #[test] - fn validate() { - // Unchanged - assert_eq!( - ExtraName::validate("friendly-bard").unwrap().as_ref(), - "friendly-bard" - ); - assert_eq!(ExtraName::validate("1okay").unwrap().as_ref(), "1okay"); - assert_eq!(ExtraName::validate("okay2").unwrap().as_ref(), "okay2"); - // Normalizes - assert_eq!( - ExtraName::validate("Friendly-Bard").unwrap().as_ref(), - "friendly-bard" - ); - // Failures... - assert!(ExtraName::validate(" starts-with-space").is_err()); - assert!(ExtraName::validate("-starts-with-dash").is_err()); - assert!(ExtraName::validate("ends-with-dash-").is_err()); - assert!(ExtraName::validate("ends-with-space ").is_err()); - assert!(ExtraName::validate("includes!invalid-char").is_err()); - assert!(ExtraName::validate("space in middle").is_err()); + &self.0 } } diff --git a/crates/puffin-normalize/src/lib.rs b/crates/puffin-normalize/src/lib.rs index ed6a73fd9..6d3362f44 100644 --- a/crates/puffin-normalize/src/lib.rs +++ b/crates/puffin-normalize/src/lib.rs @@ -1,5 +1,110 @@ +use std::error::Error; +use std::fmt::{Display, Formatter}; + +use once_cell::sync::Lazy; +use regex::Regex; + pub use extra_name::ExtraName; pub use package_name::PackageName; mod extra_name; mod package_name; + +pub(crate) static NAME_NORMALIZE: Lazy = Lazy::new(|| Regex::new(r"[-_.]+").unwrap()); +pub(crate) static NAME_VALIDATE: Lazy = + Lazy::new(|| Regex::new(r"(?i)^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$").unwrap()); + +pub(crate) fn validate_and_normalize_ref( + name: impl AsRef, +) -> Result { + if !NAME_VALIDATE.is_match(name.as_ref()) { + return Err(InvalidNameError(name.as_ref().to_string())); + } + let mut normalized = NAME_NORMALIZE.replace_all(name.as_ref(), "-").to_string(); + normalized.make_ascii_lowercase(); + Ok(normalized) +} + +pub(crate) fn validate_and_normalize_owned(mut name: String) -> Result { + if !NAME_VALIDATE.is_match(name.as_ref()) { + return Err(InvalidNameError(name)); + } + let normalized = NAME_NORMALIZE.replace_all(&name, "-"); + // fast path: Don't allocate if we don't need to. An inplace ascii char replace would be + // nicer but doesn't exist + if normalized != name { + name = normalized.to_string(); + } + name.make_ascii_lowercase(); + Ok(name) +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct InvalidNameError(String); + +impl Display for InvalidNameError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Not a valid package or extra name: \"{}\". Names must start and end with a letter or \ + digit and may only contain -, _, ., and alphanumeric characters", + self.0 + ) + } +} + +impl Error for InvalidNameError {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn normalize() { + let inputs = [ + "friendly-bard", + "Friendly-Bard", + "FRIENDLY-BARD", + "friendly.bard", + "friendly_bard", + "friendly--bard", + "FrIeNdLy-._.-bArD", + ]; + for input in inputs { + assert_eq!(validate_and_normalize_ref(input).unwrap(), "friendly-bard"); + assert_eq!( + validate_and_normalize_owned(input.to_string()).unwrap(), + "friendly-bard" + ); + } + } + + #[test] + fn unchanged() { + // Unchanged + let unchanged = ["friendly-bard", "1okay", "okay2"]; + for input in unchanged { + assert_eq!(validate_and_normalize_ref(input).unwrap(), input); + assert_eq!( + validate_and_normalize_owned(input.to_string()).unwrap(), + input + ); + } + } + + #[test] + fn failures() { + let failures = [ + " starts-with-space", + "-starts-with-dash", + "ends-with-dash-", + "ends-with-space ", + "includes!invalid-char", + "space in middle", + ]; + for input in failures { + assert!(validate_and_normalize_ref(input).is_err()); + assert!(validate_and_normalize_owned(input.to_string()).is_err(),); + } + } +} diff --git a/crates/puffin-normalize/src/package_name.rs b/crates/puffin-normalize/src/package_name.rs index 19f89d93a..ada7b9d8c 100644 --- a/crates/puffin-normalize/src/package_name.rs +++ b/crates/puffin-normalize/src/package_name.rs @@ -1,10 +1,11 @@ use std::fmt; use std::fmt::{Display, Formatter}; +use std::str::FromStr; -use once_cell::sync::Lazy; -use regex::Regex; use serde::{Deserialize, Deserializer, Serialize}; +use crate::{validate_and_normalize_owned, validate_and_normalize_ref, InvalidNameError}; + /// The normalized name of a package. /// /// Converts the name to lowercase and collapses any run of the characters `-`, `_` and `.` @@ -14,27 +15,10 @@ use serde::{Deserialize, Deserializer, Serialize}; #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize)] pub struct PackageName(String); -impl From<&PackageName> for PackageName { - /// Required for `WaitMap::wait` - fn from(package_name: &PackageName) -> Self { - package_name.clone() - } -} - -impl Display for PackageName { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - self.0.fmt(f) - } -} - -static NAME_NORMALIZE: Lazy = Lazy::new(|| Regex::new(r"[-_.]+").unwrap()); - impl PackageName { - pub fn new(name: impl AsRef) -> Self { - // TODO(charlie): Avoid allocating in the common case (when no normalization is required). - let mut normalized = NAME_NORMALIZE.replace_all(name.as_ref(), "-").to_string(); - normalized.make_ascii_lowercase(); - Self(normalized) + /// Create a validated, normalized package name. + pub fn new(name: String) -> Result { + validate_and_normalize_owned(name).map(Self) } /// Escape this name with underscores (`_`) instead of dashes (`-`) @@ -45,9 +29,18 @@ impl PackageName { } } -impl AsRef for PackageName { - fn as_ref(&self) -> &str { - self.0.as_ref() +impl From<&PackageName> for PackageName { + /// Required for `WaitMap::wait` + fn from(package_name: &PackageName) -> Self { + package_name.clone() + } +} + +impl FromStr for PackageName { + type Err = InvalidNameError; + + fn from_str(name: &str) -> Result { + validate_and_normalize_ref(name).map(Self) } } @@ -57,25 +50,18 @@ impl<'de> Deserialize<'de> for PackageName { D: Deserializer<'de>, { let s = String::deserialize(deserializer)?; - Ok(Self::new(s)) + Self::from_str(&s).map_err(serde::de::Error::custom) } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn normalize() { - assert_eq!(PackageName::new("friendly-bard").as_ref(), "friendly-bard"); - assert_eq!(PackageName::new("Friendly-Bard").as_ref(), "friendly-bard"); - assert_eq!(PackageName::new("FRIENDLY-BARD").as_ref(), "friendly-bard"); - assert_eq!(PackageName::new("friendly.bard").as_ref(), "friendly-bard"); - assert_eq!(PackageName::new("friendly_bard").as_ref(), "friendly-bard"); - assert_eq!(PackageName::new("friendly--bard").as_ref(), "friendly-bard"); - assert_eq!( - PackageName::new("FrIeNdLy-._.-bArD").as_ref(), - "friendly-bard" - ); +impl Display for PackageName { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl AsRef for PackageName { + fn as_ref(&self) -> &str { + &self.0 } } diff --git a/crates/puffin-package/src/pypi_types/metadata.rs b/crates/puffin-package/src/pypi_types/metadata.rs index 3058efaad..b2290b5bf 100644 --- a/crates/puffin-package/src/pypi_types/metadata.rs +++ b/crates/puffin-package/src/pypi_types/metadata.rs @@ -13,7 +13,7 @@ use tracing::warn; use pep440_rs::{Pep440Error, Version, VersionSpecifiers}; use pep508_rs::{Pep508Error, Requirement}; -use puffin_normalize::PackageName; +use puffin_normalize::{ExtraName, InvalidNameError, PackageName}; /// Python Package Metadata 2.1 as specified in /// @@ -48,7 +48,7 @@ pub struct Metadata21 { pub requires_python: Option, pub requires_external: Vec, pub project_urls: HashMap, - pub provides_extras: Vec, + pub provides_extras: Vec, } /// @@ -86,6 +86,8 @@ pub enum Error { /// Invalid Requirement #[error(transparent)] Pep508Error(#[from] Pep508Error), + #[error(transparent)] + InvalidName(#[from] InvalidNameError), } /// From @@ -127,7 +129,7 @@ impl Metadata21 { headers .get_first_value("Name") .ok_or(Error::FieldNotFound("Name"))?, - ); + )?; let version = Version::from_str( &headers .get_first_value("Version") @@ -155,9 +157,9 @@ impl Metadata21 { .map(|requires_dist| LenientRequirement::from_str(requires_dist).map(Requirement::from)) .collect::, _>>()?; let provides_dist = get_all_values("Provides-Dist") - .iter() + .into_iter() .map(PackageName::new) - .collect(); + .collect::, _>>()?; let obsoletes_dist = get_all_values("Obsoletes-Dist"); let maintainer = get_first_value("Maintainer"); let maintainer_email = get_first_value("Maintainer-email"); @@ -174,7 +176,10 @@ impl Metadata21 { Some((name, value)) => Ok((name.to_string(), value.trim().to_string())), }) .collect::>()?; - let provides_extras = get_all_values("Provides-Extra"); + let provides_extras = get_all_values("Provides-Extra") + .into_iter() + .map(ExtraName::new) + .collect::, _>>()?; let description_content_type = get_first_value("Description-Content-Type"); Ok(Metadata21 { metadata_version, diff --git a/crates/puffin-resolver/src/pubgrub/dependencies.rs b/crates/puffin-resolver/src/pubgrub/dependencies.rs index 25dda164b..f59247384 100644 --- a/crates/puffin-resolver/src/pubgrub/dependencies.rs +++ b/crates/puffin-resolver/src/pubgrub/dependencies.rs @@ -53,7 +53,7 @@ impl PubGrubDependencies { .clone() .into_iter() .flatten() - .map(|extra| to_pubgrub(requirement, Some(ExtraName::new(extra)))), + .map(|extra| to_pubgrub(requirement, Some(extra))), ) { let (package, version) = result?; @@ -101,7 +101,7 @@ impl PubGrubDependencies { .clone() .into_iter() .flatten() - .map(|extra| to_pubgrub(constraint, Some(ExtraName::new(extra)))), + .map(|extra| to_pubgrub(constraint, Some(extra))), ) { let (package, version) = result?; diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 33b7a2931..b77ef9055 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -518,7 +518,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { if !metadata .provides_extras .iter() - .any(|provided_extra| ExtraName::new(provided_extra) == *extra) + .any(|provided_extra| provided_extra == extra) { return Ok(Dependencies::Unknown); } diff --git a/crates/puffin-workspace/src/workspace.rs b/crates/puffin-workspace/src/workspace.rs index 524d5dab7..48bc5a81c 100644 --- a/crates/puffin-workspace/src/workspace.rs +++ b/crates/puffin-workspace/src/workspace.rs @@ -98,7 +98,7 @@ impl Workspace { } /// Remove a dependency from the workspace. - pub fn remove_dependency(&mut self, name: &str) -> Result<(), WorkspaceError> { + pub fn remove_dependency(&mut self, name: &PackageName) -> Result<(), WorkspaceError> { let Some(project) = self .document .get_mut("project") @@ -123,7 +123,7 @@ impl Workspace { return false; }; - PackageName::new(name) == existing.name + name == &existing.name }); let Some(index) = index else {