Combine sys_platform and platform_system in marker algebra

Author: charliermarsh

crates/uv-pep508/src/marker/algebra.rs

@@ -56,11 +56,9 @@ use std::sync::LazyLock;
 use uv_pep440::{release_specifier_to_range, Operator, Version, VersionSpecifier};
 use version_ranges::Ranges;
 
-use crate::marker::lowering::{
-    LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion,
-};
+use crate::marker::lowering::{LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion, Platform};
 use crate::marker::MarkerValueExtra;
-use crate::ExtraOperator;
+use crate::{ExtraOperator, MarkerValueString};
 use crate::{MarkerExpression, MarkerOperator, MarkerValueVersion};
 
 /// The global node interner.
@@ -260,6 +258,24 @@ impl InternerGuard<'_> {
                 },
                 Edges::from_bool(false),
             ),
+            // A variable representing the output of a platform key. Edges correspond
+            // to disjoint platform ranges.
+            MarkerExpression::String {
+                key: MarkerValueString::PlatformSystem,
+                operator,
+                value,
+            } => {
+                let platform = Platform::from_platform_system(value);
+                (Variable::Platform, Edges::from_platform(operator, platform))
+            }
+            MarkerExpression::String {
+                key: MarkerValueString::SysPlatform,
+                operator,
+                value,
+            } => {
+                let platform = Platform::from_sys_platform(value);
+                (Variable::Platform, Edges::from_platform(operator, platform))
+            }
             // A variable representing the output of a string key. Edges correspond
             // to disjoint string ranges.
             MarkerExpression::String {
@@ -624,6 +640,8 @@ pub(crate) enum Variable {
     Version(LoweredMarkerValueVersion),
     /// A string marker, such as `os_name`.
     String(LoweredMarkerValueString),
+    /// A platform marker, combining `sys_platform` with `platform_system`.
+    Platform,
     /// A variable representing a `<key> in <value>` expression for a particular
     /// string marker and value.
     In {
@@ -748,6 +766,11 @@ pub(crate) enum Edges {
     String {
         edges: SmallVec<(Ranges<String>, NodeId)>,
     },
+    // The edges of a platform variable, representing a disjoint set of ranges that cover
+    // the output space.
+    Platform {
+        edges: SmallVec<(Ranges<Platform>, NodeId)>,
+    },
     // The edges of a boolean variable, representing the values `true` (the `high` child)
     // and `false` (the `low` child).
     Boolean {
@@ -793,6 +816,24 @@ impl Edges {
         }
     }
 
+    /// Returns the [`Edges`] for a platform expression.
+    fn from_platform(operator: MarkerOperator, value: Platform) -> Edges {
+        let range: Ranges<Platform> = match operator {
+            MarkerOperator::Equal => Ranges::singleton(value),
+            MarkerOperator::NotEqual => Ranges::singleton(value).complement(),
+            MarkerOperator::GreaterThan => Ranges::strictly_higher_than(value),
+            MarkerOperator::GreaterEqual => Ranges::higher_than(value),
+            MarkerOperator::LessThan => Ranges::strictly_lower_than(value),
+            MarkerOperator::LessEqual => Ranges::lower_than(value),
+            MarkerOperator::TildeEqual => unreachable!("string comparisons with ~= are ignored"),
+            _ => unreachable!("`in` and `contains` are treated as boolean variables"),
+        };
+
+        Edges::Platform {
+            edges: Edges::from_range(&range),
+        }
+    }
+
     /// Returns the [`Edges`] for a version specifier.
     fn from_specifier(specifier: VersionSpecifier) -> Edges {
         let specifier = release_specifier_to_range(normalize_specifier(specifier));
@@ -901,6 +942,9 @@ impl Edges {
             (Edges::String { edges }, Edges::String { edges: right_edges }) => Edges::String {
                 edges: Edges::apply_ranges(edges, parent, right_edges, right_parent, apply),
             },
+            (Edges::Platform { edges }, Edges::Platform { edges: right_edges }) => Edges::Platform {
+                edges: Edges::apply_ranges(edges, parent, right_edges, right_parent, apply),
+            },
             // For boolean variables, we simply merge the low and high edges.
             (
                 Edges::Boolean { high, low },
@@ -1000,6 +1044,9 @@ impl Edges {
             (Edges::String { edges }, Edges::String { edges: right_edges }) => {
                 Edges::is_disjoint_ranges(edges, parent, right_edges, right_parent, interner)
             }
+            (Edges::Platform { edges }, Edges::Platform { edges: right_edges }) => {
+                Edges::is_disjoint_ranges(edges, parent, right_edges, right_parent, interner)
+            }
             // For boolean variables, we simply check the low and high edges.
             (
                 Edges::Boolean { high, low },
@@ -1065,6 +1112,13 @@ impl Edges {
                     .map(|(range, node)| (range, f(node.negate(parent))))
                     .collect(),
             },
+            Edges::Platform { edges: map } => Edges::Platform {
+                edges: map
+                    .iter()
+                    .cloned()
+                    .map(|(range, node)| (range, f(node.negate(parent))))
+                    .collect(),
+            },
             Edges::Boolean { high, low } => Edges::Boolean {
                 low: f(low.negate(parent)),
                 high: f(high.negate(parent)),
@@ -1081,7 +1135,10 @@ impl Edges {
             Edges::String { edges: map } => {
                 Either::Left(Either::Right(map.iter().map(|(_, node)| *node)))
             }
-            Edges::Boolean { high, low } => Either::Right([*high, *low].into_iter()),
+            Edges::Platform { edges: map } => {
+                Either::Right(Either::Left(map.iter().map(|(_, node)| *node)))
+            }
+            Edges::Boolean { high, low } => Either::Right(Either::Right([*high, *low].into_iter())),
         }
     }
 
@@ -1100,6 +1157,12 @@ impl Edges {
                     .map(|(range, node)| (range, node.not()))
                     .collect(),
             },
+            Edges::Platform { edges: map } => Edges::Platform {
+                edges: map
+                    .into_iter()
+                    .map(|(range, node)| (range, node.not()))
+                    .collect(),
+            },
             Edges::Boolean { high, low } => Edges::Boolean {
                 high: high.not(),
                 low: low.not(),

crates/uv-pep508/src/marker/lowering.rs

@@ -141,3 +141,65 @@ impl Display for LoweredMarkerValueExtra {
         }
     }
 }
+
+/// The [`MarkerValue`] value used in `platform` markers. A combination of `sys_platform` and
+/// `platform_system`.
+///
+/// The core idea is that we normalize `platform_system == 'Linux'` to `sys_platform == 'linux'`,
+/// `platform_system == 'Windows'` to `sys_platform == 'win32'`, and `platform_system == 'Darwin'`
+/// to `sys_platform == 'darwin'`. Apart from that, values are passed through as-is.
+#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
+#[allow(clippy::enum_variant_names)]
+pub enum Platform {
+    Darwin,
+    Linux,
+    Windows,
+    SysPlatform(String),
+    PlatformSystem(String),
+}
+
+impl Platform {
+    /// Construct a [`Platform`] from the `sys_platform` value.
+    pub fn from_sys_platform(value: String) -> Self {
+        match value.as_str() {
+            "linux" => Self::Linux,
+            "win32" => Self::Windows,
+            "darwin" => Self::Darwin,
+            _ => Self::SysPlatform(value),
+        }
+    }
+
+    /// Construct a [`Platform`] from the `platform_system` value.
+    pub fn from_platform_system(value: String) -> Self {
+        match value.as_str() {
+            "Linux" => Self::Linux,
+            "Windows" => Self::Windows,
+            "Darwin" => Self::Darwin,
+            _ => Self::PlatformSystem(value),
+        }
+    }
+}
+
+impl Display for Platform {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Platform::Linux => f.write_str("sys_platform == 'linux'"),
+            Platform::Windows => f.write_str("sys_platform == 'win32'"),
+            Platform::Darwin => f.write_str("sys_platform == 'darwin'"),
+            Platform::SysPlatform(platform) => write!(f, "sys_platform == '{platform}'"),
+            Platform::PlatformSystem(system) => write!(f, "platform_system == '{system}'"),
+        }
+    }
+}
+
+impl From<Platform> for (MarkerValueString, String) {
+    fn from(value: Platform) -> Self {
+        match value {
+            Platform::Linux => (MarkerValueString::SysPlatform, "linux".to_string()),
+            Platform::Windows => (MarkerValueString::SysPlatform, "win32".to_string()),
+            Platform::Darwin => (MarkerValueString::SysPlatform, "darwin".to_string()),
+            Platform::SysPlatform(value) => (MarkerValueString::SysPlatform, value),
+            Platform::PlatformSystem(value) => (MarkerValueString::PlatformSystem, value),
+        }
+    }
+}

crates/uv-pep508/src/marker/simplify.rs

@@ -120,6 +120,41 @@ fn collect_dnf(
                 }
             }
         }
+        MarkerTreeKind::Platform(marker) => {
+            for (tree, range) in collect_edges(marker.children()) {
+                // Detect whether the range for this edge can be simplified as an inequality.
+                if let Some(excluded) = range_inequality(&range) {
+                    let current = path.len();
+                    for value in excluded {
+                        let (key, value) = value.clone().into();
+                        path.push(MarkerExpression::String {
+                            key,
+                            operator: MarkerOperator::NotEqual,
+                            value,
+                        });
+                    }
+
+                    collect_dnf(&tree, dnf, path);
+                    path.truncate(current);
+                    continue;
+                }
+
+                for bounds in range.iter() {
+                    let current = path.len();
+                    for (operator, value) in MarkerOperator::from_bounds(bounds) {
+                        let (key, value) = value.clone().into();
+                        path.push(MarkerExpression::String {
+                            key,
+                            operator,
+                            value,
+                        });
+                    }
+
+                    collect_dnf(&tree, dnf, path);
+                    path.truncate(current);
+                }
+            }
+        }
         MarkerTreeKind::In(marker) => {
             for (value, tree) in marker.children() {
                 let operator = if value {