Skip to content

Commit

Permalink
Avoid AND-ing multi-term specifiers in marker normalization (#4911)
Browse files Browse the repository at this point in the history
## Summary

Given `python_version != '3.8' and python_version < '3.10'`, the first
term was expanded to `python_version < '3.8'` and `python_version >
'3.8'`. We then AND'd all three terms together. We don't seem to have a
way to differentiate between the terms to AND and the terms to OR in the
normalization code (it all gets flattened together), so instead this PR
expands the expressions at the leaf level and then flattens them at the
level above when appropriate.

Closes #4910.
  • Loading branch information
charliermarsh authored Jul 9, 2024
1 parent b4a7d96 commit a046d23
Showing 1 changed file with 67 additions and 4 deletions.
71 changes: 67 additions & 4 deletions crates/uv-resolver/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ pub(crate) fn normalize_all(tree: MarkerTree) -> Option<MarkerTree> {
for subtree in trees {
// Simplify nested expressions as much as possible first.
//
// If the expression gets normalized out (e.g., `version < '3.8' and version >= '3.8'`), omit it.
// If the expression gets normalized out (e.g., `version < '3.8' and version >= '3.8'`),
// omit it.
let Some(subtree) = normalize_all(subtree) else {
continue;
};
Expand Down Expand Up @@ -207,13 +208,29 @@ pub(crate) fn normalize_all(tree: MarkerTree) -> Option<MarkerTree> {
for subtree in trees {
// Simplify nested expressions as much as possible first.
//
// If the expression gets normalized out (e.g., `version < '3.8' and version >= '3.8'`), return `true`.
// If the expression gets normalized out (e.g., `version < '3.8' and version >= '3.8'`),
// return `true`.
let subtree = normalize_all(subtree)?;

match subtree {
MarkerTree::And(_) => reduced.push(subtree),
// Flatten nested `Or` expressions.
MarkerTree::Or(subtrees) => reduced.extend(subtrees),
MarkerTree::Or(subtrees) => {
for subtree in subtrees {
match subtree {
// Look one level deeper for expressions to simplify, as
// `normalize_all` can return `MarkerTree::Or` for some expressions.
MarkerTree::Expression(ref expr) => {
if let Some((key, range)) = keyed_range(expr) {
versions.entry(key.clone()).or_default().push(range);
continue;
}
reduced.push(subtree);
}
_ => reduced.push(subtree),
}
}
}
// Extract expressions we may be able to simplify more.
MarkerTree::Expression(ref expr) => {
if let Some((key, range)) = keyed_range(expr) {
Expand Down Expand Up @@ -243,7 +260,35 @@ pub(crate) fn normalize_all(tree: MarkerTree) -> Option<MarkerTree> {
}
}

MarkerTree::Expression(_) => Some(tree),
MarkerTree::Expression(ref expr) => {
if let Some((key, range)) = keyed_range(expr) {
// If multiple terms are required to express the range, flatten them into an `Or`
// expression.
let mut iter = range.iter().flat_map(VersionSpecifier::from_bounds);
let first = iter.next().unwrap();
if let Some(second) = iter.next() {
Some(MarkerTree::Or(
std::iter::once(first)
.chain(std::iter::once(second))
.chain(iter)
.map(|specifier| {
MarkerTree::Expression(MarkerExpression::Version {
key: key.clone(),
specifier,
})
})
.collect(),
))
} else {
Some(MarkerTree::Expression(MarkerExpression::Version {
key: key.clone(),
specifier: first,
}))
}
} else {
Some(tree)
}
}
}
}

Expand Down Expand Up @@ -762,6 +807,24 @@ mod tests {
"extra == 'a' and (python_version < '3.12.0rc1' or python_version >= '3.12.0rc1')",
"extra == 'a'",
);

// normalize `!=` operators
assert_normalizes_out("python_version != '3.10' or python_version < '3.12'");

assert_normalizes_to(
"python_version != '3.10' or python_version > '3.12'",
"python_version < '3.10' or python_version > '3.10'",
);

assert_normalizes_to(
"python_version != '3.8' and python_version < '3.10'",
"python_version < '3.10' and (python_version < '3.8' or python_version > '3.8')",
);

assert_normalizes_to(
"python_version != '3.8' and python_version != '3.9'",
"(python_version < '3.8' or python_version > '3.8') and (python_version < '3.9' or python_version > '3.9')",
);
}

#[test]
Expand Down

0 comments on commit a046d23

Please sign in to comment.