Support extras and dependency_groups markers in PEP 508 grammar

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

@@ -59,8 +59,10 @@ use uv_pep440::{Operator, Version, VersionSpecifier, release_specifier_to_range}
 
 use crate::marker::MarkerValueExtra;
 use crate::marker::lowering::{
-    CanonicalMarkerValueExtra, CanonicalMarkerValueString, CanonicalMarkerValueVersion,
+    CanonicalMarkerValueDependencyGroup, CanonicalMarkerValueExtra, CanonicalMarkerValueString,
+    CanonicalMarkerValueVersion,
 };
+use crate::marker::tree::{ContainerOperator, MarkerValueDependencyGroup};
 use crate::{
     ExtraOperator, MarkerExpression, MarkerOperator, MarkerValueString, MarkerValueVersion,
 };
@@ -328,11 +330,53 @@ impl InternerGuard<'_> {
                 Variable::Extra(CanonicalMarkerValueExtra::Extra(extra)),
                 Edges::from_bool(false),
             ),
-            // Invalid extras are always `false`.
+            // Invalid `extra` names are always `false`.
             MarkerExpression::Extra {
                 name: MarkerValueExtra::Arbitrary(_),
                 ..
             } => return NodeId::FALSE,
+            // A variable representing the existence or absence of a particular extra, in the
+            // context of a PEP 751 lockfile.
+            MarkerExpression::Extras {
+                name: MarkerValueExtra::Extra(extra),
+                operator: ContainerOperator::In,
+            } => (
+                Variable::Extras(CanonicalMarkerValueExtra::Extra(extra)),
+                Edges::from_bool(true),
+            ),
+            MarkerExpression::Extras {
+                name: MarkerValueExtra::Extra(extra),
+                operator: ContainerOperator::NotIn,
+            } => (
+                Variable::Extras(CanonicalMarkerValueExtra::Extra(extra)),
+                Edges::from_bool(false),
+            ),
+            // Invalid `extras` names are always `false`.
+            MarkerExpression::Extras {
+                name: MarkerValueExtra::Arbitrary(_),
+                ..
+            } => return NodeId::FALSE,
+            // A variable representing the existence or absence of a particular extra, in the
+            // context of a PEP 751 lockfile.
+            MarkerExpression::DependencyGroups {
+                name: MarkerValueDependencyGroup::Group(group),
+                operator: ContainerOperator::In,
+            } => (
+                Variable::DependencyGroups(CanonicalMarkerValueDependencyGroup::Group(group)),
+                Edges::from_bool(true),
+            ),
+            MarkerExpression::DependencyGroups {
+                name: MarkerValueDependencyGroup::Group(group),
+                operator: ContainerOperator::NotIn,
+            } => (
+                Variable::DependencyGroups(CanonicalMarkerValueDependencyGroup::Group(group)),
+                Edges::from_bool(false),
+            ),
+            // Invalid `dependency_group` names are always `false`.
+            MarkerExpression::DependencyGroups {
+                name: MarkerValueDependencyGroup::Arbitrary(_),
+                ..
+            } => return NodeId::FALSE,
         };
 
         self.create_node(var, children)
@@ -1046,6 +1090,18 @@ pub(crate) enum Variable {
     /// We keep extras at the leaves of the tree, so when simplifying extras we can
     /// trivially remove the leaves without having to reconstruct the entire tree.
     Extra(CanonicalMarkerValueExtra),
+    /// A variable representing the existence or absence of a given extra, in the context of a
+    /// PEP 751 lockfile marker.
+    ///
+    /// We keep extras at the leaves of the tree, so when simplifying extras we can
+    /// trivially remove the leaves without having to reconstruct the entire tree.
+    Extras(CanonicalMarkerValueExtra),
+    /// A variable representing the existence or absence of a given dependency group, in the context of a
+    /// PEP 751 lockfile marker.
+    ///
+    /// We keep groups at the leaves of the tree, so when simplifying groups we can
+    /// trivially remove the leaves without having to reconstruct the entire tree.
+    DependencyGroups(CanonicalMarkerValueDependencyGroup),
 }
 
 impl Variable {

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

@@ -1,7 +1,8 @@
 use std::fmt::{Display, Formatter};
 
-use uv_normalize::ExtraName;
+use uv_normalize::{ExtraName, GroupName};
 
+use crate::marker::tree::MarkerValueDependencyGroup;
 use crate::{MarkerValueExtra, MarkerValueString, MarkerValueVersion};
 
 /// Those environment markers with a PEP 440 version as value such as `python_version`
@@ -128,7 +129,7 @@ impl Display for CanonicalMarkerValueString {
     }
 }
 
