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 02e103f867
commit 221a0797a1
5 changed files with 175 additions and 32 deletions

View File

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

View File

@ -47,6 +47,7 @@
use std::cmp::Ordering; use std::cmp::Ordering;
use std::fmt; use std::fmt;
use std::hash::Hash;
use std::ops::Bound; use std::ops::Bound;
use std::sync::{LazyLock, Mutex, MutexGuard}; use std::sync::{LazyLock, Mutex, MutexGuard};
@ -183,6 +184,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 // A variable representing the output of a version key. Edges correspond
// to disjoint version ranges. // to disjoint version ranges.
MarkerExpression::VersionIn { MarkerExpression::VersionIn {
@ -707,7 +757,7 @@ impl InternerGuard<'_> {
if matches!(i, NodeId::TRUE) { if matches!(i, NodeId::TRUE) {
let var = Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion); let var = Variable::Version(CanonicalMarkerValueVersion::PythonFullVersion);
let edges = Edges::Version { let edges = Edges::Version {
edges: Edges::from_range(&py_range), edges: Edges::from_ranges(&py_range),
}; };
return self.create_node(var, edges).negate(i); return self.create_node(var, edges).negate(i);
} }
@ -1220,7 +1270,7 @@ impl Edges {
}; };
Edges::String { Edges::String {
edges: Edges::from_range(&range), edges: Edges::from_ranges(&range),
} }
} }
@ -1228,7 +1278,13 @@ impl Edges {
fn from_specifier(specifier: VersionSpecifier) -> Edges { fn from_specifier(specifier: VersionSpecifier) -> Edges {
let specifier = release_specifier_to_range(specifier.only_release(), true); let specifier = release_specifier_to_range(specifier.only_release(), true);
Edges::Version { 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),
} }
} }
@ -1254,7 +1310,7 @@ impl Edges {
} }
Ok(Edges::Version { Ok(Edges::Version {
edges: Edges::from_range(&range), edges: Edges::from_ranges(&range),
}) })
} }
@ -1275,25 +1331,25 @@ impl Edges {
} }
Edges::Version { Edges::Version {
edges: Edges::from_range(&range), edges: Edges::from_ranges(&range),
} }
} }
/// Returns an [`Edges`] where values in the given range are `true`. /// 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 where
T: Ord + Clone, T: Ord + Clone,
{ {
let mut edges = SmallVec::new(); let mut edges = SmallVec::new();
// Add the `true` edges. // 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())); let range = Ranges::from_range_bounds((start.clone(), end.clone()));
edges.push((range, NodeId::TRUE)); edges.push((range, NodeId::TRUE));
} }
// Add the `false` edges. // 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())); let range = Ranges::from_range_bounds((start.clone(), end.clone()));
edges.push((range, NodeId::FALSE)); edges.push((range, NodeId::FALSE));
} }
@ -1610,10 +1666,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 // 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 // of values that would satisfy a `python_version` specifier when truncated in order to transform
// the specifier into its `python_full_version` equivalent. // 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]); 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.*`. // `python_version == 3.7` is equivalent to `python_full_version == 3.7.*`.
Operator::Equal | Operator::ExactEqual => { Operator::Equal | Operator::ExactEqual => {
VersionSpecifier::equals_star_version(version) VersionSpecifier::equals_star_version(version)
@ -1638,13 +1694,13 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result<Version
// Handled above. // Handled above.
unreachable!() unreachable!()
} }
}) }
} else { } else {
let [major, minor, ..] = *specifier.version().release() else { let [major, minor, ..] = *specifier.version().release() else {
unreachable!() unreachable!()
}; };
Ok(match specifier.operator() { match specifier.operator() {
// `python_version` cannot have more than two release segments, and we know // `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. // that the following release segments aren't purely zeroes so equality is impossible.
Operator::Equal | Operator::ExactEqual => { Operator::Equal | Operator::ExactEqual => {
@ -1668,8 +1724,10 @@ fn python_version_to_full_version(specifier: VersionSpecifier) -> Result<Version
// Handled above. // Handled above.
unreachable!() unreachable!()
} }
}) }
} };
Ok(specifier)
} }
/// Compares the start of two ranges that are known to be disjoint. /// Compares the start of two ranges that are known to be disjoint.

View File

