From 5c824b508a936d9e05f0a863160800e3ffeebcc7 Mon Sep 17 00:00:00 2001 From: Christopher Tee Date: Sat, 21 Jun 2025 11:44:45 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=85=20Add=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/uv/tests/it/edit.rs | 225 +++++++++++++++++++++++++++++++++++++ 1 file changed, 225 insertions(+) diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index d117b1e8c..ed04baf15 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -10702,6 +10702,231 @@ fn remove_all_with_comments() -> Result<()> { Ok(()) } +#[test] +fn remove_preserves_comment_on_first_dep() -> 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" + requires-python = ">=3.12" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "sniffio==1.3.1", # this is sniffio + "urllib3==2.2.1", # this is urllib3 + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("anyio"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Prepared 2 packages in [TIME] + Installed 2 packages in [TIME] + + sniffio==1.3.1 + + urllib3==2.2.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" + dependencies = [ + # top comment + "sniffio==1.3.1", # this is sniffio + "urllib3==2.2.1", # this is urllib3 + # bottom comment + ] + "### + ); + }); + Ok(()) +} + +#[test] +fn remove_preserves_comment_on_middle_deps() -> 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" + requires-python = ">=3.12" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "sniffio==1.3.1", # this is sniffio + "urllib3==2.2.1", # this is urllib3 + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("sniffio"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 5 packages in [TIME] + Prepared 4 packages in [TIME] + Installed 4 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + sniffio==1.3.1 + + urllib3==2.2.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" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "urllib3==2.2.1", # this is urllib3 + # bottom comment + ] + "### + ); + }); + Ok(()) +} + +#[test] +fn remove_preserves_comment_on_final_dep() -> 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" + requires-python = ">=3.12" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "sniffio==1.3.1", # this is sniffio + "urllib3==2.2.1", # this is urllib3 + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("urllib3"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + sniffio==1.3.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" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "sniffio==1.3.1", # this is sniffio + # bottom comment + ] + "### + ); + }); + Ok(()) +} + +/// `toml_edit` treats the trailing comments differently +#[test] +fn remove_preserves_comment_on_final_dep_without_trailing_comma() -> 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" + requires-python = ">=3.12" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "sniffio==1.3.1", # this is sniffio + "urllib3==2.2.1" # this is urllib3 + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("urllib3"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==3.7.0 + + idna==3.6 + + sniffio==1.3.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" + dependencies = [ + # top comment + "anyio==3.7.0", # this is anyio + "sniffio==1.3.1", # this is sniffio + # bottom comment + ] + "### + ); + }); + Ok(()) +} + /// If an index is repeated by the CLI and an environment variable, the CLI value should take /// precedence. /// From 3589d20e7ea7b846cd2c5c5035d6c8ebc3a1e010 Mon Sep 17 00:00:00 2001 From: Christopher Tee Date: Sat, 21 Jun 2025 11:45:03 -0400 Subject: [PATCH 2/2] Handle comments correctly --- crates/uv-workspace/src/pyproject_mut.rs | 71 +++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index 73e3833ae..89af4e6bc 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -1455,6 +1455,7 @@ fn remove_dependency(name: &PackageName, deps: &mut Array) -> Vec { .into_iter() .rev() // Reverse to preserve indices as we remove them. .filter_map(|(i, _)| { + prepare_comments_for_removal(deps, i); deps.remove(i) .as_str() .and_then(|req| Requirement::from_str(req).ok()) @@ -1464,7 +1465,6 @@ fn remove_dependency(name: &PackageName, deps: &mut Array) -> Vec { if !removed.is_empty() { reformat_array_multiline(deps); } - removed } @@ -1624,6 +1624,75 @@ fn reformat_array_multiline(deps: &mut Array) { deps.set_trailing_comma(true); } +/// Ensure that only inline comments for the i-th dependency are removed, +/// while preserving the other comments. +fn prepare_comments_for_removal(deps: &mut Array, i: usize) { + /// Remove comments on the same line as the dependency to be removed, + /// while preserving the comments in lines following it. + fn remove_inline_comment(comment: &str) -> &str { + match comment.split_once('\n') { + // `deps_to_remove` and the next dependency are on the same line. + None => "", + // `deps_to_remove` has an inline comment and no comments beneath it. + // Without this branch, the resulting dependency TOML will have an extra layer of indentation. + Some((_inline, trailing)) if trailing.trim().is_empty() => "", + // `deps_to_remove` has an inline comment and multiple lines of comments beneath it. + Some((_inline, trailing)) => trailing, + } + } + + let n_deps = deps.len(); + + let dep_to_remove = deps + .get_mut(i) + .unwrap_or_else(|| panic!("Index {i} into dependency array should be valid")); + + // Get comments that directly precede `dep_to_remove`. + let prefix = dep_to_remove + .decor() + .prefix() + .and_then(|s| s.as_str()) + .map(ToOwned::to_owned) + .expect("The prefix should not be `None`, even when there are no characters") + .clone(); + + // Replace inline comment associated with `dep_to_remove` with preceding comments. + if i == n_deps - 1 { + let trailing_comment = match dep_to_remove + .decor() + .suffix() + .and_then(|s| s.as_str()) + .filter(|s| !s.trim().is_empty()) + .map(ToOwned::to_owned) + { + // The trailing comment is part of the Array's `.trailing` field. + None => deps.trailing().as_str().unwrap().to_owned(), + // The dependency to be removed has no trailing comma and an inline comment under `.suffix`. + Some(suffix) => suffix, + }; + let updated_comment = format!("{}{}", prefix, remove_inline_comment(&trailing_comment)); + deps.set_trailing(updated_comment); + } else { + let following_decor = deps + .get_mut(i + 1) + .unwrap_or_else(|| { + panic!( + "Encountered invalid index {} when updating the suffix", + i + 1 + ) + }) + .decor_mut(); + + let trailing_comment = following_decor + .prefix() + .and_then(|s| s.as_str()) + .map(ToOwned::to_owned) + .expect("The prefix should not be `None`, even when there are no characters"); + let updated_comment = format!("{}{}", prefix, remove_inline_comment(&trailing_comment)); + following_decor.set_prefix(updated_comment); + } +} + /// 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