-/// The [`ExtraName`] value used in `extra` markers.
+/// The [`ExtraName`] value used in `extra` and `extras` markers.
 #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
 pub enum CanonicalMarkerValueExtra {
     /// A valid [`ExtraName`].
@@ -159,3 +160,35 @@ impl Display for CanonicalMarkerValueExtra {
         }
     }
 }
+
+/// The [`GroupName`] value used in `dependency_group` markers.
+#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
+pub enum CanonicalMarkerValueDependencyGroup {
+    /// A valid [`GroupName`].
+    Group(GroupName),
+}
+
+impl CanonicalMarkerValueDependencyGroup {
+    /// Returns the [`GroupName`] value.
+    pub fn group(&self) -> &GroupName {
+        match self {
+            Self::Group(group) => group,
+        }
+    }
+}
+
+impl From<CanonicalMarkerValueDependencyGroup> for MarkerValueDependencyGroup {
+    fn from(value: CanonicalMarkerValueDependencyGroup) -> Self {
+        match value {
+            CanonicalMarkerValueDependencyGroup::Group(group) => Self::Group(group),
+        }
+    }
+}
+
+impl Display for CanonicalMarkerValueDependencyGroup {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Group(group) => group.fmt(f),
+        }
+    }
+}

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

@@ -1,10 +1,11 @@
 use arcstr::ArcStr;
 use std::str::FromStr;
-use uv_normalize::ExtraName;
+use uv_normalize::{ExtraName, GroupName};
 use uv_pep440::{Version, VersionPattern, VersionSpecifier};
 
 use crate::cursor::Cursor;
 use crate::marker::MarkerValueExtra;