@ -1,7 +1,7 @@
use arcstr::ArcStr; use arcstr::ArcStr;
use std::str::FromStr; use std::str::FromStr;
use uv_normalize::{ExtraName, GroupName}; use uv_normalize::{ExtraName, GroupName};
use uv_pep440::{Version, VersionPattern, VersionSpecifier}; use uv_pep440::{Operator, Version, VersionPattern, VersionSpecifier};
use crate::cursor::Cursor; use crate::cursor::Cursor;
use crate::marker::MarkerValueExtra; use crate::marker::MarkerValueExtra;
@ -282,12 +282,26 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
parse_inverted_version_expr(&l_string, operator, key, reporter) parse_inverted_version_expr(&l_string, operator, key, reporter)
} }
// '...' == <env key> // '...' == <env key>
MarkerValue::MarkerEnvString(key) => Some(MarkerExpression::String { MarkerValue::MarkerEnvString(key) => {
key,
// Invert the operator to normalize the expression order. // Invert the operator to normalize the expression order.
operator: operator.invert(), if let Some(operator) = operator.invert() {
value: l_string, 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
}
}
// `"test" in extras` or `"dev" in dependency_groups` // `"test" in extras` or `"dev" in dependency_groups`
MarkerValue::MarkerEnvList(key) => { MarkerValue::MarkerEnvList(key) => {
let operator = let operator =
@ -484,9 +498,6 @@ fn parse_inverted_version_expr(
key: MarkerValueVersion, key: MarkerValueVersion,
reporter: &mut impl Reporter, reporter: &mut impl Reporter,
) -> Option<MarkerExpression> { ) -> 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. // Not star allowed here, `'3.*' == python_version` is not a valid PEP 440 comparison.
let version = match value.parse::<Version>() { let version = match value.parse::<Version>() {
Ok(version) => version, Ok(version) => version,
@ -503,6 +514,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 { let Some(operator) = marker_operator.to_pep440_operator() else {
reporter.report( reporter.report(
MarkerWarningKind::Pep440Error, MarkerWarningKind::Pep440Error,

View File

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

View File

@ -268,20 +268,22 @@ impl MarkerOperator {
} }
/// Inverts this marker operator. /// 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::LessThan => Self::GreaterThan,
Self::LessEqual => Self::GreaterEqual, Self::LessEqual => Self::GreaterEqual,
Self::GreaterThan => Self::LessThan, Self::GreaterThan => Self::LessThan,
Self::GreaterEqual => Self::LessEqual, Self::GreaterEqual => Self::LessEqual,
Self::Equal => Self::Equal, Self::Equal => Self::Equal,
Self::NotEqual => Self::NotEqual, Self::NotEqual => Self::NotEqual,
Self::TildeEqual => Self::TildeEqual, Self::TildeEqual => return None,
Self::In => Self::Contains, Self::In => Self::Contains,
Self::NotIn => Self::NotContains, Self::NotIn => Self::NotContains,
Self::Contains => Self::In, Self::Contains => Self::In,
Self::NotContains => Self::NotIn, Self::NotContains => Self::NotIn,
} })
} }
/// Negates this marker operator. /// Negates this marker operator.
@ -513,6 +515,12 @@ pub enum MarkerExpression {
key: MarkerValueVersion, key: MarkerValueVersion,
specifier: VersionSpecifier, 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 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 /// A special case of [`MarkerExpression::String`] with the [`MarkerOperator::In`] operator for
@ -526,7 +534,7 @@ pub enum MarkerExpression {
versions: Vec<Version>, versions: Vec<Version>,
operator: ContainerOperator, operator: ContainerOperator,
}, },
/// 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. /// Inverted string expressions, e.g `'...' == sys_platform`, are also normalized to this form.
String { String {
@ -551,6 +559,8 @@ pub enum MarkerExpression {
pub(crate) enum MarkerExpressionKind { pub(crate) enum MarkerExpressionKind {
/// A version expression, e.g. `<version key> <version op> <quoted PEP 440 version>`. /// A version expression, e.g. `<version key> <version op> <quoted PEP 440 version>`.
Version(MarkerValueVersion), 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>`. /// A version `in` expression, e.g. `<version key> in <quoted list of PEP 440 versions>`.
VersionIn(MarkerValueVersion), VersionIn(MarkerValueVersion),
/// A string marker comparison, e.g. `sys_platform == '...'`. /// A string marker comparison, e.g. `sys_platform == '...'`.
@ -667,6 +677,9 @@ impl MarkerExpression {
pub(crate) fn kind(&self) -> MarkerExpressionKind { pub(crate) fn kind(&self) -> MarkerExpressionKind {
match self { match self {
MarkerExpression::Version { key, .. } => MarkerExpressionKind::Version(*key), MarkerExpression::Version { key, .. } => MarkerExpressionKind::Version(*key),
MarkerExpression::VersionInvertedTilde { key, .. } => {
MarkerExpressionKind::VersionInvertedTilde(*key)
}
MarkerExpression::VersionIn { key, .. } => MarkerExpressionKind::VersionIn(*key), MarkerExpression::VersionIn { key, .. } => MarkerExpressionKind::VersionIn(*key),
MarkerExpression::String { key, .. } => MarkerExpressionKind::String(*key), MarkerExpression::String { key, .. } => MarkerExpressionKind::String(*key),
MarkerExpression::List { pair, .. } => MarkerExpressionKind::List(pair.key()), MarkerExpression::List { pair, .. } => MarkerExpressionKind::List(pair.key()),
@ -686,6 +699,10 @@ impl Display for MarkerExpression {
} }
write!(f, "{key} {op} '{version}'") write!(f, "{key} {op} '{version}'")
} }
MarkerExpression::VersionInvertedTilde { key, specifier } => {
let (op, version) = (specifier.operator(), specifier.version());
write!(f, "'{version}' {op} {key}")
}
MarkerExpression::VersionIn { MarkerExpression::VersionIn {
key, key,
versions, versions,
@ -703,7 +720,8 @@ impl Display for MarkerExpression {
operator, operator,
MarkerOperator::Contains | MarkerOperator::NotContains 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}'") write!(f, "{key} {operator} '{value}'")
@ -2843,6 +2861,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. /// This tests marker implication.
/// ///
/// Specifically, these test cases come from a [bug] where `foo` and `bar` /// Specifically, these test cases come from a [bug] where `foo` and `bar`
@ -2926,7 +2977,7 @@ mod test {
); );
assert_eq!( assert_eq!(
m("'3.6' ~= python_version").negate(), 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!( assert_eq!(
m("python_version ~= '3.6.2'").negate(), m("python_version ~= '3.6.2'").negate(),