Skip to content

Commit 6d628da

Browse files
zaniebclaude
andauthored
Preserve end-of-line comments on previous entries when removing dependencies (#18557)
When a dependency is removed, `toml_edit` stores any trailing comments in the prefix of the next item. Our implementation did not account for this, causing trailing comments on a dependency to be dropped when the subsequent item is removed. For example, when removing `requests` from: ```toml dependencies = [ "numpy", # essential comment "requests", ] ``` With this fix, the comment is preserved. Closes #18555 --------- Co-authored-by: Claude <[email protected]>
1 parent 46c9bac commit 6d628da

File tree

2 files changed

+341
-3
lines changed

2 files changed

+341
-3
lines changed

crates/uv-workspace/src/pyproject_mut.rs

Lines changed: 232 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,11 +1545,51 @@ fn update_requirement(old: &mut Requirement, new: &Requirement, has_source: bool
15451545

15461546
/// Removes all occurrences of dependencies with the given name from the given `deps` array.
15471547
fn remove_dependency(name: &PackageName, deps: &mut Array) -> Vec<Requirement> {
1548-
// Remove matching dependencies.
1548+
// Remove in reverse to preserve indices. Before each removal, transfer the item's
1549+
// prefix (which may contain end-of-line comments belonging to the previous line) to
1550+
// the next item or array trailing so comments are not lost.
1551+
//
1552+
// For example, in:
1553+
// ```toml
1554+
// dependencies = [
1555+
// "numpy>=2.4.3", # essential comment
1556+
// "requests>=2.32.5",
1557+
// ]
1558+
// ```
1559+
//
1560+
// The comment `# essential comment` is stored by `toml_edit` in the prefix of
1561+
// `requests`. When `requests` is removed, we transfer it so it remains on the
1562+
// `numpy` line.
15491563
let removed = find_dependencies(name, None, deps)
15501564
.into_iter()
1551-
.rev() // Reverse to preserve indices as we remove them.
1565+
.rev()
15521566
.filter_map(|(i, _)| {
1567+
if let Some(prefix) = deps
1568+
.get(i)
1569+
.and_then(|item| item.decor().prefix().and_then(|s| s.as_str()))
1570+
.filter(|s| !s.is_empty())
1571+
{
1572+
let prefix = prefix.to_string();
1573+
if let Some(next) = deps.get(i + 1)
1574+
&& let Some(existing) = next.decor().prefix().and_then(|s| s.as_str())
1575+
{
1576+
// Transfer removed item's prefix to the next item's prefix.
1577+
let existing = existing.to_string();
1578+
deps.get_mut(i + 1)
1579+
.unwrap()
1580+
.decor_mut()
1581+
.set_prefix(format!("{prefix}{existing}"));
1582+
} else if let Some(next) = deps.get_mut(i + 1) {
1583+
// Next item exists but has no prefix; use ours directly.
1584+
next.decor_mut().set_prefix(&prefix);
1585+
} else if let Some(existing) = deps.trailing().as_str() {
1586+
// No next item; move comments to the array trailing.
1587+
deps.set_trailing(format!("{prefix}{existing}"));
1588+
} else {
1589+
deps.set_trailing(&prefix);
1590+
}
1591+
}
1592+
15531593
deps.remove(i)
15541594
.as_str()
15551595
.and_then(|req| Requirement::from_str(req).ok())
@@ -1750,9 +1790,11 @@ fn split_specifiers(req: &str) -> (&str, &str) {
17501790

17511791
#[cfg(test)]
17521792
mod test {
1753-
use super::{AddBoundsKind, reformat_array_multiline, split_specifiers};
1793+
use super::{AddBoundsKind, reformat_array_multiline, remove_dependency, split_specifiers};
1794+
use insta::assert_snapshot;
17541795
use std::str::FromStr;
17551796
use toml_edit::DocumentMut;
1797+
use uv_normalize::PackageName;
17561798
use uv_pep440::Version;
17571799

17581800
#[test]
@@ -1927,4 +1969,191 @@ dependencies = [
19271969
assert_eq!(actual, expected, "{version}");
19281970
}
19291971
}
1972+
1973+
#[test]
1974+
fn remove_preserves_end_of_line_comment_on_previous_item() {
1975+
let toml = r#"
1976+
[project]
1977+
dependencies = [
1978+
"numpy>=2.4.3", # this comment is clearly essential
1979+
"requests>=2.32.5",
1980+
]
1981+
"#;
1982+
let mut doc: DocumentMut = toml.parse().unwrap();
1983+
let deps = doc["project"]["dependencies"]
1984+
.as_array_mut()
1985+
.expect("dependencies array");
1986+
1987+
let name = PackageName::from_str("requests").unwrap();
1988+
remove_dependency(&name, deps);
1989+
1990+
assert_snapshot!(
1991+
doc.to_string(),
1992+
@r#"
1993+
[project]
1994+
dependencies = [
1995+
"numpy>=2.4.3", # this comment is clearly essential
1996+
]
1997+
"#
1998+
);
1999+
}
2000+
2001+
#[test]
2002+
fn remove_preserves_end_of_line_comment_on_previous_item_middle() {
2003+
let toml = r#"
2004+
[project]
2005+
dependencies = [
2006+
"numpy>=2.4.3", # numpy comment
2007+
"requests>=2.32.5",
2008+
"flask>=3.0.0",
2009+
]
2010+
"#;
2011+
let mut doc: DocumentMut = toml.parse().unwrap();
2012+
let deps = doc["project"]["dependencies"]
2013+
.as_array_mut()
2014+
.expect("dependencies array");
2015+
2016+
let name = PackageName::from_str("requests").unwrap();
2017+
remove_dependency(&name, deps);
2018+
2019+
assert_snapshot!(
2020+
doc.to_string(),
2021+
@r#"
2022+
[project]
2023+
dependencies = [
2024+
"numpy>=2.4.3", # numpy comment
2025+
"flask>=3.0.0",
2026+
]
2027+
"#
2028+
);
2029+
}
2030+
2031+
#[test]
2032+
fn remove_preserves_own_line_comment_above_removed_item() {
2033+
let toml = r#"
2034+
[project]
2035+
dependencies = [
2036+
"numpy>=2.4.3",
2037+
# This is a comment about requests
2038+
"requests>=2.32.5",
2039+
]
2040+
"#;
2041+
let mut doc: DocumentMut = toml.parse().unwrap();
2042+
let deps = doc["project"]["dependencies"]
2043+
.as_array_mut()
2044+
.expect("dependencies array");
2045+
2046+
let name = PackageName::from_str("requests").unwrap();
2047+
remove_dependency(&name, deps);
2048+
2049+
assert_snapshot!(
2050+
doc.to_string(),
2051+
@r#"
2052+
[project]
2053+
dependencies = [
2054+
"numpy>=2.4.3",
2055+
# This is a comment about requests
2056+
]
2057+
"#
2058+
);
2059+
}
2060+
2061+
#[test]
2062+
fn remove_item_with_trailing_comment_last() {
2063+
// When the removed item itself has an end-of-line comment and is the last item,
2064+
// toml_edit stores the comment in the array trailing. The comment is preserved
2065+
// (as an own-line comment in the trailing section) but moves position since it
2066+
// can no longer be on the removed item's line.
2067+
let toml = r#"
2068+
[project]
2069+
dependencies = [
2070+
"requests>=2.32.5",
2071+
"numpy>=2.4.3", # comment on numpy
2072+
]
2073+
"#;
2074+
let mut doc: DocumentMut = toml.parse().unwrap();
2075+
let deps = doc["project"]["dependencies"]
2076+
.as_array_mut()
2077+
.expect("dependencies array");
2078+
2079+
let name = PackageName::from_str("numpy").unwrap();
2080+
remove_dependency(&name, deps);
2081+
2082+
assert_snapshot!(
2083+
doc.to_string(),
2084+
@r#"
2085+
[project]
2086+
dependencies = [
2087+
"requests>=2.32.5",
2088+
# comment on numpy
2089+
]
2090+
"#
2091+
);
2092+
}
2093+
2094+
#[test]
2095+
fn remove_item_with_trailing_comment_middle() {
2096+
// When the removed item has an end-of-line comment and is in the middle,
2097+
// toml_edit stores the comment in the next item's prefix. After removal,
2098+
// reformat_array_multiline repositions it as an own-line comment.
2099+
let toml = r#"
2100+
[project]
2101+
dependencies = [
2102+
"requests>=2.32.5",
2103+
"numpy>=2.4.3", # comment on numpy
2104+
"flask>=3.0.0",
2105+
]
2106+
"#;
2107+
let mut doc: DocumentMut = toml.parse().unwrap();
2108+
let deps = doc["project"]["dependencies"]
2109+
.as_array_mut()
2110+
.expect("dependencies array");
2111+
2112+
let name = PackageName::from_str("numpy").unwrap();
2113+
remove_dependency(&name, deps);
2114+
2115+
assert_snapshot!(
2116+
doc.to_string(),
2117+
@r#"
2118+
[project]
2119+
dependencies = [
2120+
"requests>=2.32.5",
2121+
# comment on numpy
2122+
"flask>=3.0.0",
2123+
]
2124+
"#
2125+
);
2126+
}
2127+
2128+
#[test]
2129+
fn remove_multiple_adjacent_matches_preserves_comment_order() {
2130+
let toml = r#"
2131+
[project]
2132+
dependencies = [
2133+
"iniconfig>=2.0.0", # comment on iniconfig
2134+
"typing-extensions>=4.0.0 ; python_version < '3.11'", # comment on first typing-extensions
2135+
"typing-extensions>=4.0.0 ; python_version >= '3.11'",
2136+
"sniffio>=1.3.0",
2137+
]
2138+
"#;
2139+
let mut doc: DocumentMut = toml.parse().unwrap();
2140+
let deps = doc["project"]["dependencies"]
2141+
.as_array_mut()
2142+
.expect("dependencies array");
2143+
2144+
let name = PackageName::from_str("typing-extensions").unwrap();
2145+
remove_dependency(&name, deps);
2146+
2147+
assert_snapshot!(
2148+
doc.to_string(),
2149+
@r#"
2150+
[project]
2151+
dependencies = [
2152+
"iniconfig>=2.0.0", # comment on iniconfig
2153+
# comment on first typing-extensions
2154+
"sniffio>=1.3.0",
2155+
]
2156+
"#
2157+
);
2158+
}
19302159
}

crates/uv/tests/it/edit.rs

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12587,6 +12587,115 @@ fn remove_all_with_comments() -> Result<()> {
1258712587
Ok(())
1258812588
}
1258912589

12590+
/// Removing a dependency should preserve end-of-line comments on nearby lines.
12591+
///
12592+
/// See: <https://github.com/astral-sh/uv/issues/18555>
12593+
#[test]
12594+
fn remove_preserves_nearby_end_of_line_comments() -> Result<()> {
12595+
let context = uv_test::test_context!("3.12");
12596+
12597+
let pyproject_toml = context.temp_dir.child("pyproject.toml");
12598+
pyproject_toml.write_str(indoc! {r#"
12599+
[project]
12600+
name = "project"
12601+
version = "0.1.0"
12602+
requires-python = ">=3.12"
12603+
dependencies = [
12604+
"iniconfig>=2.0.0", # this comment is clearly essential
12605+
"typing-extensions>=4.0.0",
12606+
]
12607+
"#})?;
12608+
12609+
uv_snapshot!(context.filters(), context.remove().arg("typing-extensions"), @"
12610+
success: true
12611+
exit_code: 0
12612+
----- stdout -----
12613+
12614+
----- stderr -----
12615+
Resolved 2 packages in [TIME]
12616+
Prepared 1 package in [TIME]
12617+
Installed 1 package in [TIME]
12618+
+ iniconfig==2.0.0
12619+
");
12620+
12621+
let pyproject_toml = context.read("pyproject.toml");
12622+
12623+
insta::with_settings!({
12624+
filters => context.filters(),
12625+
}, {
12626+
assert_snapshot!(
12627+
pyproject_toml, @r#"
12628+
[project]
12629+
name = "project"
12630+
version = "0.1.0"
12631+
requires-python = ">=3.12"
12632+
dependencies = [
12633+
"iniconfig>=2.0.0", # this comment is clearly essential
12634+
]
12635+
"#
12636+
);
12637+
});
12638+
12639+
Ok(())
12640+
}
12641+
12642+
/// Removing multiple adjacent matching dependencies should preserve comment order.
12643+
///
12644+
/// See: <https://github.com/astral-sh/uv/issues/18555>
12645+
#[test]
12646+
fn remove_preserves_comment_order_for_multiple_adjacent_matches() -> Result<()> {
12647+
let context = uv_test::test_context!("3.12");
12648+
12649+
let pyproject_toml = context.temp_dir.child("pyproject.toml");
12650+
pyproject_toml.write_str(indoc! {r#"
12651+
[project]
12652+
name = "project"
12653+
version = "0.1.0"
12654+
requires-python = ">=3.12"
12655+
dependencies = [
12656+
"iniconfig>=2.0.0", # comment on iniconfig
12657+
"typing-extensions>=4.0.0 ; python_version < '3.11'", # comment on first typing-extensions
12658+
"typing-extensions>=4.0.0 ; python_version >= '3.11'",
12659+
"sniffio>=1.3.0",
12660+
]
12661+
"#})?;
12662+
12663+
uv_snapshot!(context.filters(), context.remove().arg("typing-extensions"), @"
12664+
success: true
12665+
exit_code: 0
12666+
----- stdout -----
12667+
12668+
----- stderr -----
12669+
Resolved 3 packages in [TIME]
12670+
Prepared 2 packages in [TIME]
12671+
Installed 2 packages in [TIME]
12672+
+ iniconfig==2.0.0
12673+
+ sniffio==1.3.1
12674+
");
12675+
12676+
let pyproject_toml = context.read("pyproject.toml");
12677+
12678+
insta::with_settings!({
12679+
filters => context.filters(),
12680+
}, {
12681+
assert_snapshot!(
12682+
pyproject_toml, @r#"
12683+
[project]
12684+
name = "project"
12685+
version = "0.1.0"
12686+
requires-python = ">=3.12"
12687+
dependencies = [
12688+
"iniconfig>=2.0.0", # comment on iniconfig
12689+
# comment on first typing-extensions
12690+
"sniffio>=1.3.0",
12691+
]
12692+
"#
12693+
);
12694+
});
12695+
12696+
Ok(())
12697+
}
12698+
1259012699
/// If multiple indexes are provided on the CLI, the first-provided index should take precedence
1259112700
/// during resolution, and should appear first in the `pyproject.toml` file.
1259212701
///

0 commit comments

Comments
 (0)