+use crate::marker::tree::{ContainerOperator, MarkerValueDependencyGroup};
 use crate::{
     ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueString,
     MarkerValueVersion, MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
@@ -208,6 +209,8 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
         MarkerValue::MarkerEnvString(key) => {
             let value = match r_value {
                 MarkerValue::Extra
+                | MarkerValue::Extras
+                | MarkerValue::DependencyGroups
                 | MarkerValue::MarkerEnvVersion(_)
                 | MarkerValue::MarkerEnvString(_) => {
                     reporter.report(
@@ -242,7 +245,9 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
             let value = match r_value {
                 MarkerValue::MarkerEnvVersion(_)
                 | MarkerValue::MarkerEnvString(_)
-                | MarkerValue::Extra => {
+                | MarkerValue::Extra
+                | MarkerValue::Extras
+                | MarkerValue::DependencyGroups => {
                     reporter.report(
                         MarkerWarningKind::ExtraInvalidComparison,
                         "Comparing extra with something other than a quoted string is wrong,
@@ -257,7 +262,7 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
 
             parse_extra_expr(operator, &value, reporter)
         }
-        // This is either MarkerEnvVersion, MarkerEnvString or Extra inverted
+        // This is either MarkerEnvVersion, MarkerEnvString, Extra (inverted), or Extras
         MarkerValue::QuotedString(l_string) => {
             match r_value {
                 // The only sound choice for this is `<quoted PEP 440 version> <version op>` <version key>
@@ -273,6 +278,12 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
                 }),
                 // `'...' == extra`
                 MarkerValue::Extra => parse_extra_expr(operator, &l_string, reporter),
+                // `'...' in extras`
+                MarkerValue::Extras => parse_extras_expr(operator, &l_string, reporter),
+                // `'...' in dependency_groups`
+                MarkerValue::DependencyGroups => {
+                    parse_dependency_groups_expr(operator, &l_string, reporter)
+                }
                 // `'...' == '...'`, doesn't make much sense
                 MarkerValue::QuotedString(_) => {
                     // Not even pypa/packaging 22.0 supports this
@@ -289,6 +300,26 @@ pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
                 }
             }
         }
+        MarkerValue::Extras => {
+            reporter.report(
+                MarkerWarningKind::Pep440Error,
+                format!(
+                    "The `extras` marker must be used as '...' in extras' or '... not in extras',
+                    found `{l_value} {operator} {r_value}`, will be ignored"
+                ),
+            );
+            return Ok(None);
+        }
+        MarkerValue::DependencyGroups => {
+            reporter.report(
+                MarkerWarningKind::Pep440Error,
+                format!(
+                    "The `dependency_groups` marker must be used as '...' in dependency_groups' or '... not in dependency_groups',
+                    found `{l_value} {operator} {r_value}`, will be ignored"
+                ),
+            );
+            return Ok(None);
+        }
     };
 
     Ok(expr)
@@ -491,8 +522,69 @@ fn parse_extra_expr(
 
     reporter.report(
         MarkerWarningKind::ExtraInvalidComparison,
-        "Comparing extra with something other than a quoted string is wrong,
-        will be ignored"
+        "Comparing `extra` with any operator other than `==` or `!=` is wrong and will be ignored"
+            .to_string(),
+    );
+
+    None
+}
+
+/// Creates an instance of [`MarkerExpression::Extras`] with the given values, falling back to
+/// [`MarkerExpression::Arbitrary`] on failure.
+fn parse_extras_expr(
+    operator: MarkerOperator,
+    value: &str,
+    reporter: &mut impl Reporter,
+) -> Option<MarkerExpression> {
+    let name = match ExtraName::from_str(value) {
+        Ok(name) => MarkerValueExtra::Extra(name),
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::ExtrasInvalidComparison,
+                format!("Expected extra name (found `{value}`): {err}"),
+            );
+            MarkerValueExtra::Arbitrary(value.to_string())
+        }
+    };
+
+    if let Some(operator) = ContainerOperator::from_marker_operator(operator) {
+        return Some(MarkerExpression::Extras { operator, name });
+    }
+
+    reporter.report(
+        MarkerWarningKind::ExtrasInvalidComparison,
+        "Comparing `extras` with any operator other than `in` or `not in` is wrong and will be ignored"
+            .to_string(),
+    );
+
+    None
+}
+
+/// Creates an instance of [`MarkerExpression::DependencyGroups`] with the given values, falling
+/// back to [`MarkerExpression::Arbitrary`] on failure.
+fn parse_dependency_groups_expr(
+    operator: MarkerOperator,
+    value: &str,
+    reporter: &mut impl Reporter,
+) -> Option<MarkerExpression> {
+    let name = match GroupName::from_str(value) {
+        Ok(name) => MarkerValueDependencyGroup::Group(name),
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::ExtrasInvalidComparison,
+                format!("Expected extra name (found `{value}`): {err}"),
+            );
+            MarkerValueDependencyGroup::Arbitrary(value.to_string())
+        }
+    };
+
+    if let Some(operator) = ContainerOperator::from_marker_operator(operator) {
+        return Some(MarkerExpression::DependencyGroups { operator, name });
+    }
+
+    reporter.report(
+        MarkerWarningKind::ExtrasInvalidComparison,
+        "Comparing `extras` with any operator other than `in` or `not in` is wrong and will be ignored"
             .to_string(),
     );
 

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

@@ -9,6 +9,7 @@ use version_ranges::Ranges;
 
 use uv_pep440::{Version, VersionSpecifier};
 
+use crate::marker::tree::ContainerOperator;
 use crate::{ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeKind};
 
 /// Returns a simplified DNF expression for a given marker tree.
@@ -174,6 +175,42 @@ fn collect_dnf(
                     operator,
                 };
 
+                path.push(expr);
+                collect_dnf(tree, dnf, path);
+                path.pop();
+            }
+        }
+        MarkerTreeKind::Extras(marker) => {
+            for (value, tree) in marker.children() {
+                let operator = if value {
+                    ContainerOperator::In
+                } else {
+                    ContainerOperator::NotIn
+                };
+
+                let expr = MarkerExpression::Extras {
+                    name: marker.name().clone().into(),
+                    operator,
+                };
+
+                path.push(expr);
+                collect_dnf(tree, dnf, path);
+                path.pop();
+            }
+        }
+        MarkerTreeKind::DependencyGroups(marker) => {
+            for (value, tree) in marker.children() {
+                let operator = if value {
+                    ContainerOperator::In
+                } else {
+                    ContainerOperator::NotIn
+                };
+
+                let expr = MarkerExpression::DependencyGroups {
+                    name: marker.name().clone().into(),
+                    operator,
+                };
+
                 path.push(expr);
                 collect_dnf(tree, dnf, path);
                 path.pop();
@@ -440,5 +477,27 @@ fn is_negation(left: &MarkerExpression, right: &MarkerExpression) -> bool {
 
             name == name2 && operator.negate() == *operator2
         }
+        MarkerExpression::Extras { name, operator } => {
+            let MarkerExpression::Extras {
+                name: name2,
+                operator: operator2,
+            } = right
+            else {
+                return false;
+            };
+
+            name == name2 && *operator == operator2.negate()
+        }
+        MarkerExpression::DependencyGroups { name, operator } => {
+            let MarkerExpression::DependencyGroups {
+                name: name2,
+                operator: operator2,
+            } = right
+            else {
+                return false;
+            };
+
+            name == name2 && *operator == operator2.negate()
+        }
     }
 }