Treat invalid extras as false in marker evaluation (#6395)

## Summary

Closes https://github.com/astral-sh/uv/issues/6279.

Closes https://github.com/astral-sh/uv/pull/6395.

crates/pep508-rs/src/lib.rs

@@ -37,6 +37,7 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
 use thiserror::Error;
 use url::Url;
 
+use crate::marker::MarkerValueExtra;
 use cursor::Cursor;
 pub use marker::{
     ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerEnvironment,
@@ -408,7 +409,7 @@ impl<T: Pep508Url> Requirement<T> {
         self.marker
             .and(MarkerTree::expression(MarkerExpression::Extra {
                 operator: ExtraOperator::Equal,
-                name: extra.clone(),
+                name: MarkerValueExtra::Extra(extra.clone()),
             }));
 
         self

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

@@ -56,9 +56,9 @@ use pep440_rs::{Version, VersionSpecifier};
 use pubgrub::Range;
 use rustc_hash::FxHashMap;
 use std::sync::LazyLock;
-use uv_normalize::ExtraName;
 use uv_pubgrub::PubGrubSpecifier;
 
+use crate::marker::MarkerValueExtra;
 use crate::ExtraOperator;
 use crate::{MarkerExpression, MarkerOperator, MarkerValueString, MarkerValueVersion};
 
@@ -446,7 +446,7 @@ 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(ExtraName),
+    Extra(MarkerValueExtra),
 }
 
 /// A decision node in an Algebraic Decision Diagram.

crates/pep508-rs/src/marker/mod.rs

@@ -18,8 +18,9 @@ mod tree;
 pub use environment::{MarkerEnvironment, MarkerEnvironmentBuilder};
 pub use tree::{
     ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerExpression,
-    MarkerOperator, MarkerTree, MarkerTreeContents, MarkerTreeKind, MarkerValue, MarkerValueString,
+    MarkerOperator, MarkerTree, MarkerTreeContents, MarkerTreeKind, MarkerValue, MarkerValueExtra,
-    MarkerValueVersion, MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree,
+    MarkerValueString, MarkerValueVersion, MarkerWarningKind, StringMarkerTree, StringVersion,
+    VersionMarkerTree,
 };
 
 /// `serde` helpers for [`MarkerTree`].

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

@@ -4,6 +4,7 @@ use pep440_rs::{Version, VersionPattern, VersionSpecifier};
 use uv_normalize::ExtraName;
 
 use crate::cursor::Cursor;
+use crate::marker::MarkerValueExtra;
 use crate::{
     ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueVersion,
     MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
@@ -427,14 +428,13 @@ fn parse_extra_expr(
     reporter: &mut impl Reporter,
 ) -> Option<MarkerExpression> {
     let name = match ExtraName::from_str(value) {
-        Ok(name) => name,
+        Ok(name) => MarkerValueExtra::Extra(name),
         Err(err) => {
             reporter.report(
                 MarkerWarningKind::ExtraInvalidComparison,
-                format!("Expected extra name, found '{value}', will be ignored: {err}"),
+                format!("Expected extra name (found `{value}`): {err}"),
             );
-
+            MarkerValueExtra::Arbitrary(value.to_string())
-            return None;
         }
     };
 

crates/pep508-rs/src/marker/tree.rs

@@ -435,6 +435,33 @@ impl Deref for StringVersion {
     }
 }
 
+/// The [`ExtraName`] value used in `extra` markers.
+#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
+pub enum MarkerValueExtra {
+    /// A valid [`ExtraName`].
+    Extra(ExtraName),
+    /// An invalid name, preserved as an arbitrary string.
+    Arbitrary(String),
+}
+
+impl MarkerValueExtra {
+    fn as_extra(&self) -> Option<&ExtraName> {
+        match self {
+            Self::Extra(extra) => Some(extra),
+            Self::Arbitrary(_) => None,
+        }
+    }
+}
+
+impl Display for MarkerValueExtra {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Extra(extra) => extra.fmt(f),
+            Self::Arbitrary(string) => string.fmt(f),
+        }
+    }
+}
+
 /// Represents one clause such as `python_version > "3.8"`.
 #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
 #[allow(missing_docs)]
@@ -471,7 +498,7 @@ pub enum MarkerExpression {
     /// `extra <extra op> '...'` or `'...' <extra op> extra`.
     Extra {
         operator: ExtraOperator,
-        name: ExtraName,
+        name: MarkerValueExtra,
     },
 }
 
@@ -885,7 +912,12 @@ impl MarkerTree {
             }
             MarkerTreeKind::Extra(marker) => {
                 return marker
-                    .edge(extras.contains(marker.name()))
+                    .edge(
+                        marker
+                            .name()
+                            .as_extra()
+                            .is_some_and(|extra| extras.contains(extra)),
+                    )
                     .evaluate_reporter_impl(env, extras, reporter);
             }
         }
