From aa9f47bbde51fb43030049715b1c21bfd649429e Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Tue, 19 Dec 2023 12:25:32 -0500 Subject: [PATCH] improve tests for version parser (#696) The high level goal here is to improve the tests for the version parser. Namely, we now check not just that version strings parse successfully, but that they parse to the expected result. We also do a few other cleanups. Most notably, `Version` is now an opaque type so that we can more easily change its representation going forward. Reviewing commit-by-commit is suggested. :-) --- crates/pep440-rs/src/lib.rs | 15 +- crates/pep440-rs/src/version.rs | 1345 +++++++++++------ crates/pep440-rs/src/version_specifier.rs | 123 +- crates/pep508-rs/src/lib.rs | 23 +- crates/pep508-rs/src/marker.rs | 10 +- crates/puffin-cli/src/python_version.rs | 8 +- crates/puffin-interpreter/src/interpreter.rs | 4 +- .../puffin-resolver/src/pubgrub/specifier.rs | 75 +- crates/puffin-resolver/src/pubgrub/version.rs | 11 +- 9 files changed, 968 insertions(+), 646 deletions(-) diff --git a/crates/pep440-rs/src/lib.rs b/crates/pep440-rs/src/lib.rs index 5fcfe90af..839b751cb 100644 --- a/crates/pep440-rs/src/lib.rs +++ b/crates/pep440-rs/src/lib.rs @@ -42,14 +42,15 @@ //! the version matching needs to catch all sorts of special cases #![deny(missing_docs)] +pub use { + version::{LocalSegment, Operator, PreRelease, Version}, + version_specifier::{parse_version_specifiers, VersionSpecifier, VersionSpecifiers}, +}; + #[cfg(feature = "pyo3")] use pyo3::{pymodule, types::PyModule, PyResult, Python}; -use std::error::Error; -use std::fmt::{Display, Formatter}; #[cfg(feature = "pyo3")] pub use version::PyVersion; -pub use version::{LocalSegment, Operator, PreRelease, Version}; -pub use version_specifier::{parse_version_specifiers, VersionSpecifier, VersionSpecifiers}; mod version; mod version_specifier; @@ -67,8 +68,8 @@ pub struct Pep440Error { pub width: usize, } -impl Display for Pep440Error { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl std::fmt::Display for Pep440Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { writeln!(f, "Failed to parse version:")?; writeln!(f, "{}", self.line)?; writeln!(f, "{}{}", " ".repeat(self.start), "^".repeat(self.width))?; @@ -76,7 +77,7 @@ impl Display for Pep440Error { } } -impl Error for Pep440Error {} +impl std::error::Error for Pep440Error {} /// Python bindings shipped as `pep440_rs` #[cfg(feature = "pyo3")] diff --git a/crates/pep440-rs/src/version.rs b/crates/pep440-rs/src/version.rs index 6105c074b..964178dd7 100644 --- a/crates/pep440-rs/src/version.rs +++ b/crates/pep440-rs/src/version.rs @@ -1,23 +1,21 @@ -use once_cell::sync::Lazy; +use std::{ + borrow::Borrow, + cmp::Ordering, + hash::{Hash, Hasher}, + str::FromStr, +}; + #[cfg(feature = "pyo3")] use pyo3::{ basic::CompareOp, exceptions::PyValueError, pyclass, pymethods, FromPyObject, IntoPy, PyAny, PyObject, PyResult, Python, }; -use regex::Captures; -use regex::Regex; #[cfg(feature = "serde")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; -use std::cmp::{max, Ordering}; -#[cfg(feature = "pyo3")] -use std::collections::hash_map::DefaultHasher; -use std::fmt::{Debug, Display, Formatter}; -use std::hash::{Hash, Hasher}; -use std::iter; -use std::str::FromStr; - -#[cfg(feature = "tracing")] -use tracing::warn; +use { + once_cell::sync::Lazy, + regex::{Captures, Regex}, +}; /// A regex copied from , /// updated to support stars for version ranges @@ -98,7 +96,7 @@ impl FromStr for Operator { "===" => { #[cfg(feature = "tracing")] { - warn!("Using arbitrary equality (`===`) is discouraged"); + tracing::warn!("Using arbitrary equality (`===`) is discouraged"); } #[allow(deprecated)] Self::ExactEqual @@ -120,9 +118,9 @@ impl FromStr for Operator { } } -impl Display for Operator { +impl std::fmt::Display for Operator { /// Note the `EqualStar` is also `==`. - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let operator = match self { Operator::Equal => "==", // Beware, this doesn't print the star @@ -157,7 +155,7 @@ impl Operator { /// Optional prerelease modifier (alpha, beta or release candidate) appended to version /// /// -#[derive(PartialEq, Eq, Debug, Hash, Clone, Ord, PartialOrd)] +#[derive(PartialEq, Eq, Debug, Hash, Clone, Copy, Ord, PartialOrd)] #[cfg_attr(feature = "pyo3", pyclass)] pub enum PreRelease { /// alpha prerelease @@ -183,8 +181,8 @@ impl FromStr for PreRelease { } } -impl Display for PreRelease { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl std::fmt::Display for PreRelease { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Alpha => write!(f, "a"), Self::Beta => write!(f, "b"), @@ -216,8 +214,8 @@ pub enum LocalSegment { Number(u64), } -impl Display for LocalSegment { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl std::fmt::Display for LocalSegment { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::String(string) => write!(f, "{string}"), Self::Number(number) => write!(f, "{number}"), @@ -263,25 +261,25 @@ impl FromStr for LocalSegment { pub struct Version { /// The [versioning epoch](https://peps.python.org/pep-0440/#version-epochs). Normally just 0, /// but you can increment it if you switched the versioning scheme. - pub epoch: u64, + epoch: u64, /// The normal number part of the version /// (["final release"](https://peps.python.org/pep-0440/#final-releases)), /// such a `1.2.3` in `4!1.2.3-a8.post9.dev1` /// /// Note that we drop the * placeholder by moving it to `Operator` - pub release: Vec, + release: Vec, /// The [prerelease](https://peps.python.org/pep-0440/#pre-releases), i.e. alpha, beta or rc /// plus a number /// /// Note that whether this is Some influences the version /// range matching since normally we exclude all prerelease versions - pub pre: Option<(PreRelease, u64)>, + pre: Option<(PreRelease, u64)>, /// The [Post release version](https://peps.python.org/pep-0440/#post-releases), /// higher post version are preferred over lower post or none-post versions - pub post: Option, + post: Option, /// The [developmental release](https://peps.python.org/pep-0440/#developmental-releases), /// if any - pub dev: Option, + dev: Option, /// A [local version identifier](https://peps.python.org/pep-0440/#local-version-identifiers) /// such as `+deadbeef` in `1.2.3+deadbeef` /// @@ -289,132 +287,7 @@ pub struct Version { /// > along with an arbitrary “local version label”, separated from the public version /// > identifier by a plus. Local version labels have no specific semantics assigned, but some /// > syntactic restrictions are imposed. - pub local: Option>, -} - -#[cfg(feature = "pyo3")] -impl IntoPy for Version { - fn into_py(self, py: Python<'_>) -> PyObject { - PyVersion(self).into_py(py) - } -} - -#[cfg(feature = "pyo3")] -impl<'source> FromPyObject<'source> for Version { - fn extract(ob: &'source PyAny) -> PyResult { - Ok(ob.extract::()?.0) - } -} - -/// Workaround for -#[cfg(feature = "pyo3")] -#[derive(Clone, Debug)] -#[pyclass(name = "Version")] -pub struct PyVersion(pub Version); - -#[cfg(feature = "pyo3")] -#[pymethods] -impl PyVersion { - /// The [versioning epoch](https://peps.python.org/pep-0440/#version-epochs). Normally just 0, - /// but you can increment it if you switched the versioning scheme. - #[getter] - pub fn epoch(&self) -> u64 { - self.0.epoch - } - /// The normal number part of the version - /// (["final release"](https://peps.python.org/pep-0440/#final-releases)), - /// such a `1.2.3` in `4!1.2.3-a8.post9.dev1` - /// - /// Note that we drop the * placeholder by moving it to `Operator` - #[getter] - pub fn release(&self) -> Vec { - self.0.release.clone() - } - /// The [prerelease](https://peps.python.org/pep-0440/#pre-releases), i.e. alpha, beta or rc - /// plus a number - /// - /// Note that whether this is Some influences the version - /// range matching since normally we exclude all prerelease versions - #[getter] - pub fn pre(&self) -> Option<(PreRelease, u64)> { - self.0.pre.clone() - } - /// The [Post release version](https://peps.python.org/pep-0440/#post-releases), - /// higher post version are preferred over lower post or none-post versions - #[getter] - pub fn post(&self) -> Option { - self.0.post - } - /// The [developmental release](https://peps.python.org/pep-0440/#developmental-releases), - /// if any - #[getter] - pub fn dev(&self) -> Option { - self.0.dev - } - /// The first item of release or 0 if unavailable. - #[getter] - #[allow(clippy::get_first)] - pub fn major(&self) -> u64 { - self.0.release.get(0).copied().unwrap_or_default() - } - /// The second item of release or 0 if unavailable. - #[getter] - pub fn minor(&self) -> u64 { - self.0.release.get(1).copied().unwrap_or_default() - } - /// The third item of release or 0 if unavailable. - #[getter] - pub fn micro(&self) -> u64 { - self.0.release.get(2).copied().unwrap_or_default() - } - - /// Parses a PEP 440 version string - #[cfg(feature = "pyo3")] - #[new] - pub fn parse(version: &str) -> PyResult { - Ok(Self( - Version::from_str(version).map_err(PyValueError::new_err)?, - )) - } - - // Maps the error type - /// Parse a PEP 440 version optionally ending with `.*` - #[cfg(feature = "pyo3")] - #[staticmethod] - pub fn parse_star(version_specifier: &str) -> PyResult<(Self, bool)> { - Version::from_str_star(version_specifier) - .map_err(PyValueError::new_err) - .map(|(version, star)| (Self(version), star)) - } - - /// Returns the normalized representation - #[cfg(feature = "pyo3")] - pub fn __str__(&self) -> String { - self.0.to_string() - } - - /// Returns the normalized representation - #[cfg(feature = "pyo3")] - pub fn __repr__(&self) -> String { - format!(r#""#, self.0) - } - - /// Returns the normalized representation - #[cfg(feature = "pyo3")] - pub fn __hash__(&self) -> u64 { - let mut hasher = DefaultHasher::new(); - self.0.hash(&mut hasher); - hasher.finish() - } - - #[cfg(feature = "pyo3")] - fn __richcmp__(&self, other: &Self, op: CompareOp) -> bool { - op.matches(self.0.cmp(&other.0)) - } - - fn any_prerelease(&self) -> bool { - self.0.any_prerelease() - } + local: Option>, } /// @@ -441,277 +314,28 @@ impl Serialize for Version { } impl Version { - /// Constructor for a version that is just a release such as `3.8` - pub fn from_release(release: Vec) -> Self { - Self { + /// Create a new version from an iterator of segments in the release part + /// of a version. + #[inline] + pub fn new(release_numbers: I) -> Version + where + I: IntoIterator, + R: Borrow, + { + Version { epoch: 0, - release, + release: vec![], pre: None, post: None, dev: None, local: None, } + .with_release(release_numbers) } - /// For PEP 440 specifier matching: "Except where specifically noted below, local version - /// identifiers MUST NOT be permitted in version specifiers, and local version labels MUST be - /// ignored entirely when checking if candidate versions match a given version specifier." - pub(crate) fn without_local(&self) -> Self { - Self { - local: None, - ..self.clone() - } - } -} - -impl Version { - /// Whether this is an alpha/beta/rc or dev version - pub fn any_prerelease(&self) -> bool { - self.is_pre() || self.is_dev() - } - - /// Whether this is an alpha/beta/rc version - pub fn is_pre(&self) -> bool { - self.pre.is_some() - } - - /// Whether this is a dev version - pub fn is_dev(&self) -> bool { - self.dev.is_some() - } - - /// Whether this is a post version - pub fn is_post(&self) -> bool { - self.post.is_some() - } - - /// Whether this is a local version (e.g. `1.2.3+localsuffixesareweird`) - pub fn is_local(&self) -> bool { - self.local.is_some() - } -} - -/// Shows normalized version -impl Display for Version { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let epoch = if self.epoch == 0 { - String::new() - } else { - format!("{}!", self.epoch) - }; - let release = self - .release - .iter() - .map(ToString::to_string) - .collect::>() - .join("."); - let pre = self - .pre - .as_ref() - .map(|(pre_kind, pre_version)| format!("{pre_kind}{pre_version}")) - .unwrap_or_default(); - let post = self - .post - .map(|post| format!(".post{post}")) - .unwrap_or_default(); - let dev = self.dev.map(|dev| format!(".dev{dev}")).unwrap_or_default(); - let local = self - .local - .as_ref() - .map(|segments| { - format!( - "+{}", - segments - .iter() - .map(std::string::ToString::to_string) - .collect::>() - .join(".") - ) - }) - .unwrap_or_default(); - write!(f, "{epoch}{release}{pre}{post}{dev}{local}") - } -} - -impl Debug for Version { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "\"{}\"", self) - } -} - -/// Compare the release parts of two versions, e.g. `4.3.1` > `4.2`, `1.1.0` == `1.1` and -/// `1.16` < `1.19` -pub(crate) fn compare_release(this: &[u64], other: &[u64]) -> Ordering { - // "When comparing release segments with different numbers of components, the shorter segment - // is padded out with additional zeros as necessary" - for (this, other) in this.iter().chain(iter::repeat(&0)).zip( - other - .iter() - .chain(iter::repeat(&0)) - .take(max(this.len(), other.len())), - ) { - match this.cmp(other) { - Ordering::Less => { - return Ordering::Less; - } - Ordering::Equal => {} - Ordering::Greater => { - return Ordering::Greater; - } - } - } - Ordering::Equal -} - -/// Compare the parts attached after the release, given equal release -/// -/// According to -/// the order of pre/post-releases is: -/// .devN, aN, bN, rcN, , .postN -/// but also, you can have dev/post releases on beta releases, so we make a three stage ordering: -/// ({dev: 0, a: 1, b: 2, rc: 3, (): 4, post: 5}, , , , ) -/// -/// For post, any number is better than none (so None defaults to None<0), but for dev, no number -/// is better (so None default to the maximum). For local the Option> luckily already has the -/// correct default Ord implementation -fn sortable_tuple(version: &Version) -> (u64, u64, Option, u64, Option<&[LocalSegment]>) { - match (&version.pre, &version.post, &version.dev) { - // dev release - (None, None, Some(n)) => (0, 0, None, *n, version.local.as_deref()), - // alpha release - (Some((PreRelease::Alpha, n)), post, dev) => ( - 1, - *n, - *post, - dev.unwrap_or(u64::MAX), - version.local.as_deref(), - ), - // beta release - (Some((PreRelease::Beta, n)), post, dev) => ( - 2, - *n, - *post, - dev.unwrap_or(u64::MAX), - version.local.as_deref(), - ), - // alpha release - (Some((PreRelease::Rc, n)), post, dev) => ( - 3, - *n, - *post, - dev.unwrap_or(u64::MAX), - version.local.as_deref(), - ), - // final release - (None, None, None) => (4, 0, None, 0, version.local.as_deref()), - // post release - (None, Some(post), dev) => ( - 5, - 0, - Some(*post), - dev.unwrap_or(u64::MAX), - version.local.as_deref(), - ), - } -} - -impl PartialEq for Version { - fn eq(&self, other: &Self) -> bool { - self.cmp(other) == Ordering::Equal - } -} - -impl Eq for Version {} - -impl Hash for Version { - /// Custom implementation to ignoring trailing zero because `PartialEq` zero pads - fn hash(&self, state: &mut H) { - self.epoch.hash(state); - // Skip trailing zeros - for i in self.release.iter().rev().skip_while(|x| **x == 0) { - i.hash(state); - } - self.pre.hash(state); - self.dev.hash(state); - self.post.hash(state); - self.local.hash(state); - } -} - -impl PartialOrd for Version { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Version { - /// 1.0.dev456 < 1.0a1 < 1.0a2.dev456 < 1.0a12.dev456 < 1.0a12 < 1.0b1.dev456 < 1.0b2 - /// < 1.0b2.post345.dev456 < 1.0b2.post345 < 1.0b2-346 < 1.0c1.dev456 < 1.0c1 < 1.0rc2 < 1.0c3 - /// < 1.0 < 1.0.post456.dev34 < 1.0.post456 - fn cmp(&self, other: &Self) -> Ordering { - match self.epoch.cmp(&other.epoch) { - Ordering::Less => { - return Ordering::Less; - } - Ordering::Equal => {} - Ordering::Greater => { - return Ordering::Greater; - } - } - - match compare_release(&self.release, &other.release) { - Ordering::Less => { - return Ordering::Less; - } - Ordering::Equal => {} - Ordering::Greater => { - return Ordering::Greater; - } - } - - // release is equal, so compare the other parts - sortable_tuple(self).cmp(&sortable_tuple(other)) - } -} - -impl Ord for LocalSegment { - fn cmp(&self, other: &Self) -> Ordering { - // - match (self, other) { - (Self::Number(n1), Self::Number(n2)) => n1.cmp(n2), - (Self::String(s1), Self::String(s2)) => s1.cmp(s2), - (Self::Number(_), Self::String(_)) => Ordering::Greater, - (Self::String(_), Self::Number(_)) => Ordering::Less, - } - } -} - -impl FromStr for Version { - type Err = String; - - /// Parses a version such as `1.19`, `1.0a1`,`1.0+abc.5` or `1!2012.2` + /// Like [`Self::from_str`], but also allows the version to end with a star + /// and returns whether it did. This variant is for use in specifiers. /// - /// Note that this variant doesn't allow the version to end with a star, see - /// [`Self::from_str_star`] if you want to parse versions for specifiers - fn from_str(version: &str) -> Result { - if let Some(v) = version_fast_parse(version) { - return Ok(v); - } - - let captures = VERSION_RE - .captures(version) - .ok_or_else(|| format!("Version `{version}` doesn't match PEP 440 rules"))?; - let (version, star) = Version::parse_impl(&captures)?; - if star { - return Err("A star (`*`) must not be used in a fixed version (use `Version::from_string_star` otherwise)".to_string()); - } - Ok(version) - } -} - -impl Version { - /// Like [`Self::from_str`], but also allows the version to end with a star and returns whether it - /// did. This variant is for use in specifiers. /// * `1.2.3` -> false /// * `1.2.3.*` -> true /// * `1.2.*.4` -> err @@ -728,6 +352,135 @@ impl Version { Ok((version, star)) } + /// Whether this is an alpha/beta/rc or dev version + #[inline] + pub fn any_prerelease(&self) -> bool { + self.is_pre() || self.is_dev() + } + + /// Whether this is an alpha/beta/rc version + #[inline] + pub fn is_pre(&self) -> bool { + self.pre.is_some() + } + + /// Whether this is a dev version + #[inline] + pub fn is_dev(&self) -> bool { + self.dev.is_some() + } + + /// Whether this is a post version + #[inline] + pub fn is_post(&self) -> bool { + self.post.is_some() + } + + /// Whether this is a local version (e.g. `1.2.3+localsuffixesareweird`) + #[inline] + pub fn is_local(&self) -> bool { + self.local.is_some() + } + + /// Returns the epoch of this version. + #[inline] + pub fn epoch(&self) -> u64 { + self.epoch + } + + /// Returns the release number part of the version. + #[inline] + pub fn release(&self) -> &[u64] { + &self.release + } + + /// Returns the pre-relase part of this version, if it exists. + #[inline] + pub fn pre(&self) -> Option<(PreRelease, u64)> { + self.pre + } + + /// Returns the post-release part of this version, if it exists. + #[inline] + pub fn post(&self) -> Option { + self.post + } + + /// Returns the dev-release part of this version, if it exists. + #[inline] + pub fn dev(&self) -> Option { + self.dev + } + + /// Returns the local segments in this version, if any exist. + #[inline] + pub fn local(&self) -> Option<&[LocalSegment]> { + self.local.as_deref() + } + + /// Set the release numbers and return the updated version. + /// + /// Usually one can just use `Version::new` to create a new version with + /// the updated release numbers, but this is useful when one wants to + /// preserve the other components of a version number while only changing + /// the release numbers. + #[inline] + pub fn with_release(self, release_numbers: I) -> Version + where + I: IntoIterator, + R: Borrow, + { + Version { + release: release_numbers.into_iter().map(|r| *r.borrow()).collect(), + ..self + } + } + + /// Set the epoch and return the updated version. + #[inline] + pub fn with_epoch(self, epoch: u64) -> Version { + Version { epoch, ..self } + } + + /// Set the pre-release component and return the updated version. + #[inline] + pub fn with_pre(self, pre: Option<(PreRelease, u64)>) -> Version { + Version { pre, ..self } + } + + /// Set the post-release component and return the updated version. + #[inline] + pub fn with_post(self, post: Option) -> Version { + Version { post, ..self } + } + + /// Set the dev-release component and return the updated version. + #[inline] + pub fn with_dev(self, dev: Option) -> Version { + Version { dev, ..self } + } + + /// Set the local segments and return the updated version. + #[inline] + pub fn with_local(self, local: Vec) -> Version { + Version { + local: Some(local), + ..self + } + } + + /// For PEP 440 specifier matching: "Except where specifically noted below, + /// local version identifiers MUST NOT be permitted in version specifiers, + /// and local version labels MUST be ignored entirely when checking if + /// candidate versions match a given version specifier." + #[inline] + pub fn without_local(self) -> Version { + Version { + local: None, + ..self + } + } + /// Extracted for reusability around star/non-star pub(crate) fn parse_impl(captures: &Captures) -> Result<(Version, bool), String> { let number_field = |field_name| { @@ -831,6 +584,353 @@ impl Version { } } +/// Shows normalized version +impl std::fmt::Display for Version { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let epoch = if self.epoch == 0 { + String::new() + } else { + format!("{}!", self.epoch) + }; + let release = self + .release + .iter() + .map(ToString::to_string) + .collect::>() + .join("."); + let pre = self + .pre + .as_ref() + .map(|(pre_kind, pre_version)| format!("{pre_kind}{pre_version}")) + .unwrap_or_default(); + let post = self + .post + .map(|post| format!(".post{post}")) + .unwrap_or_default(); + let dev = self.dev.map(|dev| format!(".dev{dev}")).unwrap_or_default(); + let local = self + .local + .as_ref() + .map(|segments| { + format!( + "+{}", + segments + .iter() + .map(std::string::ToString::to_string) + .collect::>() + .join(".") + ) + }) + .unwrap_or_default(); + write!(f, "{epoch}{release}{pre}{post}{dev}{local}") + } +} + +impl std::fmt::Debug for Version { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "\"{}\"", self) + } +} + +impl PartialEq for Version { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl Eq for Version {} + +impl Hash for Version { + /// Custom implementation to ignoring trailing zero because `PartialEq` zero pads + fn hash(&self, state: &mut H) { + self.epoch.hash(state); + // Skip trailing zeros + for i in self.release.iter().rev().skip_while(|x| **x == 0) { + i.hash(state); + } + self.pre.hash(state); + self.dev.hash(state); + self.post.hash(state); + self.local.hash(state); + } +} + +impl PartialOrd for Version { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Version { + /// 1.0.dev456 < 1.0a1 < 1.0a2.dev456 < 1.0a12.dev456 < 1.0a12 < 1.0b1.dev456 < 1.0b2 + /// < 1.0b2.post345.dev456 < 1.0b2.post345 < 1.0b2-346 < 1.0c1.dev456 < 1.0c1 < 1.0rc2 < 1.0c3 + /// < 1.0 < 1.0.post456.dev34 < 1.0.post456 + fn cmp(&self, other: &Self) -> Ordering { + match self.epoch.cmp(&other.epoch) { + Ordering::Less => { + return Ordering::Less; + } + Ordering::Equal => {} + Ordering::Greater => { + return Ordering::Greater; + } + } + + match compare_release(&self.release, &other.release) { + Ordering::Less => { + return Ordering::Less; + } + Ordering::Equal => {} + Ordering::Greater => { + return Ordering::Greater; + } + } + + // release is equal, so compare the other parts + sortable_tuple(self).cmp(&sortable_tuple(other)) + } +} + +impl Ord for LocalSegment { + fn cmp(&self, other: &Self) -> Ordering { + // + match (self, other) { + (Self::Number(n1), Self::Number(n2)) => n1.cmp(n2), + (Self::String(s1), Self::String(s2)) => s1.cmp(s2), + (Self::Number(_), Self::String(_)) => Ordering::Greater, + (Self::String(_), Self::Number(_)) => Ordering::Less, + } + } +} + +impl FromStr for Version { + type Err = String; + + /// Parses a version such as `1.19`, `1.0a1`,`1.0+abc.5` or `1!2012.2` + /// + /// Note that this variant doesn't allow the version to end with a star, see + /// [`Self::from_str_star`] if you want to parse versions for specifiers + fn from_str(version: &str) -> Result { + if let Some(v) = version_fast_parse(version) { + return Ok(v); + } + + let captures = VERSION_RE + .captures(version) + .ok_or_else(|| format!("Version `{version}` doesn't match PEP 440 rules"))?; + let (version, star) = Version::parse_impl(&captures)?; + if star { + return Err("A star (`*`) must not be used in a fixed version (use `Version::from_string_star` otherwise)".to_string()); + } + Ok(version) + } +} + +/// Workaround for +#[cfg(feature = "pyo3")] +#[derive(Clone, Debug)] +#[pyclass(name = "Version")] +pub struct PyVersion(pub Version); + +#[cfg(feature = "pyo3")] +#[pymethods] +impl PyVersion { + /// The [versioning epoch](https://peps.python.org/pep-0440/#version-epochs). Normally just 0, + /// but you can increment it if you switched the versioning scheme. + #[getter] + pub fn epoch(&self) -> u64 { + self.0.epoch + } + /// The normal number part of the version + /// (["final release"](https://peps.python.org/pep-0440/#final-releases)), + /// such a `1.2.3` in `4!1.2.3-a8.post9.dev1` + /// + /// Note that we drop the * placeholder by moving it to `Operator` + #[getter] + pub fn release(&self) -> Vec { + self.0.release.clone() + } + /// The [prerelease](https://peps.python.org/pep-0440/#pre-releases), i.e. alpha, beta or rc + /// plus a number + /// + /// Note that whether this is Some influences the version + /// range matching since normally we exclude all prerelease versions + #[getter] + pub fn pre(&self) -> Option<(PreRelease, u64)> { + self.0.pre + } + /// The [Post release version](https://peps.python.org/pep-0440/#post-releases), + /// higher post version are preferred over lower post or none-post versions + #[getter] + pub fn post(&self) -> Option { + self.0.post + } + /// The [developmental release](https://peps.python.org/pep-0440/#developmental-releases), + /// if any + #[getter] + pub fn dev(&self) -> Option { + self.0.dev + } + /// The first item of release or 0 if unavailable. + #[getter] + #[allow(clippy::get_first)] + pub fn major(&self) -> u64 { + self.0.release.get(0).copied().unwrap_or_default() + } + /// The second item of release or 0 if unavailable. + #[getter] + pub fn minor(&self) -> u64 { + self.0.release.get(1).copied().unwrap_or_default() + } + /// The third item of release or 0 if unavailable. + #[getter] + pub fn micro(&self) -> u64 { + self.0.release.get(2).copied().unwrap_or_default() + } + + /// Parses a PEP 440 version string + #[cfg(feature = "pyo3")] + #[new] + pub fn parse(version: &str) -> PyResult { + Ok(Self( + Version::from_str(version).map_err(PyValueError::new_err)?, + )) + } + + // Maps the error type + /// Parse a PEP 440 version optionally ending with `.*` + #[cfg(feature = "pyo3")] + #[staticmethod] + pub fn parse_star(version_specifier: &str) -> PyResult<(Self, bool)> { + Version::from_str_star(version_specifier) + .map_err(PyValueError::new_err) + .map(|(version, star)| (Self(version), star)) + } + + /// Returns the normalized representation + #[cfg(feature = "pyo3")] + pub fn __str__(&self) -> String { + self.0.to_string() + } + + /// Returns the normalized representation + #[cfg(feature = "pyo3")] + pub fn __repr__(&self) -> String { + format!(r#""#, self.0) + } + + /// Returns the normalized representation + #[cfg(feature = "pyo3")] + pub fn __hash__(&self) -> u64 { + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + self.0.hash(&mut hasher); + hasher.finish() + } + + #[cfg(feature = "pyo3")] + fn __richcmp__(&self, other: &Self, op: CompareOp) -> bool { + op.matches(self.0.cmp(&other.0)) + } + + fn any_prerelease(&self) -> bool { + self.0.any_prerelease() + } +} + +#[cfg(feature = "pyo3")] +impl IntoPy for Version { + fn into_py(self, py: Python<'_>) -> PyObject { + PyVersion(self).into_py(py) + } +} + +#[cfg(feature = "pyo3")] +impl<'source> FromPyObject<'source> for Version { + fn extract(ob: &'source PyAny) -> PyResult { + Ok(ob.extract::()?.0) + } +} + +/// Compare the release parts of two versions, e.g. `4.3.1` > `4.2`, `1.1.0` == +/// `1.1` and `1.16` < `1.19` +pub(crate) fn compare_release(this: &[u64], other: &[u64]) -> Ordering { + // "When comparing release segments with different numbers of components, the shorter segment + // is padded out with additional zeros as necessary" + for (this, other) in this.iter().chain(std::iter::repeat(&0)).zip( + other + .iter() + .chain(std::iter::repeat(&0)) + .take(this.len().max(other.len())), + ) { + match this.cmp(other) { + Ordering::Less => { + return Ordering::Less; + } + Ordering::Equal => {} + Ordering::Greater => { + return Ordering::Greater; + } + } + } + Ordering::Equal +} + +/// Compare the parts attached after the release, given equal release +/// +/// According to [a summary of permitted suffixes and relative +/// ordering][pep440-suffix-ordering] the order of pre/post-releases is: .devN, +/// aN, bN, rcN, , .postN but also, you can have dev/post +/// releases on beta releases, so we make a three stage ordering: ({dev: 0, a: +/// 1, b: 2, rc: 3, (): 4, post: 5}, , , , ) +/// +/// For post, any number is better than none (so None defaults to None<0), +/// but for dev, no number is better (so None default to the maximum). For +/// local the Option> luckily already has the correct default Ord +/// implementation +/// +/// [pep440-suffix-ordering]: https://peps.python.org/pep-0440/#summary-of-permitted-suffixes-and-relative-ordering +fn sortable_tuple(version: &Version) -> (u64, u64, Option, u64, Option<&[LocalSegment]>) { + match (&version.pre, &version.post, &version.dev) { + // dev release + (None, None, Some(n)) => (0, 0, None, *n, version.local.as_deref()), + // alpha release + (Some((PreRelease::Alpha, n)), post, dev) => ( + 1, + *n, + *post, + dev.unwrap_or(u64::MAX), + version.local.as_deref(), + ), + // beta release + (Some((PreRelease::Beta, n)), post, dev) => ( + 2, + *n, + *post, + dev.unwrap_or(u64::MAX), + version.local.as_deref(), + ), + // alpha release + (Some((PreRelease::Rc, n)), post, dev) => ( + 3, + *n, + *post, + dev.unwrap_or(u64::MAX), + version.local.as_deref(), + ), + // final release + (None, None, None) => (4, 0, None, 0, version.local.as_deref()), + // post release + (None, Some(post), dev) => ( + 5, + 0, + Some(*post), + dev.unwrap_or(u64::MAX), + version.local.as_deref(), + ), + } +} + /// Attempt to parse the given version string very quickly. /// /// This looks for a version string that is of the form `n(.n)*` (i.e., release @@ -855,77 +955,341 @@ fn version_fast_parse(version: &str) -> Option { } #[cfg(test)] -mod test { - #[cfg(feature = "pyo3")] - use pyo3::pyfunction; +mod tests { use std::str::FromStr; - use crate::{Version, VersionSpecifier}; + #[cfg(feature = "pyo3")] + use pyo3::pyfunction; + + use crate::{LocalSegment, PreRelease, Version, VersionSpecifier}; /// #[test] fn test_packaging_versions() { let versions = [ // Implicit epoch of 0 - "1.0.dev456", - "1.0a1", - "1.0a2.dev456", - "1.0a12.dev456", - "1.0a12", - "1.0b1.dev456", - "1.0b2", - "1.0b2.post345.dev456", - "1.0b2.post345", - "1.0b2-346", - "1.0c1.dev456", - "1.0c1", - "1.0rc2", - "1.0c3", - "1.0", - "1.0.post456.dev34", - "1.0.post456", - "1.1.dev1", - "1.2+123abc", - "1.2+123abc456", - "1.2+abc", - "1.2+abc123", - "1.2+abc123def", - "1.2+1234.abc", - "1.2+123456", - "1.2.r32+123456", - "1.2.rev33+123456", + ("1.0.dev456", Version::new([1, 0]).with_dev(Some(456))), + ( + "1.0a1", + Version::new([1, 0]).with_pre(Some((PreRelease::Alpha, 1))), + ), + ( + "1.0a2.dev456", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Alpha, 2))) + .with_dev(Some(456)), + ), + ( + "1.0a12.dev456", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Alpha, 12))) + .with_dev(Some(456)), + ), + ( + "1.0a12", + Version::new([1, 0]).with_pre(Some((PreRelease::Alpha, 12))), + ), + ( + "1.0b1.dev456", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Beta, 1))) + .with_dev(Some(456)), + ), + ( + "1.0b2", + Version::new([1, 0]).with_pre(Some((PreRelease::Beta, 2))), + ), + ( + "1.0b2.post345.dev456", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Beta, 2))) + .with_dev(Some(456)) + .with_post(Some(345)), + ), + ( + "1.0b2.post345", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Beta, 2))) + .with_post(Some(345)), + ), + ( + "1.0b2-346", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Beta, 2))) + .with_post(Some(346)), + ), + ( + "1.0c1.dev456", + Version::new([1, 0]) + .with_pre(Some((PreRelease::Rc, 1))) + .with_dev(Some(456)), + ), + ( + "1.0c1", + Version::new([1, 0]).with_pre(Some((PreRelease::Rc, 1))), + ), + ( + "1.0rc2", + Version::new([1, 0]).with_pre(Some((PreRelease::Rc, 2))), + ), + ( + "1.0c3", + Version::new([1, 0]).with_pre(Some((PreRelease::Rc, 3))), + ), + ("1.0", Version::new([1, 0])), + ( + "1.0.post456.dev34", + Version::new([1, 0]).with_post(Some(456)).with_dev(Some(34)), + ), + ("1.0.post456", Version::new([1, 0]).with_post(Some(456))), + ("1.1.dev1", Version::new([1, 1]).with_dev(Some(1))), + ( + "1.2+123abc", + Version::new([1, 2]).with_local(vec![LocalSegment::String("123abc".to_string())]), + ), + ( + "1.2+123abc456", + Version::new([1, 2]) + .with_local(vec![LocalSegment::String("123abc456".to_string())]), + ), + ( + "1.2+abc", + Version::new([1, 2]).with_local(vec![LocalSegment::String("abc".to_string())]), + ), + ( + "1.2+abc123", + Version::new([1, 2]).with_local(vec![LocalSegment::String("abc123".to_string())]), + ), + ( + "1.2+abc123def", + Version::new([1, 2]) + .with_local(vec![LocalSegment::String("abc123def".to_string())]), + ), + ( + "1.2+1234.abc", + Version::new([1, 2]).with_local(vec![ + LocalSegment::Number(1234), + LocalSegment::String("abc".to_string()), + ]), + ), + ( + "1.2+123456", + Version::new([1, 2]).with_local(vec![LocalSegment::Number(123456)]), + ), + ( + "1.2.r32+123456", + Version::new([1, 2]) + .with_post(Some(32)) + .with_local(vec![LocalSegment::Number(123456)]), + ), + ( + "1.2.rev33+123456", + Version::new([1, 2]) + .with_post(Some(33)) + .with_local(vec![LocalSegment::Number(123456)]), + ), // Explicit epoch of 1 - "1!1.0.dev456", - "1!1.0a1", - "1!1.0a2.dev456", - "1!1.0a12.dev456", - "1!1.0a12", - "1!1.0b1.dev456", - "1!1.0b2", - "1!1.0b2.post345.dev456", - "1!1.0b2.post345", - "1!1.0b2-346", - "1!1.0c1.dev456", - "1!1.0c1", - "1!1.0rc2", - "1!1.0c3", - "1!1.0", - "1!1.0.post456.dev34", - "1!1.0.post456", - "1!1.1.dev1", - "1!1.2+123abc", - "1!1.2+123abc456", - "1!1.2+abc", - "1!1.2+abc123", - "1!1.2+abc123def", - "1!1.2+1234.abc", - "1!1.2+123456", - "1!1.2.r32+123456", - "1!1.2.rev33+123456", + ( + "1!1.0.dev456", + Version::new([1, 0]).with_epoch(1).with_dev(Some(456)), + ), + ( + "1!1.0a1", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Alpha, 1))), + ), + ( + "1!1.0a2.dev456", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Alpha, 2))) + .with_dev(Some(456)), + ), + ( + "1!1.0a12.dev456", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Alpha, 12))) + .with_dev(Some(456)), + ), + ( + "1!1.0a12", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Alpha, 12))), + ), + ( + "1!1.0b1.dev456", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Beta, 1))) + .with_dev(Some(456)), + ), + ( + "1!1.0b2", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Beta, 2))), + ), + ( + "1!1.0b2.post345.dev456", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Beta, 2))) + .with_post(Some(345)) + .with_dev(Some(456)), + ), + ( + "1!1.0b2.post345", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Beta, 2))) + .with_post(Some(345)), + ), + ( + "1!1.0b2-346", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Beta, 2))) + .with_post(Some(346)), + ), + ( + "1!1.0c1.dev456", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Rc, 1))) + .with_dev(Some(456)), + ), + ( + "1!1.0c1", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Rc, 1))), + ), + ( + "1!1.0rc2", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Rc, 2))), + ), + ( + "1!1.0c3", + Version::new([1, 0]) + .with_epoch(1) + .with_pre(Some((PreRelease::Rc, 3))), + ), + ("1!1.0", Version::new([1, 0]).with_epoch(1)), + ( + "1!1.0.post456.dev34", + Version::new([1, 0]) + .with_epoch(1) + .with_post(Some(456)) + .with_dev(Some(34)), + ), + ( + "1!1.0.post456", + Version::new([1, 0]).with_epoch(1).with_post(Some(456)), + ), + ( + "1!1.1.dev1", + Version::new([1, 1]).with_epoch(1).with_dev(Some(1)), + ), + ( + "1!1.2+123abc", + Version::new([1, 2]) + .with_epoch(1) + .with_local(vec![LocalSegment::String("123abc".to_string())]), + ), + ( + "1!1.2+123abc456", + Version::new([1, 2]) + .with_epoch(1) + .with_local(vec![LocalSegment::String("123abc456".to_string())]), + ), + ( + "1!1.2+abc", + Version::new([1, 2]) + .with_epoch(1) + .with_local(vec![LocalSegment::String("abc".to_string())]), + ), + ( + "1!1.2+abc123", + Version::new([1, 2]) + .with_epoch(1) + .with_local(vec![LocalSegment::String("abc123".to_string())]), + ), + ( + "1!1.2+abc123def", + Version::new([1, 2]) + .with_epoch(1) + .with_local(vec![LocalSegment::String("abc123def".to_string())]), + ), + ( + "1!1.2+1234.abc", + Version::new([1, 2]).with_epoch(1).with_local(vec![ + LocalSegment::Number(1234), + LocalSegment::String("abc".to_string()), + ]), + ), + ( + "1!1.2+123456", + Version::new([1, 2]) + .with_epoch(1) + .with_local(vec![LocalSegment::Number(123456)]), + ), + ( + "1!1.2.r32+123456", + Version::new([1, 2]) + .with_epoch(1) + .with_post(Some(32)) + .with_local(vec![LocalSegment::Number(123456)]), + ), + ( + "1!1.2.rev33+123456", + Version::new([1, 2]) + .with_epoch(1) + .with_post(Some(33)) + .with_local(vec![LocalSegment::Number(123456)]), + ), + ( + "98765!1.2.rev33+123456", + Version::new([1, 2]) + .with_epoch(98765) + .with_post(Some(33)) + .with_local(vec![LocalSegment::Number(123456)]), + ), ]; - for version in versions { - Version::from_str(version).unwrap(); - VersionSpecifier::from_str(&format!("=={version}")).unwrap(); + for (string, structured) in versions { + match Version::from_str(string) { + Err(err) => { + unreachable!( + "expected {string:?} to parse as {structured:?}, but got {err:?}", + structured = structured.as_bloated_debug(), + ) + } + Ok(v) => assert!( + v == structured, + "for {string:?}, expected {structured:?} but got {v:?}", + structured = structured.as_bloated_debug(), + v = v.as_bloated_debug(), + ), + } + let spec = format!("=={string}"); + match VersionSpecifier::from_str(&spec) { + Err(err) => { + unreachable!( + "expected version in {spec:?} to parse as {structured:?}, but got {err:?}", + structured = structured.as_bloated_debug(), + ) + } + Ok(v) => assert!( + v.version() == &structured, + "for {string:?}, expected {structured:?} but got {v:?}", + structured = structured.as_bloated_debug(), + v = v.version.as_bloated_debug(), + ), + } } } @@ -1206,4 +1570,35 @@ mod test { fn _convert_in_and_out(version: Version) -> Version { version } + + /// Wraps a `Version` and provides a more "bloated" debug but standard + /// representation. + /// + /// We don't do this by default because it takes up a ton of space, and + /// just printing out the display version of the version is quite a bit + /// simpler. + /// + /// Nevertheless, when *testing* version parsing, you really want to + /// be able to peek at all of its constituent parts. So we use this in + /// assertion failure messages. + struct VersionBloatedDebug<'a>(&'a Version); + + impl<'a> std::fmt::Debug for VersionBloatedDebug<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Version") + .field("epoch", &self.0.epoch()) + .field("release", &self.0.release()) + .field("pre", &self.0.pre()) + .field("post", &self.0.post()) + .field("dev", &self.0.dev()) + .field("local", &self.0.local()) + .finish() + } + } + + impl Version { + fn as_bloated_debug(&self) -> VersionBloatedDebug<'_> { + VersionBloatedDebug(self) + } + } } diff --git a/crates/pep440-rs/src/version_specifier.rs b/crates/pep440-rs/src/version_specifier.rs index 3dc4f8446..2cb640854 100644 --- a/crates/pep440-rs/src/version_specifier.rs +++ b/crates/pep440-rs/src/version_specifier.rs @@ -1,6 +1,7 @@ #[cfg(feature = "pyo3")] -use crate::version::PyVersion; -use crate::{version, Operator, Pep440Error, Version}; +use std::hash::{Hash, Hasher}; +use std::{cmp::Ordering, str::FromStr}; + #[cfg(feature = "pyo3")] use pyo3::{ exceptions::{PyIndexError, PyNotImplementedError, PyValueError}, @@ -10,20 +11,11 @@ use pyo3::{ }; #[cfg(feature = "serde")] use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; -use std::cmp::Ordering; -#[cfg(feature = "pyo3")] -use std::collections::hash_map::DefaultHasher; -use std::fmt::Formatter; -use std::fmt::{Debug, Display}; -#[cfg(feature = "pyo3")] -use std::hash::{Hash, Hasher}; -use std::ops::Deref; -use std::str::FromStr; use unicode_width::UnicodeWidthStr; -#[cfg(feature = "tracing")] -use tracing::warn; -use unscanny::Scanner; +#[cfg(feature = "pyo3")] +use crate::version::PyVersion; +use crate::{version, Operator, Pep440Error, Version}; /// A thin wrapper around `Vec` with a serde implementation /// @@ -46,7 +38,7 @@ use unscanny::Scanner; #[cfg_attr(feature = "pyo3", pyclass(sequence))] pub struct VersionSpecifiers(Vec); -impl Deref for VersionSpecifiers { +impl std::ops::Deref for VersionSpecifiers { type Target = [VersionSpecifier]; fn deref(&self) -> &Self::Target { @@ -81,8 +73,8 @@ impl From for VersionSpecifiers { } } -impl Display for VersionSpecifiers { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl std::fmt::Display for VersionSpecifiers { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { for (idx, version_specifier) in self.0.iter().enumerate() { // Separate version specifiers by comma, but we need one comma less than there are // specifiers @@ -255,7 +247,7 @@ impl VersionSpecifier { /// Returns the normalized representation pub fn __hash__(&self) -> u64 { - let mut hasher = DefaultHasher::new(); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); self.hash(&mut hasher); hasher.finish() } @@ -289,7 +281,7 @@ impl VersionSpecifier { /// parameter indicates a trailing `.*`, to differentiate between `1.1.*` and `1.1` pub fn new(operator: Operator, version: Version, star: bool) -> Result { // "Local version identifiers are NOT permitted in this version specifier." - if let Some(local) = &version.local { + if let Some(local) = &version.local() { if matches!( operator, Operator::GreaterThan @@ -327,7 +319,7 @@ impl VersionSpecifier { operator }; - if operator == Operator::TildeEqual && version.release.len() < 2 { + if operator == Operator::TildeEqual && version.release().len() < 2 { return Err( "The ~= operator requires at least two parts in the release version".to_string(), ); @@ -379,39 +371,42 @@ impl VersionSpecifier { // "Except where specifically noted below, local version identifiers MUST NOT be permitted // in version specifiers, and local version labels MUST be ignored entirely when checking // if candidate versions match a given version specifier." - let (this, other) = if self.version.local.is_some() { + let (this, other) = if self.version.local().is_some() { (self.version.clone(), version.clone()) } else { // self is already without local - (self.version.without_local(), version.without_local()) + ( + self.version.clone().without_local(), + version.clone().without_local(), + ) }; match self.operator { Operator::Equal => other == this, Operator::EqualStar => { - this.epoch == other.epoch + this.epoch() == other.epoch() && self .version - .release + .release() .iter() - .zip(&other.release) + .zip(other.release()) .all(|(this, other)| this == other) } #[allow(deprecated)] Operator::ExactEqual => { #[cfg(feature = "tracing")] { - warn!("Using arbitrary equality (`===`) is discouraged"); + tracing::warn!("Using arbitrary equality (`===`) is discouraged"); } self.version.to_string() == version.to_string() } Operator::NotEqual => other != this, Operator::NotEqualStar => { - this.epoch != other.epoch + this.epoch() != other.epoch() || !this - .release + .release() .iter() - .zip(&version.release) + .zip(version.release()) .all(|(this, other)| this == other) } Operator::TildeEqual => { @@ -419,14 +414,14 @@ impl VersionSpecifier { // approximately equivalent to the pair of comparison clauses: `>= V.N, == V.*`" // First, we test that every but the last digit matches. // We know that this must hold true since we checked it in the constructor - assert!(this.release.len() > 1); - if this.epoch != other.epoch { + assert!(this.release().len() > 1); + if this.epoch() != other.epoch() { return false; } - if !this.release[..this.release.len() - 1] + if !this.release()[..this.release().len() - 1] .iter() - .zip(&other.release) + .zip(other.release()) .all(|(this, other)| this == other) { return false; @@ -440,7 +435,8 @@ impl VersionSpecifier { Operator::GreaterThanEqual => Self::greater_than(&this, &other) || other >= this, Operator::LessThan => { Self::less_than(&this, &other) - && !(version::compare_release(&this.release, &other.release) == Ordering::Equal + && !(version::compare_release(this.release(), other.release()) + == Ordering::Equal && other.any_prerelease()) } Operator::LessThanEqual => Self::less_than(&this, &other) || other <= this, @@ -448,7 +444,7 @@ impl VersionSpecifier { } fn less_than(this: &Version, other: &Version) -> bool { - if other.epoch < this.epoch { + if other.epoch() < this.epoch() { return true; } @@ -458,7 +454,7 @@ impl VersionSpecifier { // not match 3.1.dev0, but should match 3.0.dev0). if !this.any_prerelease() && other.is_pre() - && version::compare_release(&this.release, &other.release) == Ordering::Equal + && version::compare_release(this.release(), other.release()) == Ordering::Equal { return false; } @@ -467,11 +463,11 @@ impl VersionSpecifier { } fn greater_than(this: &Version, other: &Version) -> bool { - if other.epoch > this.epoch { + if other.epoch() > this.epoch() { return true; } - if version::compare_release(&this.release, &other.release) == Ordering::Equal { + if version::compare_release(this.release(), other.release()) == Ordering::Equal { // This special case is here so that, unless the specifier itself // includes is a post-release version, that we do not accept // post-release versions for the version mentioned in the specifier @@ -495,7 +491,7 @@ impl FromStr for VersionSpecifier { /// Parses a version such as `>= 1.19`, `== 1.1.*`,`~=1.0+abc.5` or `<=1!2012.2` fn from_str(spec: &str) -> Result { - let mut s = Scanner::new(spec); + let mut s = unscanny::Scanner::new(spec); s.eat_while(|c: char| c.is_whitespace()); // operator but we don't know yet if it has a star let operator = s.eat_while(['=', '!', '~', '<', '>']); @@ -518,8 +514,8 @@ impl FromStr for VersionSpecifier { } } -impl Display for VersionSpecifier { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl std::fmt::Display for VersionSpecifier { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if self.operator == Operator::EqualStar || self.operator == Operator::NotEqualStar { return write!(f, "{}{}.*", self.operator, self.version); } @@ -567,11 +563,12 @@ pub fn parse_version_specifiers(spec: &str) -> Result, Pep } #[cfg(test)] -mod test { - use crate::{Operator, Version, VersionSpecifier, VersionSpecifiers}; +mod tests { + use std::{cmp::Ordering, str::FromStr}; + use indoc::indoc; - use std::cmp::Ordering; - use std::str::FromStr; + + use crate::{Operator, Version, VersionSpecifier, VersionSpecifiers}; /// #[test] @@ -1062,47 +1059,19 @@ mod test { [ VersionSpecifier { operator: Operator::TildeEqual, - version: Version { - epoch: 0, - release: vec![0, 9], - pre: None, - post: None, - dev: None, - local: None - } + version: Version::new([0, 9]), }, VersionSpecifier { operator: Operator::GreaterThanEqual, - version: Version { - epoch: 0, - release: vec![1, 0], - pre: None, - post: None, - dev: None, - local: None - } + version: Version::new([1, 0]), }, VersionSpecifier { operator: Operator::NotEqualStar, - version: Version { - epoch: 0, - release: vec![1, 3, 4], - pre: None, - post: None, - dev: None, - local: None - } + version: Version::new([1, 3, 4]), }, VersionSpecifier { operator: Operator::LessThan, - version: Version { - epoch: 0, - release: vec![2, 0], - pre: None, - post: None, - dev: None, - local: None - } + version: Version::new([2, 0]), } ] ); diff --git a/crates/pep508-rs/src/lib.rs b/crates/pep508-rs/src/lib.rs index eae8015ac..0d0a02764 100644 --- a/crates/pep508-rs/src/lib.rs +++ b/crates/pep508-rs/src/lib.rs @@ -977,30 +977,11 @@ mod tests { [ VersionSpecifier::new( Operator::GreaterThanEqual, - Version { - epoch: 0, - release: vec![2, 8, 1], - pre: None, - post: None, - dev: None, - local: None, - }, + Version::new([2, 8, 1]), false, ) .unwrap(), - VersionSpecifier::new( - Operator::Equal, - Version { - epoch: 0, - release: vec![2, 8], - pre: None, - post: None, - dev: None, - local: None, - }, - true, - ) - .unwrap(), + VersionSpecifier::new(Operator::Equal, Version::new([2, 8]), true).unwrap(), ] .into_iter() .collect(), diff --git a/crates/pep508-rs/src/marker.rs b/crates/pep508-rs/src/marker.rs index 2d9058422..8a869ef85 100644 --- a/crates/pep508-rs/src/marker.rs +++ b/crates/pep508-rs/src/marker.rs @@ -715,14 +715,14 @@ impl MarkerExpression { /// /// # fn main() -> Result<(), Pep508Error> { /// let marker_tree = MarkerTree::from_str(r#"("linux" in sys_platform) and extra == 'day'"#)?; - /// let versions: Vec = (8..12).map(|minor| Version::from_release(vec![3, minor])).collect(); + /// let versions: Vec = (8..12).map(|minor| Version::new([3, minor])).collect(); /// assert!(marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &versions)); /// assert!(!marker_tree.evaluate_extras_and_python_version(&["night".to_string()].into(), &versions)); /// /// let marker_tree = MarkerTree::from_str(r#"extra == 'day' and python_version < '3.11' and '3.10' <= python_version"#)?; - /// assert!(!marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &vec![Version::from_release(vec![3, 9])])); - /// assert!(marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &vec![Version::from_release(vec![3, 10])])); - /// assert!(!marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &vec![Version::from_release(vec![3, 11])])); + /// assert!(!marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &vec![Version::new([3, 9])])); + /// assert!(marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &vec![Version::new([3, 10])])); + /// assert!(!marker_tree.evaluate_extras_and_python_version(&["day".to_string()].into(), &vec![Version::new([3, 11])])); /// # Ok(()) /// # } /// ``` @@ -1556,7 +1556,7 @@ mod test { fn test_marker_environment_from_json() { let _env: MarkerEnvironment = serde_json::from_str( r##"{ - "implementation_name": "cpython", + "implementation_name": "cpython", "implementation_version": "3.7.13", "os_name": "posix", "platform_machine": "x86_64", diff --git a/crates/puffin-cli/src/python_version.rs b/crates/puffin-cli/src/python_version.rs index b6c16ad25..a8561b1e6 100644 --- a/crates/puffin-cli/src/python_version.rs +++ b/crates/puffin-cli/src/python_version.rs @@ -18,18 +18,18 @@ impl FromStr for PythonVersion { if version.is_local() { return Err(format!("Python version {s} is a local version")); } - if version.epoch != 0 { + if version.epoch() != 0 { return Err(format!("Python version {s} has a non-zero epoch")); } - if version.version < Version::from_release(vec![3, 7]) { + if version.version < Version::new([3, 7]) { return Err(format!("Python version {s} must be >= 3.7")); } - if version.version >= Version::from_release(vec![4, 0]) { + if version.version >= Version::new([4, 0]) { return Err(format!("Python version {s} must be < 4.0")); } // If the version lacks a patch, assume the most recent known patch for that minor version. - match version.release.as_slice() { + match version.release() { [3, 7] => { debug!("Assuming Python 3.7.17"); Ok(Self(StringVersion::from_str("3.7.17")?)) diff --git a/crates/puffin-interpreter/src/interpreter.rs b/crates/puffin-interpreter/src/interpreter.rs index 9f11b3ca4..029c6112d 100644 --- a/crates/puffin-interpreter/src/interpreter.rs +++ b/crates/puffin-interpreter/src/interpreter.rs @@ -82,8 +82,8 @@ impl Interpreter { /// Returns the Python version as a simple tuple. pub fn simple_version(&self) -> (u8, u8) { ( - u8::try_from(self.version().release[0]).expect("invalid major version"), - u8::try_from(self.version().release[1]).expect("invalid minor version"), + u8::try_from(self.version().release()[0]).expect("invalid major version"), + u8::try_from(self.version().release()[1]).expect("invalid minor version"), ) } diff --git a/crates/puffin-resolver/src/pubgrub/specifier.rs b/crates/puffin-resolver/src/pubgrub/specifier.rs index 00fe25490..8bc0df317 100644 --- a/crates/puffin-resolver/src/pubgrub/specifier.rs +++ b/crates/puffin-resolver/src/pubgrub/specifier.rs @@ -36,21 +36,14 @@ impl TryFrom<&VersionSpecifier> for PubGrubSpecifier { Range::singleton(version).complement() } Operator::TildeEqual => { - let [rest @ .., last, _] = specifier.version().release.as_slice() else { + let [rest @ .., last, _] = specifier.version().release() else { return Err(ResolveError::InvalidTildeEquals(specifier.clone())); }; - let upper = PubGrubVersion::from(pep440_rs::Version { - dev: Some(0), - epoch: specifier.version().epoch, - local: None, - post: None, - pre: None, - release: rest - .iter() - .chain(std::iter::once(&(last + 1))) - .copied() - .collect(), - }); + let upper = PubGrubVersion::from( + pep440_rs::Version::new(rest.iter().chain([&(last + 1)])) + .with_epoch(specifier.version().epoch()) + .with_dev(Some(0)), + ); let version = PubGrubVersion::from(specifier.version().clone()); Range::from_range_bounds(version..upper) } @@ -65,14 +58,6 @@ impl TryFrom<&VersionSpecifier> for PubGrubSpecifier { Operator::GreaterThan => { // Per PEP 440: "The exclusive ordered comparison >V MUST NOT allow a post-release of // the given version unless V itself is a post release." - let mut low = specifier.version().clone(); - if let Some(dev) = low.dev { - low.dev = Some(dev + 1); - } else if let Some(post) = low.post { - low.post = Some(post + 1); - } else { - low.post = Some(u64::MAX); - } let version = PubGrubVersion::from(specifier.version().clone()); Range::strictly_higher_than(version) } @@ -81,46 +66,38 @@ impl TryFrom<&VersionSpecifier> for PubGrubSpecifier { Range::higher_than(version) } Operator::EqualStar => { - let low = pep440_rs::Version { - dev: Some(0), - ..specifier.version().clone() - }; - let mut high = pep440_rs::Version { - dev: Some(0), - ..specifier.version().clone() - }; - if let Some(post) = high.post { - high.post = Some(post + 1); - } else if let Some(pre) = high.pre { - high.pre = Some(match pre { + let low = specifier.version().clone().with_dev(Some(0)); + let mut high = low.clone(); + if let Some(post) = high.post() { + high = high.with_post(Some(post + 1)); + } else if let Some(pre) = high.pre() { + high = high.with_pre(Some(match pre { (pep440_rs::PreRelease::Rc, n) => (pep440_rs::PreRelease::Rc, n + 1), (pep440_rs::PreRelease::Alpha, n) => (pep440_rs::PreRelease::Alpha, n + 1), (pep440_rs::PreRelease::Beta, n) => (pep440_rs::PreRelease::Beta, n + 1), - }); + })); } else { - *high.release.last_mut().unwrap() += 1; + let mut release = high.release().to_vec(); + *release.last_mut().unwrap() += 1; + high = high.with_release(release); } Range::from_range_bounds(PubGrubVersion::from(low)..PubGrubVersion::from(high)) } Operator::NotEqualStar => { - let low = pep440_rs::Version { - dev: Some(0), - ..specifier.version().clone() - }; - let mut high = pep440_rs::Version { - dev: Some(0), - ..specifier.version().clone() - }; - if let Some(post) = high.post { - high.post = Some(post + 1); - } else if let Some(pre) = high.pre { - high.pre = Some(match pre { + let low = specifier.version().clone().with_dev(Some(0)); + let mut high = low.clone(); + if let Some(post) = high.post() { + high = high.with_post(Some(post + 1)); + } else if let Some(pre) = high.pre() { + high = high.with_pre(Some(match pre { (pep440_rs::PreRelease::Rc, n) => (pep440_rs::PreRelease::Rc, n + 1), (pep440_rs::PreRelease::Alpha, n) => (pep440_rs::PreRelease::Alpha, n + 1), (pep440_rs::PreRelease::Beta, n) => (pep440_rs::PreRelease::Beta, n + 1), - }); + })); } else { - *high.release.last_mut().unwrap() += 1; + let mut release = high.release().to_vec(); + *release.last_mut().unwrap() += 1; + high = high.with_release(release); } Range::from_range_bounds(PubGrubVersion::from(low)..PubGrubVersion::from(high)) .complement() diff --git a/crates/puffin-resolver/src/pubgrub/version.rs b/crates/puffin-resolver/src/pubgrub/version.rs index 71c27518b..818fb1ff3 100644 --- a/crates/puffin-resolver/src/pubgrub/version.rs +++ b/crates/puffin-resolver/src/pubgrub/version.rs @@ -10,13 +10,12 @@ impl PubGrubVersion { /// Returns the smallest PEP 440 version that is larger than `self`. pub fn next(&self) -> PubGrubVersion { let mut next = self.clone(); - if let Some(dev) = &mut next.0.dev { - *dev += 1; - } else if let Some(post) = &mut next.0.post { - *post += 1; + if let Some(dev) = next.0.dev() { + next.0 = next.0.with_dev(Some(dev + 1)); + } else if let Some(post) = next.0.post() { + next.0 = next.0.with_post(Some(post + 1)); } else { - next.0.post = Some(0); - next.0.dev = Some(0); + next.0 = next.0.with_post(Some(0)).with_dev(Some(0)); } next }