diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index 3a76620d4..63863e6a0 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -1,4 +1,3 @@ -use std::cmp::Ordering; use std::path::Path; use std::str::FromStr; use std::{fmt, mem}; @@ -1019,96 +1018,76 @@ pub fn add_dependency( Unsorted, } - /// Compare two [`Value`] requirements case-insensitively. - fn case_insensitive(a: &Value, b: &Value) -> Ordering { - a.as_str() - .map(str::to_lowercase) - .as_deref() - .map(split_specifiers) - .cmp( - &b.as_str() - .map(str::to_lowercase) - .as_deref() - .map(split_specifiers), - ) + fn is_sorted(items: I) -> bool + where + I: IntoIterator, + T: PartialOrd + Copy, + { + items.into_iter().tuple_windows().all(|(a, b)| a <= b) } - /// Naively compare two [`Value`] requirements case-insensitively. - fn case_insensitive_naive(a: &Value, b: &Value) -> Ordering { - a.as_str() - .map(str::to_lowercase) - .cmp(&b.as_str().map(str::to_lowercase)) - } - - /// Compare two [`Value`] requirements case-sensitively. - fn case_sensitive(a: &Value, b: &Value) -> Ordering { - a.as_str() - .map(split_specifiers) - .cmp(&b.as_str().map(split_specifiers)) - } - - /// Naively compare two [`Value`] requirements case-sensitively. - fn case_sensitive_naive(a: &Value, b: &Value) -> Ordering { - a.as_str().cmp(&b.as_str()) - } + // `deps` are either requirements (strings) or include groups (inline tables). + // Here we pull out just the requirements for determining the sort. + let reqs: Vec<_> = deps.iter().filter_map(Value::as_str).collect(); + let reqs_lowercase: Vec<_> = reqs.iter().copied().map(str::to_lowercase).collect(); // Determine if the dependency list is sorted prior to // adding the new dependency; the new dependency list // will be sorted only when the original list is sorted // so that user's custom dependency ordering is preserved. // - // Additionally, if the table is invalid (i.e. contains non-string values) - // we still treat it as unsorted for the sake of simplicity. + // Any items which aren't strings are ignored, e.g. + // `{ include-group = "..." }` in dependency-groups. // // We account for both case-sensitive and case-insensitive sorting. - let sort = deps - .iter() - .all(Value::is_str) - .then(|| { - if deps.iter().tuple_windows().all(|(a, b)| { - matches!(case_insensitive(a, b), Ordering::Less | Ordering::Equal) - }) { - Some(Sort::CaseInsensitive) - } else if deps.iter().tuple_windows().all(|(a, b)| { - matches!(case_sensitive(a, b), Ordering::Less | Ordering::Equal) - }) { - Some(Sort::CaseSensitive) - } else if deps.iter().tuple_windows().all(|(a, b)| { - matches!( - case_insensitive_naive(a, b), - Ordering::Less | Ordering::Equal - ) - }) { - Some(Sort::CaseInsensitiveNaive) - } else if deps.iter().tuple_windows().all(|(a, b)| { - matches!(case_sensitive_naive(a, b), Ordering::Less | Ordering::Equal) - }) { - Some(Sort::CaseSensitiveNaive) - } else { - None - } - }) - .flatten() - .unwrap_or(Sort::Unsorted); + let sort = if is_sorted( + reqs_lowercase + .iter() + .map(String::as_str) + .map(split_specifiers), + ) { + Sort::CaseInsensitive + } else if is_sorted(reqs.iter().copied().map(split_specifiers)) { + Sort::CaseSensitive + } else if is_sorted(reqs_lowercase.iter().map(String::as_str)) { + Sort::CaseInsensitiveNaive + } else if is_sorted(reqs) { + Sort::CaseSensitiveNaive + } else { + Sort::Unsorted + }; let req_string = req.to_string(); let index = match sort { - Sort::CaseInsensitive => deps.iter().position(|d| { - case_insensitive(d, &Value::from(req_string.as_str())) == Ordering::Greater + Sort::CaseInsensitive => deps.iter().position(|dep| { + dep.as_str().is_some_and(|dep| { + split_specifiers(&dep.to_lowercase()) + > split_specifiers(&req_string.to_lowercase()) + }) }), - Sort::CaseInsensitiveNaive => deps.iter().position(|d| { - case_insensitive_naive(d, &Value::from(req_string.as_str())) - == Ordering::Greater + Sort::CaseInsensitiveNaive => deps.iter().position(|dep| { + dep.as_str() + .is_some_and(|dep| dep.to_lowercase() > req_string.to_lowercase()) }), - Sort::CaseSensitive => deps.iter().position(|d| { - case_sensitive(d, &Value::from(req_string.as_str())) == Ordering::Greater - }), - Sort::CaseSensitiveNaive => deps.iter().position(|d| { - case_sensitive_naive(d, &Value::from(req_string.as_str())) == Ordering::Greater + Sort::CaseSensitive => deps.iter().position(|dep| { + dep.as_str() + .is_some_and(|dep| split_specifiers(dep) > split_specifiers(&req_string)) }), + Sort::CaseSensitiveNaive => deps + .iter() + .position(|dep| dep.as_str().is_some_and(|dep| *dep > *req_string)), Sort::Unsorted => None, }; - let index = index.unwrap_or(deps.len()); + let index = index.unwrap_or_else(|| { + // The dependency should be added to the end, ignoring any + // `include-group` items. This preserves the order for users who + // keep their `include-groups` at the bottom. + deps.iter() + .enumerate() + .filter_map(|(i, dep)| if dep.is_str() { Some(i + 1) } else { None }) + .last() + .unwrap_or(deps.len()) + }); let mut value = Value::from(req_string.as_str()); diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index a2dc91df4..b5894fa59 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -7514,6 +7514,201 @@ fn sorted_dependencies_name_specifiers() -> Result<()> { Ok(()) } +#[test] +fn sorted_dependencies_with_include_group() -> Result<()> { + let context = TestContext::new("3.12").with_filtered_counts(); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + + [dependency-groups] + dev = [ + { include-group = "coverage" }, + "pytest-mock>=3.14", + "pytest>=8.1.1", + ] + coverage = [ + "coverage>=7.4.4", + ] + "#})?; + + uv_snapshot!(context.filters(), context.add().args(["--dev", "pytest-randomly"]), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] packages in [TIME] + + coverage==7.4.4 + + iniconfig==2.0.0 + + packaging==24.0 + + pluggy==1.4.0 + + pytest==8.1.1 + + pytest-mock==3.14.0 + + pytest-randomly==3.15.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" + requires-python = ">=3.12" + + [dependency-groups] + dev = [ + { include-group = "coverage" }, + "pytest-mock>=3.14", + "pytest-randomly>=3.15.0", + "pytest>=8.1.1", + ] + coverage = [ + "coverage>=7.4.4", + ] + "# + ); + }); + Ok(()) +} + +#[test] +fn sorted_dependencies_new_dependency_after_include_group() -> Result<()> { + let context = TestContext::new("3.12").with_filtered_counts(); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + + [dependency-groups] + dev = [ + { include-group = "coverage" }, + ] + coverage = [ + "coverage>=7.4.4", + ] + "#})?; + + uv_snapshot!(context.filters(), context.add().args(["--dev", "pytest"]), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] packages in [TIME] + + coverage==7.4.4 + + iniconfig==2.0.0 + + packaging==24.0 + + pluggy==1.4.0 + + pytest==8.1.1 + "); + + 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" + requires-python = ">=3.12" + + [dependency-groups] + dev = [ + { include-group = "coverage" }, + "pytest>=8.1.1", + ] + coverage = [ + "coverage>=7.4.4", + ] + "# + ); + }); + Ok(()) +} + +#[test] +fn sorted_dependencies_include_group_kept_at_bottom() -> Result<()> { + let context = TestContext::new("3.12").with_filtered_counts(); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + + [dependency-groups] + dev = [ + "pytest>=8.1.1", + { include-group = "coverage" }, + ] + coverage = [ + "coverage>=7.4.4", + ] + "#})?; + + uv_snapshot!(context.filters(), context.add().args(["--dev", "pytest-randomly"]), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved [N] packages in [TIME] + Prepared [N] packages in [TIME] + Installed [N] packages in [TIME] + + coverage==7.4.4 + + iniconfig==2.0.0 + + packaging==24.0 + + pluggy==1.4.0 + + pytest==8.1.1 + + pytest-randomly==3.15.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" + requires-python = ">=3.12" + + [dependency-groups] + dev = [ + "pytest>=8.1.1", + "pytest-randomly>=3.15.0", + { include-group = "coverage" }, + ] + coverage = [ + "coverage>=7.4.4", + ] + "# + ); + }); + Ok(()) +} + /// Ensure that the custom ordering of the dependencies is preserved /// after adding a package. #[test]