pep440: some minor refactoring, mostly around error types (#780)

This PR does a bit of refactoring to the pep440 crate, and in
particular around the erorr types. This PR is meant to be a precursor
to another PR that does some surgery (both in parsing and in `Version`
representation) that benefits somewhat from this refactoring.

As usual, please review commit-by-commit.

crates/pep440-rs/src/lib.rs

@@ -44,7 +44,9 @@
 
 pub use {
     version::{LocalSegment, Operator, PreRelease, Version},
-    version_specifier::{parse_version_specifiers, VersionSpecifier, VersionSpecifiers},
+    version_specifier::{
+        parse_version_specifiers, VersionSpecifier, VersionSpecifiers, VersionSpecifiersParseError,
+    },
 };
 
 #[cfg(feature = "pyo3")]
@@ -55,30 +57,6 @@ pub use version::PyVersion;
 mod version;
 mod version_specifier;
 
-/// Error with span information (unicode width) inside the parsed line
-#[derive(Debug, Eq, PartialEq, Clone)]
-pub struct Pep440Error {
-    /// The actual error message
-    pub message: String,
-    /// The string that failed to parse
-    pub line: String,
-    /// First character for underlining (unicode width)
-    pub start: usize,
-    /// Number of characters to underline (unicode width)
-    pub width: usize,
-}
-
-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))?;
-        Ok(())
-    }
-}
-
-impl std::error::Error for Pep440Error {}
-
 /// Python bindings shipped as `pep440_rs`
 #[cfg(feature = "pyo3")]
 #[pymodule]

crates/pep440-rs/src/version.rs

@@ -56,7 +56,7 @@ static VERSION_RE: Lazy<Regex> =
     Lazy::new(|| Regex::new(&format!(r#"(?xi)^(?:\s*){VERSION_RE_INNER}(?:\s*)$"#)).unwrap());
 
 /// One of `~=` `==` `!=` `<=` `>=` `<` `>` `===`
-#[derive(Eq, PartialEq, Debug, Hash, Clone)]
+#[derive(Eq, PartialEq, Debug, Hash, Clone, Copy)]
 #[cfg_attr(feature = "pyo3", pyclass)]
 pub enum Operator {
     /// `== 1.2.3`
@@ -86,6 +86,41 @@ pub enum Operator {
     GreaterThanEqual,
 }
 
+impl Operator {
+    /// Returns true if and only if this operator can be used in a version
+    /// specifier with a version containing a non-empty local segment.
+    ///
+    /// Specifically, this comes from the "Local version identifiers are
+    /// NOT permitted in this version specifier." phrasing in the version
+    /// specifiers [spec].
+    ///
+    /// [spec]: https://packaging.python.org/en/latest/specifications/version-specifiers/
+    pub(crate) fn is_local_compatible(&self) -> bool {
+        !matches!(
+            *self,
+            Operator::GreaterThan
+                | Operator::GreaterThanEqual
+                | Operator::LessThan
+                | Operator::LessThanEqual
+                | Operator::TildeEqual
+                | Operator::EqualStar
+                | Operator::NotEqualStar
+        )
+    }
+
+    /// Returns the wildcard version of this operator, if appropriate.
+    ///
+    /// This returns `None` when this operator doesn't have an analogous
+    /// wildcard operator.
+    pub(crate) fn to_star(self) -> Option<Operator> {
+        match self {
+            Operator::Equal => Some(Operator::EqualStar),
+            Operator::NotEqual => Some(Operator::NotEqualStar),
+            _ => None,
+        }
+    }
+}
+
 impl FromStr for Operator {
     type Err = String;
 
@@ -152,96 +187,67 @@ impl Operator {
     }
 }
 
-/// Optional prerelease modifier (alpha, beta or release candidate) appended to version
+// NOTE: I did a little bit of experimentation to determine what most version
-///
+// numbers actually look like. The idea here is that if we know what most look
-/// <https://peps.python.org/pep-0440/#pre-releases>
+// like, then we can optimize our representation for the common case, while
-#[derive(PartialEq, Eq, Debug, Hash, Clone, Copy, Ord, PartialOrd)]
+// falling back to something more complete for any cases that fall outside of
-#[cfg_attr(feature = "pyo3", pyclass)]
+// that.
-pub enum PreRelease {
+//
-    /// alpha prerelease
+// The experiment downloaded PyPI's distribution metadata from Google BigQuery,
-    Alpha,
+// and then counted the number of versions with various qualities:
-    /// beta prerelease
+//
-    Beta,
+//     total: 11264078
-    /// release candidate prerelease
+//     release counts:
-    Rc,
+//         01: 51204 (0.45%)
-}
+//         02: 754520 (6.70%)
-
+//         03: 9757602 (86.63%)
-impl FromStr for PreRelease {
+//         04: 527403 (4.68%)
-    type Err = String;
+//         05: 77994 (0.69%)
-
+//         06: 91346 (0.81%)
-    fn from_str(prerelease: &str) -> Result<Self, Self::Err> {
+//         07: 1421 (0.01%)
-        match prerelease.to_lowercase().as_str() {
+//         08: 205 (0.00%)
-            "a" | "alpha" => Ok(Self::Alpha),
+//         09: 72 (0.00%)
-            "b" | "beta" => Ok(Self::Beta),
+//         10: 2297 (0.02%)
-            "c" | "rc" | "pre" | "preview" => Ok(Self::Rc),
+//         11: 5 (0.00%)
-            _ => Err(format!(
+//         12: 2 (0.00%)
-                "'{prerelease}' isn't recognized as alpha, beta or release candidate",
+//         13: 4 (0.00%)
-            )),
+//         20: 2 (0.00%)
-        }
+//         39: 1 (0.00%)
-    }
+//     JUST release counts:
-}
+//         01: 48297 (0.43%)
-
+//         02: 604692 (5.37%)
-impl std::fmt::Display for PreRelease {
+//         03: 8460917 (75.11%)
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+//         04: 465354 (4.13%)
-        match self {
+//         05: 49293 (0.44%)
-            Self::Alpha => write!(f, "a"),
+//         06: 25909 (0.23%)
-            Self::Beta => write!(f, "b"),
+//         07: 1413 (0.01%)
-            Self::Rc => write!(f, "rc"),
+//         08: 192 (0.00%)
-        }
+//         09: 72 (0.00%)
-    }
+//         10: 2292 (0.02%)
-}
+//         11: 5 (0.00%)
-
+//         12: 2 (0.00%)
-/// A part of the [local version identifier](<https://peps.python.org/pep-0440/#local-version-identifiers>)
+//         13: 4 (0.00%)
-///
+//         20: 2 (0.00%)
-/// Local versions are a mess:
+//         39: 1 (0.00%)
-///
+//     non-zero epochs: 1902 (0.02%)
-/// > Comparison and ordering of local versions considers each segment of the local version
+//     pre-releases: 752184 (6.68%)
-/// > (divided by a .) separately. If a segment consists entirely of ASCII digits then that section
+//     post-releases: 134383 (1.19%)
-/// > should be considered an integer for comparison purposes and if a segment contains any ASCII
+//     dev-releases: 765099 (6.79%)
-/// > letters then that segment is compared lexicographically with case insensitivity. When
+//     locals: 1 (0.00%)
-/// > comparing a numeric and lexicographic segment, the numeric section always compares as greater
+//     fitsu8: 10388430 (92.23%)
-/// > than the lexicographic segment. Additionally a local version with a great number of segments
+//     sweetspot: 10236089 (90.87%)
-/// > will always compare as greater than a local version with fewer segments, as long as the
+//
-/// > shorter local version’s segments match the beginning of the longer local version’s segments
+// The "JUST release counts" corresponds to versions that only have a release
-/// > exactly.
+// component and nothing else. The "fitsu8" property indicates that all numbers
-///
+// (except for local numeric segments) fit into `u8`. The "sweetspot" property
-/// Luckily the default `Ord` implementation for `Vec<LocalSegment>` matches the PEP 440 rules.
+// consists of any version number with no local part, 4 or fewer parts in the
-#[derive(Eq, PartialEq, Debug, Clone, Hash)]
+// release version and *all* numbers fit into a u8.
-pub enum LocalSegment {
+//
-    /// Not-parseable as integer segment of local version
+// This somewhat confirms what one might expect: the vast majority of versions
-    String(String),
+// (75%) are precisely in the format of `x.y.z`. That is, a version with only a
-    /// Inferred integer segment of local version
+// release version of 3 components.
-    Number(u64),
+//
-}
+// ---AG
-
-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}"),
-        }
-    }
-}
-
-impl PartialOrd for LocalSegment {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl FromStr for LocalSegment {
-    /// This can be a never type when stabilized
-    type Err = ();
-
-    fn from_str(segment: &str) -> Result<Self, Self::Err> {
-        Ok(if let Ok(number) = segment.parse::<u64>() {
-            Self::Number(number)
-        } else {
-            // "and if a segment contains any ASCII letters then that segment is compared lexicographically with case insensitivity"
-            Self::String(segment.to_lowercase())
-        })
-    }
-}
 
 /// A version number such as `1.2.3` or `4!5.6.7-a8.post9.dev0`.
 ///
@@ -290,29 +296,6 @@ pub struct Version {
     local: Option<Vec<LocalSegment>>,
 }
 
-/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
-#[cfg(feature = "serde")]
-impl<'de> Deserialize<'de> for Version {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        let s = String::deserialize(deserializer)?;
-        FromStr::from_str(&s).map_err(de::Error::custom)
-    }
-}
-
-/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
-#[cfg(feature = "serde")]
-impl Serialize for Version {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        serializer.collect_str(self)
-    }
-}
-
 impl Version {
     /// Create a new version from an iterator of segments in the release part
     /// of a version.
@@ -584,6 +567,29 @@ impl Version {
     }
 }
 