@@ -931,7 +963,12 @@ impl MarkerTree {
                 .children()
                 .any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
             MarkerTreeKind::Extra(marker) => marker
-                .edge(extras.contains(marker.name()))
+                .edge(
+                    marker
+                        .name()
+                        .as_extra()
+                        .is_some_and(|extra| extras.contains(extra)),
+                )
                 .evaluate_extras_and_python_version(extras, python_versions),
         }
     }
@@ -956,7 +993,12 @@ impl MarkerTree {
                 .children()
                 .any(|(_, tree)| tree.evaluate_extras(extras)),
             MarkerTreeKind::Extra(marker) => marker
-                .edge(extras.contains(marker.name()))
+                .edge(
+                    marker
+                        .name()
+                        .as_extra()
+                        .is_some_and(|extra| extras.contains(extra)),
+                )
                 .evaluate_extras(extras),
         }
     }
@@ -1144,9 +1186,13 @@ impl MarkerTree {
     }
 
     fn simplify_extras_with_impl(self, is_extra: &impl Fn(&ExtraName) -> bool) -> MarkerTree {
-        MarkerTree(INTERNER.lock().restrict(self.0, &|var| match var {
+        MarkerTree(INTERNER.lock().restrict(self.0, &|var| {
-            Variable::Extra(name) => is_extra(name).then_some(true),
+            match var {
-            _ => None,
+                Variable::Extra(name) => name
+                    .as_extra()
+                    .and_then(|name| is_extra(name).then_some(true)),
+                _ => None,
+            }
         }))
     }
 }
@@ -1375,14 +1421,14 @@ impl Ord for ContainsMarkerTree<'_> {
 /// A node representing the existence or absence of a given extra, such as `extra == 'bar'`.
 #[derive(PartialEq, Eq, Clone, Debug)]
 pub struct ExtraMarkerTree<'a> {
-    name: &'a ExtraName,
+    name: &'a MarkerValueExtra,
     high: NodeId,
     low: NodeId,
 }
 
 impl ExtraMarkerTree<'_> {
     /// Returns the name of the extra in this expression.
-    pub fn name(&self) -> &ExtraName {
+    pub fn name(&self) -> &MarkerValueExtra {
         self.name
     }
 

crates/uv/tests/pip_compile.rs

@@ -11829,3 +11829,81 @@ fn compatible_build_constraint() -> Result<()> {
 
     Ok(())
 }
+
+/// Ensure that we treat invalid extra markers as `false`, i.e., in projects that define
+/// non-spec-compliant extras.
+#[test]
+fn invalid_extra() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let setup_py = context.temp_dir.child("setup.py");
+    setup_py.write_str(indoc! {r#"
+        from setuptools import setup
+
+        extras_require = {
+            "_anyio": ["anyio"],
+            "config": ["jsonschema>=2.6.0"],
+            "encryption": ["iniconfig"],
+        }
+
+        setup(name="project", install_requires=[], extras_require=extras_require)
+    "#})?;
+
+    // Sync the `encryption` extra. `anyio` should be omitted.
+    let requirements_in = context.temp_dir.child("requirements.in");
+    requirements_in.write_str(".[encryption]")?;
+
+    uv_snapshot!(context.filters(), context.pip_compile()
+        .arg("requirements.in"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    # This file was autogenerated by uv via the following command:
+    #    uv pip compile --cache-dir [CACHE_DIR] requirements.in
+    iniconfig==2.0.0
+        # via project
+    .
+        # via -r requirements.in
+
+    ----- stderr -----
+    Resolved 2 packages in [TIME]
+    "###);
+
+    // Sync the `_anyio` extra. We should reject it.
+    let requirements_in = context.temp_dir.child("requirements.in");
+    requirements_in.write_str(".[_anyio]")?;
+
+    uv_snapshot!(context.filters(), context.pip_compile()
+        .arg("requirements.in"), @r###"
+    success: false
+    exit_code: 2
+    ----- stdout -----
+
+    ----- stderr -----
+    error: Couldn't parse requirement in `requirements.in` at position 0
+      Caused by: Expected an alphanumeric character starting the extra name, found '_'
+    .[_anyio]
+      ^
+    "###);
+
+    // Sync the `anyio` extra. We should reject it.
+    let requirements_in = context.temp_dir.child("requirements.in");
+    requirements_in.write_str(".[anyio]")?;
+
+    uv_snapshot!(context.filters(), context.pip_compile()
+        .arg("requirements.in"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+    # This file was autogenerated by uv via the following command:
+    #    uv pip compile --cache-dir [CACHE_DIR] requirements.in
+    .
+        # via -r requirements.in
+
+    ----- stderr -----
+    Resolved 1 package in [TIME]
+    warning: The package `project @ file://[TEMP_DIR]/` does not have an extra named `anyio`
+    "###);
+
+    Ok(())
+}