Normalize out complementary == or != markers (#5050)

## Summary

Closes https://github.com/astral-sh/uv/issues/5044.
This commit is contained in:
Charlie Marsh 2024-07-14 19:53:58 -04:00 committed by GitHub
parent 04c96c8df8
commit a571150949
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1,10 +1,10 @@
#![allow(clippy::enum_glob_use, clippy::single_match_else)] #![allow(clippy::enum_glob_use, clippy::single_match_else)]
use std::collections::HashMap;
use std::ops::Bound::{self, *}; use std::ops::Bound::{self, *};
use std::ops::RangeBounds; use std::ops::RangeBounds;
use pubgrub::range::{Range as PubGrubRange, Range}; use pubgrub::range::{Range as PubGrubRange, Range};
use rustc_hash::FxHashMap;
use pep440_rs::{Operator, Version, VersionSpecifier}; use pep440_rs::{Operator, Version, VersionSpecifier};
use pep508_rs::{ use pep508_rs::{
@ -205,7 +205,7 @@ pub(crate) fn normalize_all(
match tree { match tree {
MarkerTree::And(trees) => { MarkerTree::And(trees) => {
let mut reduced = Vec::new(); let mut reduced = Vec::new();
let mut versions: HashMap<_, Vec<_>> = HashMap::new(); let mut versions: FxHashMap<_, Vec<_>> = FxHashMap::default();
for subtree in trees { for subtree in trees {
// Normalize nested expressions as much as possible first. // Normalize nested expressions as much as possible first.
@ -251,7 +251,7 @@ pub(crate) fn normalize_all(
MarkerTree::Or(trees) => { MarkerTree::Or(trees) => {
let mut reduced = Vec::new(); let mut reduced = Vec::new();
let mut versions: HashMap<_, Vec<_>> = HashMap::new(); let mut versions: FxHashMap<_, Vec<_>> = FxHashMap::default();
for subtree in trees { for subtree in trees {
// Normalize nested expressions as much as possible first. // Normalize nested expressions as much as possible first.
@ -301,6 +301,12 @@ pub(crate) fn normalize_all(
reduced.sort(); reduced.sort();
reduced.dedup(); reduced.dedup();
// If the reduced trees contain complementary terms (e.g., `sys_platform == 'linux' or sys_platform != 'linux'`),
// the expression is always true and can be removed.
if contains_complements(&reduced) {
return None;
}
match reduced.len() { match reduced.len() {
0 => None, 0 => None,
1 => Some(reduced.remove(0)), 1 => Some(reduced.remove(0)),
@ -551,7 +557,7 @@ fn filter_conjunctions(tree: &mut MarkerTree, disjunct: &MarkerExpression) -> bo
/// Simplify version expressions. /// Simplify version expressions.
fn simplify_ranges( fn simplify_ranges(
reduced: &mut Vec<MarkerTree>, reduced: &mut Vec<MarkerTree>,
versions: HashMap<MarkerValueVersion, Vec<PubGrubRange<Version>>>, versions: FxHashMap<MarkerValueVersion, Vec<PubGrubRange<Version>>>,
combine: impl Fn(&Vec<PubGrubRange<Version>>) -> PubGrubRange<Version>, combine: impl Fn(&Vec<PubGrubRange<Version>>) -> PubGrubRange<Version>,
) { ) {
for (key, ranges) in versions { for (key, ranges) in versions {
@ -666,6 +672,42 @@ fn version_is_disjoint(this: &MarkerExpression, other: &MarkerExpression) -> boo
range.is_disjoint(&range2) range.is_disjoint(&range2)
} }
/// Return `true` if the tree contains complementary terms (e.g., `sys_platform == 'linux' or sys_platform != 'linux'`).
fn contains_complements(trees: &[MarkerTree]) -> bool {
let mut terms = FxHashMap::default();
for tree in trees {
let MarkerTree::Expression(
MarkerExpression::String {
key,
operator,
value,
}
| MarkerExpression::StringInverted {
value,
operator,
key,
},
) = tree
else {
continue;
};
match operator {
MarkerOperator::Equal => {
if let Some(MarkerOperator::NotEqual) = terms.insert((key, value), operator) {
return true;
}
}
MarkerOperator::NotEqual => {
if let Some(MarkerOperator::Equal) = terms.insert((key, value), operator) {
return true;
}
}
_ => {}
}
}
false
}
/// Returns the key and version range for a version expression. /// Returns the key and version range for a version expression.
fn keyed_range(expr: &MarkerExpression) -> Option<(&MarkerValueVersion, PubGrubRange<Version>)> { fn keyed_range(expr: &MarkerExpression) -> Option<(&MarkerValueVersion, PubGrubRange<Version>)> {
let (key, specifier) = match expr { let (key, specifier) = match expr {
@ -892,6 +934,20 @@ mod tests {
"python_version != '3.8' and python_version != '3.9'", "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')", "(python_version < '3.8' or python_version > '3.8') and (python_version < '3.9' or python_version > '3.9')",
); );
// normalize out redundant expressions
assert_normalizes_out("sys_platform == 'win32' or sys_platform != 'win32'");
assert_normalizes_out("'win32' == sys_platform or sys_platform != 'win32'");
assert_normalizes_out(
"sys_platform == 'win32' or sys_platform == 'win32' or sys_platform != 'win32'",
);
assert_normalizes_to(
"sys_platform == 'win32' and sys_platform != 'win32'",
"sys_platform == 'win32' and sys_platform != 'win32'",
);
} }
#[test] #[test]