+/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
+#[cfg(feature = "serde")]
+impl<'de> Deserialize<'de> for Version {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let s = String::deserialize(deserializer)?;
+        FromStr::from_str(&s).map_err(de::Error::custom)
+    }
+}
+
+/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
+#[cfg(feature = "serde")]
+impl Serialize for Version {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        serializer.collect_str(self)
+    }
+}
+
 /// Shows normalized version
 impl std::fmt::Display for Version {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -691,18 +697,6 @@ impl Ord for Version {
     }
 }
 
-impl Ord for LocalSegment {
-    fn cmp(&self, other: &Self) -> Ordering {
-        // <https://peps.python.org/pep-0440/#local-version-identifiers>
-        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;
 
@@ -726,6 +720,109 @@ impl FromStr for Version {
     }
 }
 
+/// Optional prerelease modifier (alpha, beta or release candidate) appended to version
+///
+/// <https://peps.python.org/pep-0440/#pre-releases>
+#[derive(PartialEq, Eq, Debug, Hash, Clone, Copy, Ord, PartialOrd)]
+#[cfg_attr(feature = "pyo3", pyclass)]
+pub enum PreRelease {
+    /// alpha prerelease
+    Alpha,
+    /// beta prerelease
+    Beta,
+    /// release candidate prerelease
+    Rc,
+}
+
+impl FromStr for PreRelease {
+    type Err = String;
+
+    fn from_str(prerelease: &str) -> Result<Self, Self::Err> {
+        match prerelease.to_lowercase().as_str() {
+            "a" | "alpha" => Ok(Self::Alpha),
+            "b" | "beta" => Ok(Self::Beta),
+            "c" | "rc" | "pre" | "preview" => Ok(Self::Rc),
+            _ => Err(format!(
+                "'{prerelease}' isn't recognized as alpha, beta or release candidate",
+            )),
+        }
+    }
+}
+
+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"),
+            Self::Rc => write!(f, "rc"),
+        }
+    }
+}
+
+/// A part of the [local version identifier](<https://peps.python.org/pep-0440/#local-version-identifiers>)
+///
+/// Local versions are a mess:
+///
+/// > Comparison and ordering of local versions considers each segment of the local version
+/// > (divided by a .) separately. If a segment consists entirely of ASCII digits then that section
+/// > should be considered an integer for comparison purposes and if a segment contains any ASCII
+/// > letters then that segment is compared lexicographically with case insensitivity. When
+/// > comparing a numeric and lexicographic segment, the numeric section always compares as greater
+/// > than the lexicographic segment. Additionally a local version with a great number of segments
+/// > will always compare as greater than a local version with fewer segments, as long as the
+/// > shorter local version’s segments match the beginning of the longer local version’s segments
+/// > exactly.
+///
+/// Luckily the default `Ord` implementation for `Vec<LocalSegment>` matches the PEP 440 rules.
+#[derive(Eq, PartialEq, Debug, Clone, Hash)]
+pub enum LocalSegment {
+    /// Not-parseable as integer segment of local version
+    String(String),
+    /// Inferred integer segment of local version
+    Number(u64),
+}
+
+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}"),
+        }
+    }
+}
+
+impl PartialOrd for LocalSegment {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl FromStr for LocalSegment {
+    /// This can be a never type when stabilized
+    type Err = ();
+
+    fn from_str(segment: &str) -> Result<Self, Self::Err> {
+        Ok(if let Ok(number) = segment.parse::<u64>() {
+            Self::Number(number)
+        } else {
+            // "and if a segment contains any ASCII letters then that segment is compared lexicographically with case insensitivity"
+            Self::String(segment.to_lowercase())
+        })
+    }
+}
+
+impl Ord for LocalSegment {
+    fn cmp(&self, other: &Self) -> Ordering {
+        // <https://peps.python.org/pep-0440/#local-version-identifiers>
+        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,
+        }
+    }
+}
+
 /// Workaround for <https://github.com/PyO3/pyo3/pull/2786>
 #[cfg(feature = "pyo3")]
 #[derive(Clone, Debug)]
