Fix handling of ~= inverted with python_version and python_full_version

This commit is contained in:
konstin 2025-06-30 11:29:15 +02:00
parent bb738aeb44
commit e62365afff
5 changed files with 176 additions and 33 deletions

View file

@ -292,7 +292,7 @@ impl Version {
///
/// When the iterator yields no elements.
#[inline]
pub fn new<I, R>(release_numbers: I) -> Self
pub fn new<I, R>(release_segments: I) -> Self
where
I: IntoIterator<Item = R>,
R: Borrow<u64>,
@ -302,7 +302,7 @@ impl Version {
small: VersionSmall::new(),
},
}
.with_release(release_numbers)
.with_release(release_segments)
}
/// Whether this is an alpha/beta/rc or dev version

View file

@ -47,6 +47,7 @@
use std::cmp::Ordering;
use std::fmt;
use std::hash::Hash;
use std::ops::Bound;
use std::sync::{LazyLock, Mutex, MutexGuard};
@ -181,6 +182,55 @@ impl InternerGuard<'_> {
}
}
},
// `<version key> ~= python_version` and `<version key> ~= python_full_version`
MarkerExpression::VersionInvertedTilde { key, specifier } => match key {
MarkerValueVersion::ImplementationVersion => {
// The number of segments in the implementation version is unknown and may
// only be a single one not support with tilde equal
return NodeId::FALSE;
}
MarkerValueVersion::PythonFullVersion => {
// Given `3.10 ~= python_full_version`,
// which matches for 3.10.0, 3.10.1, ... it becomes
// `python_full_version < 3.11 and python_version >= 3.10`
let major = specifier.version().release().first().copied().unwrap_or(0);
let minor = specifier.version().release().get(1).copied().unwrap_or(0);
let upper_bound =
VersionSpecifier::less_than_version(Version::new([major, minor + 1, 0]));
let lower_bound = if minor == 0 {
VersionSpecifier::greater_than_equal_version(Version::new([major]))
} else {
VersionSpecifier::greater_than_equal_version(Version::new([major, minor]))
};
let ranges = release_specifier_to_range(upper_bound, true)
.intersection(&release_specifier_to_range(lower_bound, true));
(
Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion),
Edges::from_version_ranges(&ranges),
)
}
// Normalize `python_version` markers to `python_full_version` nodes.
MarkerValueVersion::PythonVersion => {
// Given `3.10 ~= python_version`,
// which matches for 3.0, 3.1, ..., 3.9, 3.10, it becomes
// `python_full_version < 3.11 and python_version >= 3`
let major = specifier.version().release().first().copied().unwrap_or(0);
let minor = specifier.version().release().get(1).copied().unwrap_or(0);
let upper_bound =
VersionSpecifier::less_than_version(Version::new([major, minor + 1]));
let lower_bound =
VersionSpecifier::greater_than_equal_version(Version::new([major]));
let ranges = release_specifier_to_range(upper_bound, true)
.intersection(&release_specifier_to_range(lower_bound, true));
(
Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion),
Edges::from_version_ranges(&ranges),
)
}
},
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::VersionIn {
@ -701,7 +751,7 @@ impl InternerGuard<'_> {
if matches!(i, NodeId::TRUE) {
let var = Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion);
let edges = Edges::Version {
edges: Edges::from_range(&py_range),
edges: Edges::from_ranges(&py_range),
};
return self.create_node(var, edges).negate(i);
}
@ -1208,15 +1258,21 @@ impl Edges {
};
Edges::String {
edges: Edges::from_range(&range),
edges: Edges::from_ranges(&range),
}
}
/// Returns the [`Edges`] for a version specifier.
/// Returns the [`Edges`] for a version r.
fn from_specifier(specifier: VersionSpecifier) -> Edges {
let specifier = release_specifier_to_range(specifier.only_release(), true);
Edges::Version {
edges: Edges::from_range(&specifier),
edges: Edges::from_ranges(&specifier),
}
}
fn from_version_ranges(ranges: &Ranges<Version>) -> Edges {
Edges::Version {
edges: Edges::from_ranges(ranges),
}
}
@ -1239,7 +1295,7 @@ impl Edges {
}
Ok(Edges::Version {
edges: Edges::from_range(&range),
edges: Edges::from_ranges(&range),
})
}
@ -1260,25 +1316,25 @@ impl Edges {
}
Edges::Version {
edges: Edges::from_range(&range),
edges: Edges::from_ranges(&range),
}
}
/// Returns an [`Edges`] where values in the given range are `true`.
fn from_range<T>(range: &Ranges<T>) -> SmallVec<(Ranges<T>, NodeId)>
fn from_ranges<T>(ranges: &Ranges<T>) -> SmallVec<(Ranges<T>, NodeId)>
where
T: Ord + Clone,
{
let mut edges = SmallVec::new();
// Add the `true` edges.
for (start, end) in range.iter() {
for (start, end) in ranges.iter() {
let range = Ranges::from_range_bounds((start.clone(), end.clone()));
edges.push((range, NodeId::TRUE));
}
// Add the `false` edges.
for (start, end) in range.complement().iter() {
for (start, end) in ranges.complement().iter() {
let range = Ranges::from_range_bounds((start.clone(), end.clone()));
edges.push((range, NodeId::FALSE));
}
@ -1595,10 +1651,10 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result<Version
// result in a `python_version` marker of `3.7`. For this reason, we must consider the range
// of values that would satisfy a `python_version` specifier when truncated in order to transform
// the specifier into its `python_full_version` equivalent.
if let Some((major, minor)) = major_minor {
let specifier = if let Some((major, minor)) = major_minor {
let version = Version::new([major, minor]);
Ok(match specifier.operator() {
match specifier.operator() {
// `python_version == 3.7` is equivalent to `python_full_version == 3.7.*`.
Operator::Equal | Operator::ExactEqual => {
VersionSpecifier::equals_star_version(version)
@ -1623,13 +1679,13 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result<Version
// Handled above.
unreachable!()
}
})
}
} else {
let [major, minor, ..] = *specifier.version().release() else {
unreachable!()
};
Ok(match specifier.operator() {
match specifier.operator() {
// `python_version` cannot have more than two release segments, and we know
// that the following release segments aren't purely zeroes so equality is impossible.
Operator::Equal | Operator::ExactEqual => {
@ -1653,8 +1709,10 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result<Version
// Handled above.
unreachable!()
}
})
}
}
};
Ok(specifier)
}
/// Compares the start of two ranges that are known to be disjoint.

View file

@ -1,7 +1,7 @@
use arcstr::ArcStr;
use std::str::FromStr;
use uv_normalize::ExtraName;
use uv_pep440::{Version, VersionPattern, VersionSpecifier};
use uv_pep440::{Operator, Version, VersionPattern, VersionSpecifier};
use crate::cursor::Cursor;
use crate::marker::MarkerValueExtra;
@ -265,12 +265,26 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
parse_inverted_version_expr(&l_string, operator, key, reporter)
}
// '...' == <env key>
MarkerValue::MarkerEnvString(key) => Some(MarkerExpression::String {
key,
MarkerValue::MarkerEnvString(key) => {
// Invert the operator to normalize the expression order.
operator: operator.invert(),
value: l_string,
}),
if let Some(operator) = operator.invert() {
Some(MarkerExpression::String {
key,
operator,
value: l_string,
})
} else {
debug_assert_eq!(operator, MarkerOperator::TildeEqual);
reporter.report(
MarkerWarningKind::StringStringComparison,
format!(
"Comparing a string with `~=` doesn't make sense:
'{l_string}' {operator} {r_value}, will be ignored"
),
);
None
}
}
// `'...' == extra`
MarkerValue::Extra => parse_extra_expr(operator, &l_string, reporter),
// `'...' == '...'`, doesn't make much sense
@ -422,9 +436,6 @@ fn parse_inverted_version_expr(
key: MarkerValueVersion,
reporter: &mut impl Reporter,
) -> Option<MarkerExpression> {
// Invert the operator to normalize the expression order.
let marker_operator = marker_operator.invert();
// Not star allowed here, `'3.*' == python_version` is not a valid PEP 440 comparison.
let version = match value.parse::<Version>() {
Ok(version) => version,
@ -441,6 +452,25 @@ fn parse_inverted_version_expr(
}
};
// Invert the operator to normalize the expression order.
let Some(marker_operator) = marker_operator.invert() else {
// The only operator that can't be inverted is `~=`.
debug_assert_eq!(marker_operator, MarkerOperator::TildeEqual);
return Some(MarkerExpression::VersionInvertedTilde {
key,
specifier: match VersionSpecifier::from_version(Operator::TildeEqual, version) {
Ok(specifier) => specifier,
Err(err) => {
reporter.report(
MarkerWarningKind::Pep440Error,
format!("Invalid operator/version combination: {err}"),
);
return None;
}
},
});
};
let Some(operator) = marker_operator.to_pep440_operator() else {
reporter.report(
MarkerWarningKind::Pep440Error,

View file

@ -393,6 +393,10 @@ fn is_negation(left: &MarkerExpression, right: &MarkerExpression) -> bool {
.negate()
.is_some_and(|negated| negated == *specifier2.operator())
}
MarkerExpression::VersionInvertedTilde { .. } => {
// The inversion is not a single expression.
false
}
MarkerExpression::VersionIn {
key,
versions,

View file

@ -236,20 +236,22 @@ impl MarkerOperator {
}
/// Inverts this marker operator.
pub(crate) fn invert(self) -> MarkerOperator {
match self {
///
/// `~=` has no inverse.
pub(crate) fn invert(self) -> Option<MarkerOperator> {
Some(match self {
Self::LessThan => Self::GreaterThan,
Self::LessEqual => Self::GreaterEqual,
Self::GreaterThan => Self::LessThan,
Self::GreaterEqual => Self::LessEqual,
Self::Equal => Self::Equal,
Self::NotEqual => Self::NotEqual,
Self::TildeEqual => Self::TildeEqual,
Self::TildeEqual => return None,
Self::In => Self::Contains,
Self::NotIn => Self::NotContains,
Self::Contains => Self::In,
Self::NotContains => Self::NotIn,
}
})
}
/// Negates this marker operator.
@ -481,6 +483,12 @@ pub enum MarkerExpression {
key: MarkerValueVersion,
specifier: VersionSpecifier,
},
/// Special case for `<version key> ~= python_version` and
/// `<version key> ~= python_full_version`.
VersionInvertedTilde {
key: MarkerValueVersion,
specifier: VersionSpecifier,
},
/// A version in list expression, e.g. `<version key> in <quoted list of PEP 440 versions>`.
///
/// A special case of [`MarkerExpression::String`] with the [`MarkerOperator::In`] operator for
@ -494,7 +502,7 @@ pub enum MarkerExpression {
versions: Vec<Version>,
negated: bool,
},
/// An string marker comparison, e.g. `sys_platform == '...'`.
/// A string marker comparison, e.g. `sys_platform == '...'`.
///
/// Inverted string expressions, e.g `'...' == sys_platform`, are also normalized to this form.
String {
@ -514,6 +522,8 @@ pub enum MarkerExpression {
pub(crate) enum MarkerExpressionKind {
/// A version expression, e.g. `<version key> <version op> <quoted PEP 440 version>`.
Version(MarkerValueVersion),
/// An inverted tilde-equal version expression, e.g. `<version key> ~= <quoted PEP 440 version>`.
VersionInvertedTilde(MarkerValueVersion),
/// A version "in" expression, e.g. `<version key> in <quoted list of PEP 440 versions>`.
VersionIn(MarkerValueVersion),
/// A string marker comparison, e.g. `sys_platform == '...'`.
@ -597,6 +607,9 @@ impl MarkerExpression {
pub(crate) fn kind(&self) -> MarkerExpressionKind {
match self {
MarkerExpression::Version { key, .. } => MarkerExpressionKind::Version(*key),
MarkerExpression::VersionInvertedTilde { key, .. } => {
MarkerExpressionKind::VersionInvertedTilde(*key)
}
MarkerExpression::VersionIn { key, .. } => MarkerExpressionKind::VersionIn(*key),
MarkerExpression::String { key, .. } => MarkerExpressionKind::String(*key),
MarkerExpression::Extra { .. } => MarkerExpressionKind::Extra,
@ -615,6 +628,10 @@ impl Display for MarkerExpression {
}
write!(f, "{key} {op} '{version}'")
}
MarkerExpression::VersionInvertedTilde { key, specifier } => {
let (op, version) = (specifier.operator(), specifier.version());
write!(f, "'{version}' {op} {key}")
}
MarkerExpression::VersionIn {
key,
versions,
@ -633,7 +650,8 @@ impl Display for MarkerExpression {
operator,
MarkerOperator::Contains | MarkerOperator::NotContains
) {
return write!(f, "'{value}' {} {key}", operator.invert());
// Unwrap safety: `in` and `not in` have inverses.
return write!(f, "'{value}' {} {key}", operator.invert().unwrap());
}
write!(f, "{key} {operator} '{value}'")
@ -2599,6 +2617,39 @@ mod test {
);
}
#[test]
fn test_tilde_equal_inverted_normalization() {
assert_eq!(
m("'3.0' ~= python_version"),
m("python_version >= '3' and python_version <= '3.0'")
);
assert_eq!(
m("'3.10' ~= python_version"),
m("python_version >= '3' and python_version <= '3.10'")
);
assert_eq!(
m("'3.10.0' ~= python_version"),
m("python_version >= '3' and python_version <= '3.10'")
);
assert_eq!(
m("'3.10' ~= python_full_version"),
m("python_full_version < '3.11' and python_full_version >= '3.10.0'")
);
assert_eq!(
m("'3.10.10' ~= python_full_version"),
m("python_full_version < '3.11' and python_full_version >= '3.10.0'")
);
assert_simplifies(
"python_version == '3.0.*'",
"python_full_version == '3.0.*'",
);
}
/// This tests marker implication.
///
/// Specifically, these test cases come from a [bug] where `foo` and `bar`
@ -2682,7 +2733,7 @@ mod test {
);
assert_eq!(
m("'3.6' ~= python_version").negate(),
m("python_version < '3.6' or python_version != '3.*'")
m("python_full_version < '3' or python_full_version >= '3.7'")
);
assert_eq!(
m("python_version ~= '3.6.2'").negate(),