diff --git a/crates/uv-workspace/src/pyproject_mut.rs b/crates/uv-workspace/src/pyproject_mut.rs index ba489818b..86af7b5bc 100644 --- a/crates/uv-workspace/src/pyproject_mut.rs +++ b/crates/uv-workspace/src/pyproject_mut.rs @@ -1459,6 +1459,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()) @@ -1468,7 +1469,6 @@ fn remove_dependency(name: &PackageName, deps: &mut Array) -> Vec { if !removed.is_empty() { reformat_array_multiline(deps); } - removed } @@ -1628,6 +1628,89 @@ 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. +/// +/// toml-edit attaches comments after the comma to the next item in the array, even if they are +/// end-of-line 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) + .unwrap_or_else(|| { + if cfg!(debug_assertions) { + panic!("The prefix should not be `None`, even when there are no characters"); + } else { + String::new() + } + }); + + // 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) => format!("{}{}", suffix, deps.trailing().as_str().unwrap().to_owned()), + }; + 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) + .unwrap_or_else(|| { + if cfg!(debug_assertions) { + panic!("The prefix should not be `None`, even when there are no characters"); + } else { + String::new() + } + }); + 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 diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index f051bf3c3..77f6628d2 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -11755,6 +11755,404 @@ fn multiple_index_cli() -> 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").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + "); + + 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").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 5 packages in [TIME] + "); + + 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").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "); + + 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").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "); + + 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(()) +} + +#[test] +fn remove_handle_oddly_placed_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 inside sniffio (end of line) + # this is inside sniffio (own line) + , # this is outside sniffio (end of line) + # this is outside sniffio (own line) + "urllib3==2.2.1" # this is urllib3 + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("sniffio").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 5 packages in [TIME] + "); + + 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 + # this is outside sniffio (own line) # this is urllib3 + # bottom comment + "urllib3==2.2.1", + ] + "# + ); + }); + Ok(()) +} + +#[test] +fn remove_preserves_comments_around_single_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 + "sniffio==1.3.1", # this is sniffio + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("sniffio").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + "); + + 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 + # bottom comment + ] + "# + ); + }); + Ok(()) +} + +#[test] +fn remove_preserves_sole_top_comment() -> 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 + "urllib3==2.2.1" # this is urllib3 + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("urllib3").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + "); + + 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 + ] + "# + ); + }); + Ok(()) +} + +#[test] +fn remove_preserves_sole_bottom_comment() -> 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 = [ + "urllib3==2.2.1" # this is urllib3 + , + # bottom comment + ] + "#})?; + + uv_snapshot!(context.filters(), context.remove().arg("urllib3").arg("--no-sync"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + "); + + 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 = [ + # bottom comment + ] + "# + ); + }); + Ok(()) +} + /// If an index is repeated by the CLI and an environment variable, the CLI value should take /// precedence. ///