@@ -739,7 +836,7 @@ impl PyVersion {
     /// but you can increment it if you switched the versioning scheme.
     #[getter]
     pub fn epoch(&self) -> u64 {
-        self.0.epoch
+        self.0.epoch()
     }
     /// The normal number part of the version
     /// (["final release"](https://peps.python.org/pep-0440/#final-releases)),
@@ -748,7 +845,7 @@ impl PyVersion {
     /// Note that we drop the * placeholder by moving it to `Operator`
     #[getter]
     pub fn release(&self) -> Vec<u64> {
-        self.0.release.clone()
+        self.0.release().to_vec()
     }
     /// The [prerelease](https://peps.python.org/pep-0440/#pre-releases), i.e. alpha, beta or rc
     /// plus a number
@@ -757,35 +854,35 @@ impl PyVersion {
     /// range matching since normally we exclude all prerelease versions
     #[getter]
     pub fn pre(&self) -> Option<(PreRelease, u64)> {
-        self.0.pre
+        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<u64> {
-        self.0.post
+        self.0.post()
     }
     /// The [developmental release](https://peps.python.org/pep-0440/#developmental-releases),
     /// if any
     #[getter]
     pub fn dev(&self) -> Option<u64> {
-        self.0.dev
+        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()
+        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()
+        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()
+        self.0.release().get(2).copied().unwrap_or_default()
     }
 
     /// Parses a PEP 440 version string
@@ -1310,7 +1407,9 @@ mod tests {
                 format!("Version `{version}` doesn't match PEP 440 rules")
             );
             assert_eq!(
-                VersionSpecifier::from_str(&format!("=={version}")).unwrap_err(),
+                VersionSpecifier::from_str(&format!("=={version}"))
+                    .unwrap_err()
+                    .to_string(),
                 format!("Version `{version}` doesn't match PEP 440 rules")
             );
         }

crates/pep440-rs/src/version_specifier.rs

@@ -11,11 +11,10 @@ use pyo3::{
 };
 #[cfg(feature = "serde")]
 use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
-use unicode_width::UnicodeWidthStr;
 
 #[cfg(feature = "pyo3")]
 use crate::version::PyVersion;
-use crate::{version, Operator, Pep440Error, Version};
+use crate::{version, Operator, Version};
 
 /// A thin wrapper around `Vec<VersionSpecifier>` with a serde implementation
 ///
@@ -60,7 +59,7 @@ impl FromIterator<VersionSpecifier> for VersionSpecifiers {
 }
 
 impl FromStr for VersionSpecifiers {
-    type Err = Pep440Error;
+    type Err = VersionSpecifiersParseError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         parse_version_specifiers(s).map(Self)
@@ -184,6 +183,49 @@ impl Serialize for VersionSpecifiers {
     }
 }
 
+/// Error with span information (unicode width) inside the parsed line
+#[derive(Debug, Eq, PartialEq, Clone)]
+pub struct VersionSpecifiersParseError {
+    // Clippy complains about this error type being too big (at time of
+    // writing, over 150 bytes). That does seem a little big, so we box things.
+    inner: Box<VersionSpecifiersParseErrorInner>,
+}
+
+#[derive(Debug, Eq, PartialEq, Clone)]
+struct VersionSpecifiersParseErrorInner {
+    /// The underlying error that occurred.
+    err: VersionSpecifierParseError,
+    /// The string that failed to parse
+    line: String,
+    /// The starting byte offset into the original string where the error
+    /// occurred.
+    start: usize,
+    /// The ending byte offset into the original string where the error
+    /// occurred.
+    end: usize,
+}
+
+impl std::fmt::Display for VersionSpecifiersParseError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use unicode_width::UnicodeWidthStr;
+
+        let VersionSpecifiersParseErrorInner {
+            ref err,
+            ref line,
+            start,
+            end,
+        } = *self.inner;
+        writeln!(f, "Failed to parse version: {}:", err)?;
+        writeln!(f, "{}", line)?;
+        let indent = line[..start].width();
+        let point = line[start..end].width();
+        writeln!(f, "{}{}", " ".repeat(indent), "^".repeat(point))?;
+        Ok(())
+    }
+}
+
+impl std::error::Error for VersionSpecifiersParseError {}
+
 /// A version range such such as `>1.2.3`, `<=4!5.6.7-a8.post9.dev0` or `== 4.1.*`. Parse with
 /// `VersionSpecifier::from_str`
 ///
@@ -211,7 +253,7 @@ impl VersionSpecifier {
     /// Parse a PEP 440 version
     #[new]
     pub fn parse(version_specifier: &str) -> PyResult<Self> {
-        Self::from_str(version_specifier).map_err(PyValueError::new_err)
+        Self::from_str(version_specifier).map_err(|e| PyValueError::new_err(e.to_string()))
     }
 
     /// See [VersionSpecifier::contains]
@@ -279,40 +321,22 @@ impl Serialize for VersionSpecifier {
 impl VersionSpecifier {
     /// Build from parts, validating that the operator is allowed with that version. The last
     /// parameter indicates a trailing `.*`, to differentiate between `1.1.*` and `1.1`
-    pub fn new(operator: Operator, version: Version, star: bool) -> Result<Self, String> {
+    pub fn new(
+        operator: Operator,
+        version: Version,
+        star: bool,
+    ) -> Result<Self, VersionSpecifierBuildError> {
         // "Local version identifiers are NOT permitted in this version specifier."
-        if let Some(local) = &version.local() {
+        if version.local().is_some() && !operator.is_local_compatible() {
-            if matches!(
+            return Err(BuildErrorKind::OperatorLocalCombo { operator, version }.into());
-                operator,
-                Operator::GreaterThan
-                    | Operator::GreaterThanEqual
-                    | Operator::LessThan
-                    | Operator::LessThanEqual
-                    | Operator::TildeEqual
-                    | Operator::EqualStar
-                    | Operator::NotEqualStar
-            ) {
-                return Err(format!(
-                    "You can't mix a {} operator with a local version (`+{}`)",
-                    operator,
-                    local
-                        .iter()
-                        .map(std::string::ToString::to_string)
-                        .collect::<Vec<String>>()
-                        .join(".")
-                ));
-            }
         }
 
         // Check if there are star versions and if so, switch operator to star version
         let operator = if star {
-            match operator {
+            match operator.to_star() {
-                Operator::Equal => Operator::EqualStar,
+                Some(starop) => starop,
-                Operator::NotEqual => Operator::NotEqualStar,
+                None => {
-                other => {
+                    return Err(BuildErrorKind::OperatorWithStar { operator }.into());
-                    return Err(format!(
-                        "Operator {other} must not be used in version ending with a star"
-                    ))
                 }
             }
         } else {
@@ -320,9 +344,7 @@ impl VersionSpecifier {
         };
 
         if operator == Operator::TildeEqual && version.release().len() < 2 {
-            return Err(
+            return Err(BuildErrorKind::CompatibleRelease.into());
-                "The ~= operator requires at least two parts in the release version".to_string(),
-            );
         }
 
         Ok(Self { operator, version })
@@ -350,9 +372,7 @@ impl VersionSpecifier {
     pub fn any_prerelease(&self) -> bool {
         self.version.any_prerelease()
     }
-}
 
-impl VersionSpecifier {
     /// Whether the given version satisfies the version range
     ///
     /// e.g. `>=1.19,<2.0` and `1.21` -> true
@@ -487,7 +507,7 @@ impl VersionSpecifier {
 }
 
 impl FromStr for VersionSpecifier {
-    type Err = String;
+    type Err = VersionSpecifierParseError;
 
     /// Parses a version such as `>= 1.19`, `== 1.1.*`,`~=1.0+abc.5` or `<=1!2012.2`
     fn from_str(spec: &str) -> Result<Self, Self::Err> {
@@ -496,19 +516,21 @@ impl FromStr for VersionSpecifier {
         // operator but we don't know yet if it has a star
         let operator = s.eat_while(['=', '!', '~', '<', '>']);
         if operator.is_empty() {
-            return Err("Missing comparison operator".to_string());
+            return Err(ParseErrorKind::MissingOperator.into());
         }
-        let operator = Operator::from_str(operator)?;
+        let operator = Operator::from_str(operator).map_err(ParseErrorKind::InvalidOperator)?;
         s.eat_while(|c: char| c.is_whitespace());
         let version = s.eat_while(|c: char| !c.is_whitespace());
         if version.is_empty() {
-            return Err("Missing version".to_string());
+            return Err(ParseErrorKind::MissingVersion.into());
         }
-        let (version, star) = Version::from_str_star(version)?;
+        let (version, star) =
-        let version_specifier = VersionSpecifier::new(operator, version, star)?;
+            Version::from_str_star(version).map_err(ParseErrorKind::InvalidVersion)?;
+        let version_specifier = VersionSpecifier::new(operator, version, star)
+            .map_err(ParseErrorKind::InvalidSpecifier)?;
         s.eat_while(|c: char| c.is_whitespace());
         if !s.done() {
-            return Err(format!("Trailing `{}` not allowed", s.after()));
+            return Err(ParseErrorKind::InvalidTrailing(s.after().to_string()).into());
         }
         Ok(version_specifier)
     }
@@ -523,6 +545,138 @@ impl std::fmt::Display for VersionSpecifier {
     }
 }
 
+/// An error that can occur when constructing a version specifier.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct VersionSpecifierBuildError {
+    // We box to shrink the error type's size. This in turn keeps Result<T, E>
+    // smaller and should lead to overall better codegen.
+    kind: Box<BuildErrorKind>,
+}
+
+impl std::error::Error for VersionSpecifierBuildError {}
+
+impl std::fmt::Display for VersionSpecifierBuildError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match *self.kind {
+            BuildErrorKind::OperatorLocalCombo {
+                operator: ref op,
+                ref version,
+            } => {
+                let local = version
+                    .local()
+                    .iter()
+                    .flat_map(|segments| segments.iter())
+                    .map(|segment| segment.to_string())
+                    .collect::<Vec<String>>()
+                    .join(".");
+                write!(
+                    f,
+                    "Operator {op} is incompatible with versions \
+                     containing non-empty local segments (`+{local}`)",
+                )
+            }
+            BuildErrorKind::OperatorWithStar { operator: ref op } => {
+                write!(
+                    f,
+                    "Operator {op} cannot be used with a wildcard version specifier",
+                )
+            }
+            BuildErrorKind::CompatibleRelease => {
+                write!(
+                    f,
+                    "The ~= operator requires at least two segments in the release version"
+                )
+            }
+        }
+    }
+}
+
+/// The specific kind of error that can occur when building a version specifier
+/// from an operator and version pair.
+#[derive(Clone, Debug, Eq, PartialEq)]
+enum BuildErrorKind {
+    /// Occurs when one attempts to build a version specifier with
+    /// a version containing a non-empty local segment with and an
+    /// incompatible operator.
+    OperatorLocalCombo {
+        /// The operator given.
+        operator: Operator,
+        /// The version given.
+        version: Version,
+    },
+    /// Occurs when a version specifier contains a wildcard, but is used with
+    /// an incompatible operator.
+    OperatorWithStar {
+        /// The operator given.
+        operator: Operator,
+    },
+    /// Occurs when the compatible release operator (`~=`) is used with a
+    /// version that has fewer than 2 segments in its release version.
+    CompatibleRelease,
+}
+
+impl From<BuildErrorKind> for VersionSpecifierBuildError {
+    fn from(kind: BuildErrorKind) -> VersionSpecifierBuildError {
+        VersionSpecifierBuildError {
+            kind: Box::new(kind),
+        }
+    }
+}
+
+/// An error that can occur when parsing or constructing a version specifier.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct VersionSpecifierParseError {
+    // We box to shrink the error type's size. This in turn keeps Result<T, E>
+    // smaller and should lead to overall better codegen.
+    kind: Box<ParseErrorKind>,
+}
+
+impl std::error::Error for VersionSpecifierParseError {}
+
+impl std::fmt::Display for VersionSpecifierParseError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        // Note that even though we have nested error types here, since we
+        // don't expose them through std::error::Error::source, we emit them
+        // as part of the error message here. This makes the error a bit
+        // more self-contained. And it's not clear how useful it is exposing
+        // internal errors.
+        match *self.kind {
+            ParseErrorKind::InvalidOperator(ref err) => err.fmt(f),
+            ParseErrorKind::InvalidVersion(ref err) => err.fmt(f),
+            ParseErrorKind::InvalidSpecifier(ref err) => err.fmt(f),
+            ParseErrorKind::MissingOperator => {
+                write!(f, "Unexpected end of version specifier, expected operator")
+            }
+            ParseErrorKind::MissingVersion => {
+                write!(f, "Unexpected end of version specifier, expected version")
+            }
+            ParseErrorKind::InvalidTrailing(ref trail) => {
+                write!(f, "Trailing `{trail}` is not allowed")
+            }
+        }
+    }
+}
+
+/// The specific kind of error that occurs when parsing a single version
+/// specifier from a string.
+#[derive(Clone, Debug, Eq, PartialEq)]
+enum ParseErrorKind {
+    InvalidOperator(String),
+    InvalidVersion(String),
+    InvalidSpecifier(VersionSpecifierBuildError),
+    MissingOperator,
+    MissingVersion,
+    InvalidTrailing(String),
+}
+
+impl From<ParseErrorKind> for VersionSpecifierParseError {
+    fn from(kind: ParseErrorKind) -> VersionSpecifierParseError {
+        VersionSpecifierParseError {
+            kind: Box::new(kind),
+        }
+    }
+}
+
 /// Parses a list of specifiers such as `>= 1.0, != 1.3.*, < 2.0`.
 ///
 /// I recommend using [`VersionSpecifiers::from_str`] instead.
@@ -535,7 +689,9 @@ impl std::fmt::Display for VersionSpecifier {
 /// let version_specifiers = parse_version_specifiers(">=1.16, <2.0").unwrap();
 /// assert!(version_specifiers.iter().all(|specifier| specifier.contains(&version)));
 /// ```
-pub fn parse_version_specifiers(spec: &str) -> Result<Vec<VersionSpecifier>, Pep440Error> {
+pub fn parse_version_specifiers(
+    spec: &str,
+) -> Result<Vec<VersionSpecifier>, VersionSpecifiersParseError> {
     let mut version_ranges = Vec::new();
     if spec.is_empty() {
         return Ok(version_ranges);
@@ -545,19 +701,21 @@ pub fn parse_version_specifiers(spec: &str) -> Result<Vec<VersionSpecifier>, Pep
     for version_range_spec in spec.split(separator) {
         match VersionSpecifier::from_str(version_range_spec) {
             Err(err) => {
-                return Err(Pep440Error {
+                return Err(VersionSpecifiersParseError {
-                    message: err,
+                    inner: Box::new(VersionSpecifiersParseErrorInner {
-                    line: spec.to_string(),
+                        err,
-                    start,
+                        line: spec.to_string(),
-                    width: version_range_spec.width(),
+                        start,
+                        end: start + version_range_spec.len(),
+                    }),
                 });
             }
             Ok(version_range) => {
                 version_ranges.push(version_range);
             }
         }
-        start += version_range_spec.width();
+        start += version_range_spec.len();
-        start += separator.width();
+        start += separator.len();
     }
     Ok(version_ranges)
 }
@@ -568,7 +726,9 @@ mod tests {
 
     use indoc::indoc;
 
-    use crate::{Operator, Version, VersionSpecifier, VersionSpecifiers};
+    use crate::LocalSegment;
+
+    use super::*;
 
     /// <https://peps.python.org/pep-0440/#version-matching>
     #[test]
@@ -1083,7 +1243,7 @@ mod tests {
         assert_eq!(
             result.unwrap_err().to_string(),
             indoc! {r#"
-                Failed to parse version:
+                Failed to parse version: Unexpected end of version specifier, expected operator:
                 ~= 0.9, %‍= 1.0, != 1.3.4.*
                        ^^^^^^^
             "#}
@@ -1094,8 +1254,11 @@ mod tests {
     fn test_non_star_after_star() {
         let result = VersionSpecifiers::from_str("== 0.9.*.1");
         assert_eq!(
-            result.unwrap_err().message,
+            result.unwrap_err().inner.err,
-            "Version `0.9.*.1` doesn't match PEP 440 rules"
+            ParseErrorKind::InvalidVersion(
+                "Version `0.9.*.1` doesn't match PEP 440 rules".to_string()
+            )
+            .into()
         );
     }
 
@@ -1103,15 +1266,24 @@ mod tests {
     fn test_star_wrong_operator() {
         let result = VersionSpecifiers::from_str(">= 0.9.1.*");
         assert_eq!(
-            result.unwrap_err().message,
+            result.unwrap_err().inner.err,
-            "Operator >= must not be used in version ending with a star"
+            ParseErrorKind::InvalidSpecifier(
+                BuildErrorKind::OperatorWithStar {
+                    operator: Operator::GreaterThanEqual,
+                }
+                .into()
+            )
+            .into(),
         );
     }
 
     #[test]
     fn test_regex_mismatch() {
         let result = VersionSpecifiers::from_str("blergh");
-        assert_eq!(result.unwrap_err().message, "Missing comparison operator");
+        assert_eq!(
+            result.unwrap_err().inner.err,
+            ParseErrorKind::MissingOperator.into(),
+        );
     }
 
     /// <https://github.com/pypa/packaging/blob/e184feef1a28a5c574ec41f5c263a3a573861f5a/tests/test_specifiers.py#L44-L84>
@@ -1119,126 +1291,234 @@ mod tests {
     fn test_invalid_specifier() {
         let specifiers = [
             // Operator-less specifier
-            ("2.0", Some("Missing comparison operator")),
+            ("2.0", ParseErrorKind::MissingOperator.into()),
             // Invalid operator
             (
                 "=>2.0",
-                Some("No such comparison operator '=>', must be one of ~= == != <= >= < > ==="),
+                ParseErrorKind::InvalidOperator(
+                    "No such comparison operator '=>', must be one of ~= == != <= >= < > ==="
+                        .to_string(),
+                )
+                .into(),
             ),
             // Version-less specifier
-            ("==", Some("Missing version")),
+            ("==", ParseErrorKind::MissingVersion.into()),
             // Local segment on operators which don't support them
             (
                 "~=1.0+5",
-                Some("You can't mix a ~= operator with a local version (`+5`)"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorLocalCombo {
+                        operator: Operator::TildeEqual,
+                        version: Version::new([1, 0]).with_local(vec![LocalSegment::Number(5)]),
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 ">=1.0+deadbeef",
-                Some("You can't mix a >= operator with a local version (`+deadbeef`)"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorLocalCombo {
+                        operator: Operator::GreaterThanEqual,
+                        version: Version::new([1, 0])
+                            .with_local(vec![LocalSegment::String("deadbeef".to_string())]),
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 "<=1.0+abc123",
-                Some("You can't mix a <= operator with a local version (`+abc123`)"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorLocalCombo {
+                        operator: Operator::LessThanEqual,
+                        version: Version::new([1, 0])
+                            .with_local(vec![LocalSegment::String("abc123".to_string())]),
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 ">1.0+watwat",
-                Some("You can't mix a > operator with a local version (`+watwat`)"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorLocalCombo {
+                        operator: Operator::GreaterThan,
+                        version: Version::new([1, 0])
+                            .with_local(vec![LocalSegment::String("watwat".to_string())]),
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 "<1.0+1.0",
-                Some("You can't mix a < operator with a local version (`+1.0`)"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorLocalCombo {
+                        operator: Operator::LessThan,
+                        version: Version::new([1, 0])
+                            .with_local(vec![LocalSegment::Number(1), LocalSegment::Number(0)]),
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             // Prefix matching on operators which don't support them
             (
                 "~=1.0.*",
-                Some("Operator ~= must not be used in version ending with a star"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorWithStar {
+                        operator: Operator::TildeEqual,
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 ">=1.0.*",
-                Some("Operator >= must not be used in version ending with a star"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorWithStar {
+                        operator: Operator::GreaterThanEqual,
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 "<=1.0.*",
-                Some("Operator <= must not be used in version ending with a star"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorWithStar {
+                        operator: Operator::LessThanEqual,
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 ">1.0.*",
-                Some("Operator > must not be used in version ending with a star"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorWithStar {
+                        operator: Operator::GreaterThan,
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             (
                 "<1.0.*",
-                Some("Operator < must not be used in version ending with a star"),
+                ParseErrorKind::InvalidSpecifier(
+                    BuildErrorKind::OperatorWithStar {
+                        operator: Operator::LessThan,
+                    }
+                    .into(),
+                )
+                .into(),
             ),
             // Combination of local and prefix matching on operators which do
             // support one or the other
             (
                 "==1.0.*+5",
-                Some("Version `1.0.*+5` doesn't match PEP 440 rules"),
+                ParseErrorKind::InvalidVersion(
+                    "Version `1.0.*+5` doesn't match PEP 440 rules".to_string(),
+                )
+                .into(),
             ),
             (
                 "!=1.0.*+deadbeef",
-                Some("Version `1.0.*+deadbeef` doesn't match PEP 440 rules"),
+                ParseErrorKind::InvalidVersion(
+                    "Version `1.0.*+deadbeef` doesn't match PEP 440 rules".to_string(),
+                )
+                .into(),
             ),
             // Prefix matching cannot be used with a pre-release, post-release,
             // dev or local version
             (
                 "==2.0a1.*",
-                Some("You can't have both a trailing `.*` and a prerelease version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a prerelease version".to_string(),
+                )
+                .into(),
             ),
             (
                 "!=2.0a1.*",
-                Some("You can't have both a trailing `.*` and a prerelease version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a prerelease version".to_string(),
+                )
+                .into(),
             ),
             (
                 "==2.0.post1.*",
-                Some("You can't have both a trailing `.*` and a post version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a post version".to_string(),
+                )
+                .into(),
             ),
             (
                 "!=2.0.post1.*",
-                Some("You can't have both a trailing `.*` and a post version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a post version".to_string(),
+                )
+                .into(),
             ),
             (
                 "==2.0.dev1.*",
-                Some("You can't have both a trailing `.*` and a dev version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a dev version".to_string(),
+                )
+                .into(),
             ),
             (
                 "!=2.0.dev1.*",
-                Some("You can't have both a trailing `.*` and a dev version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a dev version".to_string(),
+                )
+                .into(),
             ),
             (
                 "==1.0+5.*",
-                Some("You can't have both a trailing `.*` and a local version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a local version".to_string(),
+                )
+                .into(),
             ),
             (
                 "!=1.0+deadbeef.*",
-                Some("You can't have both a trailing `.*` and a local version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a local version".to_string(),
+                )
+                .into(),
             ),
             // Prefix matching must appear at the end
             (
                 "==1.0.*.5",
-                Some("Version `1.0.*.5` doesn't match PEP 440 rules"),
+                ParseErrorKind::InvalidVersion(
+                    "Version `1.0.*.5` doesn't match PEP 440 rules".to_string(),
+                )
+                .into(),
             ),
             // Compatible operator requires 2 digits in the release operator
             (
                 "~=1",
-                Some("The ~= operator requires at least two parts in the release version"),
+                ParseErrorKind::InvalidSpecifier(BuildErrorKind::CompatibleRelease.into()).into(),
             ),
             // Cannot use a prefix matching after a .devN version
             (
                 "==1.0.dev1.*",
-                Some("You can't have both a trailing `.*` and a dev version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a dev version".to_string(),
+                )
+                .into(),
             ),
             (
                 "!=1.0.dev1.*",
-                Some("You can't have both a trailing `.*` and a dev version"),
+                ParseErrorKind::InvalidVersion(
+                    "You can't have both a trailing `.*` and a dev version".to_string(),
+                )
+                .into(),
             ),
         ];
         for (specifier, error) in specifiers {
-            if let Some(error) = error {
+            assert_eq!(VersionSpecifier::from_str(specifier).unwrap_err(), error);
-                assert_eq!(VersionSpecifier::from_str(specifier).unwrap_err(), error);
-            } else {
-                unreachable!("expected an error, but got valid version specifier")
-            }
         }
     }
 
@@ -1278,4 +1558,95 @@ mod tests {
     fn test_version_specifiers_empty() {
         assert_eq!(VersionSpecifiers::from_str("").unwrap().to_string(), "");
     }
+
+    /// All non-ASCII version specifiers are invalid, but the user can still
+    /// attempt to parse a non-ASCII string as a version specifier. This
+    /// ensures no panics occur and that the error reported has correct info.
+    #[test]
+    fn non_ascii_version_specifier() {
+        let s = "💩";
+        let err = s.parse::<VersionSpecifiers>().unwrap_err();
+        assert_eq!(err.inner.start, 0);
+        assert_eq!(err.inner.end, 4);
+
+        // The first test here is plain ASCII and it gives the
+        // expected result: the error starts at codepoint 12,
+        // which is the start of `>5.%`.
+        let s = ">=3.7, <4.0,>5.%";
+        let err = s.parse::<VersionSpecifiers>().unwrap_err();
+        assert_eq!(err.inner.start, 12);
+        assert_eq!(err.inner.end, 16);
+        // In this case, we replace a single ASCII codepoint
+        // with U+3000 IDEOGRAPHIC SPACE. Its *visual* width is
+        // 2 despite it being a single codepoint. This causes
+        // the offsets in the error reporting logic to become
+        // incorrect.
+        //
+        // ... it did. This bug was fixed by switching to byte
+        // offsets.
+        let s = ">=3.7,\u{3000}<4.0,>5.%";
+        let err = s.parse::<VersionSpecifiers>().unwrap_err();
+        assert_eq!(err.inner.start, 14);
+        assert_eq!(err.inner.end, 18);
+    }
+
+    /// Tests the human readable error messages generated from an invalid
+    /// sequence of version specifiers.
+    #[test]
+    fn error_message_version_specifiers_parse_error() {
+        let specs = ">=1.2.3, 5.4.3, >=3.4.5";
+        let err = VersionSpecifierParseError {
+            kind: Box::new(ParseErrorKind::MissingOperator),
+        };
+        let inner = Box::new(VersionSpecifiersParseErrorInner {
+            err,
+            line: specs.to_string(),
+            start: 8,
+            end: 14,
+        });
+        let err = VersionSpecifiersParseError { inner };
+        assert_eq!(err, VersionSpecifiers::from_str(specs).unwrap_err());
+        assert_eq!(
+            err.to_string(),
+            "\
+Failed to parse version: Unexpected end of version specifier, expected operator:
+>=1.2.3, 5.4.3, >=3.4.5
+        ^^^^^^
+"
+        );
+    }
+
+    /// Tests the human readable error messages generated when building an
+    /// invalid version specifier.
+    #[test]
+    fn error_message_version_specifier_build_error() {
+        let err = VersionSpecifierBuildError {
+            kind: Box::new(BuildErrorKind::CompatibleRelease),
+        };
+        let op = Operator::TildeEqual;
+        let v = Version::new([5]);
+        assert_eq!(err, VersionSpecifier::new(op, v, false).unwrap_err());
+        assert_eq!(
+            err.to_string(),
+            "The ~= operator requires at least two segments in the release version"
+        );
+    }
+
+    /// Tests the human readable error messages generated from parsing invalid
+    /// version specifier.
+    #[test]
+    fn error_message_version_specifier_parse_error() {
+        let err = VersionSpecifierParseError {
+            kind: Box::new(ParseErrorKind::InvalidSpecifier(
+                VersionSpecifierBuildError {
+                    kind: Box::new(BuildErrorKind::CompatibleRelease),
+                },
+            )),
+        };
+        assert_eq!(err, VersionSpecifier::from_str("~=5").unwrap_err());
+        assert_eq!(
+            err.to_string(),
+            "The ~= operator requires at least two segments in the release version"
+        );
+    }
 }

crates/pep508-rs/src/lib.rs

@@ -702,7 +702,7 @@ fn parse_specifier(
     end: usize,
 ) -> Result<VersionSpecifier, Pep508Error> {
     VersionSpecifier::from_str(buffer).map_err(|err| Pep508Error {
-        message: Pep508ErrorSource::String(err),
+        message: Pep508ErrorSource::String(err.to_string()),
         start,
         len: end - start,
         input: cursor.to_string(),
@@ -1262,7 +1262,7 @@ mod tests {
         assert_err(
             r#"numpy >=1.1.*"#,
             indoc! {"
-                Operator >= must not be used in version ending with a star
+                Operator >= cannot be used with a wildcard version specifier
                 numpy >=1.1.*
                       ^^^^^^^"
             },
@@ -1442,7 +1442,7 @@ mod tests {
         assert_err(
             "name==",
             indoc! {"
-                Missing version
+                Unexpected end of version specifier, expected version
                 name==
                     ^^"
             },
@@ -1466,7 +1466,7 @@ mod tests {
         assert_err(
             "name >= 1.0 #",
             indoc! {"
-                Trailing `#` not allowed
+                Trailing `#` is not allowed
                 name >= 1.0 #
                      ^^^^^^^^"
             },

crates/pep508-rs/src/marker.rs

@@ -783,7 +783,7 @@ impl MarkerExpression {
                     let compatible = python_versions.iter().any(|r_version| {
                         // operator and right hand side make the specifier and in this case the
                         // right hand is `python_version` so changes every iteration
-                        match VersionSpecifier::new(operator.clone(), r_version.clone(), false) {
+                        match VersionSpecifier::new(operator, r_version.clone(), false) {
                             Ok(specifier) => specifier.contains(&l_version),
                             Err(_) => true,
                         }

crates/puffin-client/src/html.rs

@@ -191,7 +191,7 @@ pub enum Error {
     UnsupportedHashAlgorithm(String),
 
     #[error("Invalid `requires-python` specifier: {0}")]
-    Pep440(#[source] pep440_rs::Pep440Error),
+    Pep440(#[source] pep440_rs::VersionSpecifiersParseError),
 }
 
 #[cfg(test)]

crates/pypi-types/src/lenient_requirement.rs

@@ -4,7 +4,7 @@ use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::{de, Deserialize, Deserializer, Serialize};
 
-use pep440_rs::{Pep440Error, VersionSpecifiers};
+use pep440_rs::{VersionSpecifiers, VersionSpecifiersParseError};
 use pep508_rs::{Pep508Error, Requirement};
 use puffin_warnings::warn_user_once;
 
@@ -99,7 +99,7 @@ impl From<LenientRequirement> for Requirement {
 pub struct LenientVersionSpecifiers(VersionSpecifiers);
 
 impl FromStr for LenientVersionSpecifiers {
-    type Err = Pep440Error;
+    type Err = VersionSpecifiersParseError;
 
     fn from_str(input: &str) -> Result<Self, Self::Err> {
         Ok(Self(parse_with_fixups(input, "version specifier")?))

crates/pypi-types/src/metadata.rs

@@ -7,7 +7,7 @@ use mailparse::{MailHeaderMap, MailParseError};
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
-use pep440_rs::{Pep440Error, Version, VersionSpecifiers};
+use pep440_rs::{Version, VersionSpecifiers, VersionSpecifiersParseError};
 use pep508_rs::{Pep508Error, Requirement};
 use puffin_normalize::{ExtraName, InvalidNameError, PackageName};
 
@@ -63,7 +63,7 @@ pub enum Error {
     Pep440VersionError(String),
     /// Invalid VersionSpecifier
     #[error(transparent)]
-    Pep440Error(#[from] Pep440Error),
+    Pep440Error(#[from] VersionSpecifiersParseError),
     /// Invalid Requirement
     #[error(transparent)]
     Pep508Error(#[from] Pep508Error),