Add validation of extra names (#257)

Extends #254 

Adds validation of extra names provided by users in `pip-compile` e.g. 

```
error: invalid value 'foo!' for '--extra <EXTRA>': Extra names must start and end with a
letter or digit and may only contain -, _, ., and alphanumeric characters
```

We'll want to add something similar to `PackageName`. I'd be curious to
improve the AP, making the unvalidated nature of `::normalize` clear?
Perhaps worth pursuing later though as I don't have a better idea.

crates/puffin-cli/tests/pip_compile.rs

@@ -444,6 +444,59 @@ optional-dependencies.foo = [
     Ok(())
 }
 
+/// Request an extra with a name that does not conform to the specification.
+#[test]
+fn invalid_extra_name() -> Result<()> {
+    let temp_dir = assert_fs::TempDir::new()?;
+    let cache_dir = assert_fs::TempDir::new()?;
+    let venv = temp_dir.child(".venv");
+
+    Command::new(get_cargo_bin(BIN_NAME))
+        .arg("venv")
+        .arg(venv.as_os_str())
+        .arg("--cache-dir")
+        .arg(cache_dir.path())
+        .current_dir(&temp_dir)
+        .assert()
+        .success();
+    venv.assert(predicates::path::is_dir());
+
+    let pyproject_toml = temp_dir.child("pyproject.toml");
+    pyproject_toml.touch()?;
+    pyproject_toml.write_str(
+        r#"[build-system]
+requires = ["setuptools", "wheel"]
+
+[project]
+name = "project"
+dependencies = []
+optional-dependencies.foo = [
+    "django==5.0b1",
+]
+"#,
+    )?;
+
+    insta::with_settings!({
+        filters => vec![
+            (r"\d+(ms|s)", "[TIME]"),
+            (r"#    .* pip-compile", "#    [BIN_PATH] pip-compile"),
+            (r"--cache-dir .*", "--cache-dir [CACHE_DIR]"),
+        ]
+    }, {
+        assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+            .arg("pip-compile")
+            .arg("pyproject.toml")
+            .arg("--extra")
+            .arg("invalid name!")
+            .arg("--cache-dir")
+            .arg(cache_dir.path())
+            .env("VIRTUAL_ENV", venv.as_os_str())
+            .current_dir(&temp_dir));
+    });
+
+    Ok(())
+}
+
 /// Resolve a specific Flask wheel via a URL dependency.
 #[test]
 fn compile_wheel_url_dependency() -> Result<()> {

crates/puffin-cli/tests/snapshots/pip_compile__invalid_extra_name.snap

@@ -0,0 +1,23 @@
+---
+source: crates/puffin-cli/tests/pip_compile.rs
+info:
+  program: puffin
+  args:
+    - pip-compile
+    - pyproject.toml
+    - "--extra"
+    - invalid name!
+    - "--cache-dir"
+    - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpXIuamZ
+  env:
+    VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp0LGEEX/.venv
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: invalid value 'invalid name!' for '--extra <EXTRA>': Extra names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters
+
+For more information, try '--help'.
+

crates/puffin-package/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 pep440_rs = { path = "../pep440-rs", features = ["serde"] }
 pep508_rs = { path = "../pep508-rs", features = ["serde"] }
 
+anyhow = { workspace = true }
 fs-err = { workspace = true }
 mailparse = { workspace = true }
 memchr = { workspace = true }
@@ -19,7 +20,6 @@ tracing.workspace = true
 unscanny = { workspace = true }
 
 [dev-dependencies]
-anyhow = { version = "1.0.75" }
 indoc = { version = "2.0.4" }
 insta = { version = "1.33.0" }
 serde_json = { version = "1.0.107" }

crates/puffin-package/src/extra_name.rs

@@ -1,6 +1,8 @@
 use std::fmt;
 use std::fmt::{Display, Formatter};
+use std::str::FromStr;
 
+use anyhow::{anyhow, Error, Result};
 use once_cell::sync::Lazy;
 use regex::Regex;
 
@@ -14,19 +16,36 @@ impl Display for ExtraName {
 }
 
 static NAME_NORMALIZE: Lazy<Regex> = Lazy::new(|| Regex::new(r"[-_.]+").unwrap());
+static NAME_VALIDATE: Lazy<Regex> =
+    Lazy::new(|| Regex::new(r"(?i)^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$").unwrap());
 
+/// An extra dependency group name.
+///
+/// See:
+/// - <https://peps.python.org/pep-0685/#specification/>
+/// - <https://packaging.python.org/en/latest/specifications/name-normalization/>
 impl ExtraName {
-    /// Collapses any run of the characters `-`, `_` and `.` down to a single `-`.
+    /// Create a normalized extra name without validation.
-    /// Ex) "---", ".", and "__" all get converted to just "."
     ///
-    /// See: <https://peps.python.org/pep-0685/#specification/>
+    /// Converts the name to lowercase and collapses any run of the characters `-`, `_` and `.`
-    ///      <https://packaging.python.org/en/latest/specifications/name-normalization/>
+    /// down to a single `-`, e.g., `---`, `.`, and `__` all get converted to just `-`.
     pub fn normalize(name: impl AsRef<str>) -> Self {
         // TODO(charlie): Avoid allocating in the common case (when no normalization is required).
         let mut normalized = NAME_NORMALIZE.replace_all(name.as_ref(), "-").to_string();
         normalized.make_ascii_lowercase();
         Self(normalized)
     }
+
+    /// Create a validated, normalized extra name.
+    pub fn validate(name: impl AsRef<str>) -> Result<Self> {
+        if NAME_VALIDATE.is_match(name.as_ref()) {
+            Ok(Self::normalize(name))
+        } else {
+            Err(anyhow!(
+                "Extra names must start and end with a letter or digit and may only contain -, _, ., and alphanumeric characters"
+            ))
+        }
+    }
 }
 
 impl AsRef<str> for ExtraName {
@@ -35,9 +54,11 @@ impl AsRef<str> for ExtraName {
     }
 }
 
-impl From<&str> for ExtraName {
+impl FromStr for ExtraName {
-    fn from(name: &str) -> Self {
+    type Err = Error;
-        Self::normalize(name)
+
+    fn from_str(name: &str) -> Result<Self> {
+        Self::validate(name)
     }
 }
 
@@ -76,4 +97,27 @@ mod tests {
             "friendly-bard"
         );
     }
+
+    #[test]
+    fn validate() {
+        // Unchanged
+        assert_eq!(
+            ExtraName::validate("friendly-bard").unwrap().as_ref(),
+            "friendly-bard"
+        );
+        assert_eq!(ExtraName::validate("1okay").unwrap().as_ref(), "1okay");
+        assert_eq!(ExtraName::validate("okay2").unwrap().as_ref(), "okay2");
+        // Normalizes
+        assert_eq!(
+            ExtraName::validate("Friendly-Bard").unwrap().as_ref(),
+            "friendly-bard"
+        );
+        // Failures...
+        assert!(ExtraName::validate(" starts-with-space").is_err());
+        assert!(ExtraName::validate("-starts-with-dash").is_err());
+        assert!(ExtraName::validate("ends-with-dash-").is_err());
+        assert!(ExtraName::validate("ends-with-space ").is_err());
+        assert!(ExtraName::validate("includes!invalid-char").is_err());
+        assert!(ExtraName::validate("space in middle").is_err());
+    }
 }