From b0e7476008dd503d94512dae6d591022a7ab5932 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Wed, 13 Nov 2024 15:47:57 -0500
Subject: [PATCH] Sort by name, then specifiers in `uv add` (#9097)

## Summary

This PR ensures that `pylint>=3.2.6` followed by
`pylint-module-boundaries>=1.3.1` is considered sorted, despite the fact
that `>` is later in the alphabetic than `-`. By purely comparing
strings, they would _not_ be sorted; but by considering the name, then
the specifiers, they are.

Closes https://github.com/astral-sh/uv/issues/9076.
---
 crates/uv-workspace/src/pyproject_mut.rs | 50 ++++++++++++++++++---
 crates/uv/tests/it/edit.rs               | 57 ++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs
index e01ecf3bae4e..47bf5ec1142d 100644
--- a/crates/uv-workspace/src/pyproject_mut.rs
+++ b/crates/uv-workspace/src/pyproject_mut.rs
@@ -920,14 +920,19 @@ pub fn add_dependency(
                 .all(Value::is_str)
                 .then(|| {
                     if deps.iter().tuple_windows().all(|(a, b)| {
-                        a.as_str().map(str::to_lowercase) <= b.as_str().map(str::to_lowercase)
+                        a.as_str()
+                            .map(str::to_lowercase)
+                            .as_deref()
+                            .map(split_specifiers)
+                            <= b.as_str()
+                                .map(str::to_lowercase)
+                                .as_deref()
+                                .map(split_specifiers)
                     }) {
                         Some(Sort::CaseInsensitive)
-                    } else if deps
-                        .iter()
-                        .tuple_windows()
-                        .all(|(a, b)| a.as_str() <= b.as_str())
-                    {
+                    } else if deps.iter().tuple_windows().all(|(a, b)| {
+                        a.as_str().map(split_specifiers) <= b.as_str().map(split_specifiers)
+                    }) {
                         Some(Sort::CaseSensitive)
                     } else {
                         None
@@ -1197,3 +1202,36 @@ fn reformat_array_multiline(deps: &mut Array) {
     });
     deps.set_trailing_comma(true);
 }
+
+/// Split a requirement into the package name and its dependency specifiers.
+///
+/// E.g., given `flask>=1.0`, this function returns `("flask", ">=1.0")`. But given
+/// `Flask>=1.0`, this function returns `("Flask", ">=1.0")`.
+///
+/// Extras are retained, such that `flask[dotenv]>=1.0` returns `("flask[dotenv]", ">=1.0")`.
+fn split_specifiers(req: &str) -> (&str, &str) {
+    let (name, specifiers) = req
+        .find(['>', '<', '=', '~', '!', '@'])
+        .map_or((req, ""), |pos| {
+            let (name, specifiers) = req.split_at(pos);
+            (name, specifiers)
+        });
+    (name.trim(), specifiers.trim())
+}
+
+#[cfg(test)]
+mod test {
+    use super::split_specifiers;
+
+    #[test]
+    fn split() {
+        assert_eq!(split_specifiers("flask>=1.0"), ("flask", ">=1.0"));
+        assert_eq!(split_specifiers("Flask>=1.0"), ("Flask", ">=1.0"));
+        assert_eq!(
+            split_specifiers("flask[dotenv]>=1.0"),
+            ("flask[dotenv]", ">=1.0")
+        );
+        assert_eq!(split_specifiers("flask[dotenv]",), ("flask[dotenv]", ""));
+        assert_eq!(split_specifiers("flask @ https://files.pythonhosted.org/packages/af/47/93213ee66ef8fae3b93b3e29206f6b251e65c97bd91d8e1c5596ef15af0a/flask-3.1.0-py3-none-any.whl"), ("flask", "@ https://files.pythonhosted.org/packages/af/47/93213ee66ef8fae3b93b3e29206f6b251e65c97bd91d8e1c5596ef15af0a/flask-3.1.0-py3-none-any.whl"));
+    }
+}
diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs
index 0a3b43baf311..eceeae20b441 100644
--- a/crates/uv/tests/it/edit.rs
+++ b/crates/uv/tests/it/edit.rs
@@ -5842,6 +5842,63 @@ fn case_sensitive_sorted_dependencies() -> Result<()> {
     Ok(())
 }
 
+/// Ensure that sorting is based on the name, rather than the combined name-and-specifiers.
+#[test]
+fn sorted_dependencies_name_specifiers() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    let pyproject_toml = context.temp_dir.child("pyproject.toml");
+    pyproject_toml.write_str(indoc! {r#"
+    [project]
+    name = "project"
+    version = "0.1.0"
+    description = "Add your description here"
+    requires-python = ">=3.12"
+    dependencies = [
+        "typing>=3",
+        "typing-extensions>=4",
+    ]
+    "#})?;
+
+    uv_snapshot!(context.filters(), context.add().args(["anyio"]), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 6 packages in [TIME]
+    Prepared 5 packages in [TIME]
+    Installed 5 packages in [TIME]
+     + anyio==4.3.0
+     + idna==3.6
+     + sniffio==1.3.1
+     + typing==3.10.0.0
+     + typing-extensions==4.10.0
+    "###);
+
+    let pyproject_toml = context.read("pyproject.toml");
+
+    insta::with_settings!({
+        filters => context.filters(),
+    }, {
+        assert_snapshot!(
+            pyproject_toml, @r###"
+        [project]
+        name = "project"
+        version = "0.1.0"
+        description = "Add your description here"
+        requires-python = ">=3.12"
+        dependencies = [
+            "anyio>=4.3.0",
+            "typing>=3",
+            "typing-extensions>=4",
+        ]
+        "###
+        );
+    });
+    Ok(())
+}
+
 /// Ensure that the custom ordering of the dependencies is preserved
 /// after adding a package.
 #[test]