Treat invalid version markers as false

This commit is contained in:
Charlie Marsh
2026-06-15 17:55:43 -04:00
parent 49e4be037c
commit 448d517b88
2 changed files with 55 additions and 22 deletions
+30 -7
View File
@@ -12,6 +12,20 @@ use crate::{
MarkerValueVersion, MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
};
struct VersionWarningReporter<'a, R> {
reporter: &'a mut R,
invalid_version: bool,
}
impl<R: Reporter> Reporter for VersionWarningReporter<'_, R> {
fn report(&mut self, kind: MarkerWarningKind, warning: String) {
if kind == MarkerWarningKind::Pep440Error {
self.invalid_version = true;
}
self.reporter.report(kind, warning);
}
}
/// ```text
/// version_cmp = wsp* <'<=' | '<' | '!=' | '==' | '>=' | '>' | '~=' | '==='>
/// marker_op = version_cmp | (wsp* 'in') | (wsp* 'not' wsp+ 'in')
@@ -193,7 +207,7 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
MarkerWarningKind::Pep440Error,
format!(
"Expected double quoted PEP 440 version to compare with {key},
found {r_value}, will be ignored"
found {r_value}, will evaluate to false"
),
);
@@ -417,7 +431,7 @@ fn parse_version_in_expr(
MarkerWarningKind::Pep440Error,
format!(
"Expected PEP 440 versions to compare with {key}, found {value},
will be ignored: {err}"
will evaluate to false: {err}"
),
);
@@ -451,7 +465,7 @@ fn parse_version_expr(
MarkerWarningKind::Pep440Error,
format!(
"Expected PEP 440 version to compare with {key}, found {value},
will be ignored: {err}"
will evaluate to false: {err}"
),
);
@@ -464,7 +478,7 @@ fn parse_version_expr(
MarkerWarningKind::Pep440Error,
format!(
"Expected PEP 440 version operator to compare {key} with `{version}`,
found `{marker_operator}`, will be ignored",
found `{marker_operator}`, will evaluate to false",
version = pattern.version()
),
);
@@ -506,7 +520,7 @@ fn parse_inverted_version_expr(
MarkerWarningKind::Pep440Error,
format!(
"Expected PEP 440 version to compare with {key}, found {value},
will be ignored: {err}"
will evaluate to false: {err}"
),
);
@@ -519,7 +533,7 @@ fn parse_inverted_version_expr(
MarkerWarningKind::Pep440Error,
format!(
"Expected PEP 440 version operator to compare {key} with `{version}`,
found `{marker_operator}`, will be ignored"
found `{marker_operator}`, will evaluate to false"
),
);
@@ -585,7 +599,16 @@ fn parse_marker_expr<T: Pep508Url>(
cursor.next_expect_char(')', start_pos)?;
Ok(marker)
} else {
Ok(parse_marker_key_op_value(cursor, reporter)?.map(MarkerTree::expression))
let mut reporter = VersionWarningReporter {
reporter,
invalid_version: false,
};
let expression = parse_marker_key_op_value(cursor, &mut reporter)?;
if reporter.invalid_version {
Ok(Some(MarkerTree::FALSE))
} else {
Ok(expression.map(MarkerTree::expression))
}
}
}
+25 -15
View File
@@ -2112,6 +2112,19 @@ mod test {
assert!(marker.evaluate(&env37, &[]));
}
#[test]
fn invalid_version_marker_evaluates_to_false() {
assert_eq!(m("python_version >= '3.9.'"), MarkerTree::FALSE);
assert_eq!(
m("python_version >= '3.9.' and sys_platform == 'linux'"),
MarkerTree::FALSE
);
assert_eq!(
m("python_version >= '3.9.' or sys_platform == 'linux'"),
m("sys_platform == 'linux'")
);
}
#[test]
#[cfg(feature = "tracing")]
#[tracing_test::traced_test]
@@ -2205,11 +2218,11 @@ mod test {
.evaluate(&env37, &[]);
assert!(!result);
// Meaningless expressions are ignored, so this is always true.
// Invalid version expressions evaluate to false.
let result = MarkerTree::from_str("'3.*' == python_version")
.unwrap()
.evaluate(&env37, &[]);
assert!(result);
assert!(!result);
}
#[test]
@@ -2459,14 +2472,12 @@ mod test {
"python_version in '3.9.0'",
"python_full_version == '3.9.*'",
);
// e.g., using a version that is not PEP 440 compliant is considered arbitrary
assert_true("python_version in 'foo'");
// e.g., including `*` versions, which would require tracking a version specifier
assert_true("python_version in '3.9.*'");
// e.g., when non-whitespace separators are present
assert_true("python_version in '3.9, 3.10'");
assert_true("python_version in '3.9,3.10'");
assert_true("python_version in '3.9 or 3.10'");
// Invalid PEP 440 versions evaluate to false.
assert_false("python_version in 'foo'");
assert_false("python_version in '3.9.*'");
assert_false("python_version in '3.9, 3.10'");
assert_false("python_version in '3.9,3.10'");
assert_false("python_version in '3.9 or 3.10'");
// This is an edge case that happens to be supported, but is not critical to support.
assert_simplifies(
@@ -3031,9 +3042,8 @@ mod test {
#[test]
fn test_arbitrary_disjointness() {
// `python_version == 'Linux'` is nonsense and ignored, thus the first marker
// is always `true` and not disjoint.
assert!(!is_disjoint(
// Invalid version expressions evaluate to false, so they are disjoint from all markers.
assert!(is_disjoint(
"python_version == 'Linux'",
"python_full_version == '3.7.1'"
));
@@ -3204,8 +3214,8 @@ mod test {
fn test_arbitrary() {
assert!(m("'wat' == 'wat'").is_true());
assert!(m("os_name ~= 'wat'").is_true());
assert!(m("python_version == 'Linux'").is_true());
assert!(m("os_name ~= 'wat' or 'wat' == 'wat' and python_version == 'Linux'").is_true());
assert!(m("python_version == 'Linux'").is_false());
assert!(m("os_name ~= 'wat' or 'wat' == 'wat' and python_version == 'Linux'").is_false());
}
#[test]