Skip to content

Commit 6235d4b

Browse files
committed
fix(cargo): keep compile errors visible when warnings are captured
Two gaps in the adopted warning-preservation change, both on the compile-error path: - Buffered (filter_cargo_test): the captured warnings section is pushed into `result` up front, so the `result.trim().is_empty()` guard could never fire once any warning existed. A run that failed to compile *and* warned returned the warnings and no errors at all. Measure the body past the warnings prefix instead. - Streaming (CargoTestHandler): warning blocks are now emitted live, but the compile-error summary re-renders them via filter_cargo_build_labeled, so every warning was reported twice. Added filter_cargo_build_inner with a skip_warning_blocks flag; only this one call site sets it, so build, check and clippy rendering are untouched. Regression tests for both paths.
1 parent ff13986 commit 6235d4b

1 file changed

Lines changed: 116 additions & 2 deletions

File tree

‎src/cmds/rust/cargo_cmd.rs‎

Lines changed: 116 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,8 @@ impl BlockHandler for CargoTestHandler {
225225
if self.has_compile_errors || !json.errors.is_empty() {
226226
// Content-based (exit 0): a real compile error yields "cargo test: N
227227
// errors"; a bare "could not compile" leaves the raw tail fallback.
228-
let build_filtered = filter_cargo_build_labeled(raw, "test", 0);
228+
// Warning blocks already went out live, so suppress them here.
229+
let build_filtered = filter_cargo_build_inner(raw, "test", 0, true);
229230
if build_filtered.contains("cargo test:") {
230231
return Some(format!("{}\n", build_filtered));
231232
}
@@ -988,6 +989,17 @@ fn filter_cargo_build(output: &str) -> String {
988989
}
989990

990991
fn filter_cargo_build_labeled(output: &str, label: &'static str, exit_code: i32) -> String {
992+
filter_cargo_build_inner(output, label, exit_code, false)
993+
}
994+
995+
/// `skip_warning_blocks` drops rendered warning blocks while keeping them in the
996+
/// counts — for callers that already emitted those blocks themselves.
997+
fn filter_cargo_build_inner(
998+
output: &str,
999+
label: &'static str,
1000+
exit_code: i32,
1001+
skip_warning_blocks: bool,
1002+
) -> String {
9911003
let mut handler = CargoBuildHandler::with_label(label);
9921004
let mut blocks: Vec<Vec<String>> = Vec::new();
9931005
let mut current_block: Vec<String> = Vec::new();
@@ -1015,6 +1027,12 @@ fn filter_cargo_build_labeled(output: &str, label: &'static str, exit_code: i32)
10151027
if !current_block.is_empty() {
10161028
blocks.push(current_block);
10171029
}
1030+
if skip_warning_blocks {
1031+
blocks.retain(|b| {
1032+
b.first()
1033+
.is_some_and(|l| !(l.starts_with("warning:") || l.starts_with("warning[")))
1034+
});
1035+
}
10181036

10191037
let json = extract_json_diagnostics(output);
10201038
let (errors, warnings) = merge_diag_counts(handler.error_count, handler.warnings, &json);
@@ -1325,7 +1343,11 @@ pub(crate) fn filter_cargo_test(output: &str) -> String {
13251343
result.push_str(&format!("{}\n", line));
13261344
}
13271345

1328-
if result.trim().is_empty() {
1346+
// The warnings section is a prefix, not content — measure the body only, or a
1347+
// compile failure that also warned would skip the fallback below and report
1348+
// warnings with no errors. filter_cargo_build_labeled renders its own warning
1349+
// blocks, so the prefix is deliberately dropped on that path.
1350+
if result[warnings_section.len()..].trim().is_empty() {
13291351
let json = extract_json_diagnostics(output);
13301352
let has_compile_errors = !json.errors.is_empty()
13311353
|| output.lines().any(|line| {
@@ -1736,6 +1758,98 @@ test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; fini
17361758
assert!(!result.contains("generated 1 warning"));
17371759
}
17381760

1761+
#[test]
1762+
fn test_filter_cargo_test_compile_error_with_warnings_keeps_errors() {
1763+
// A captured warning must not mask the compile-error fallback: warnings
1764+
// are a prefix, not content, so the "nothing to report" check has to
1765+
// ignore them or a failed build reports warnings and no errors.
1766+
let output = r#" Compiling failproj v0.1.0
1767+
warning: function `unused_helper` is never used
1768+
--> src/lib.rs:9:4
1769+
|
1770+
9 | fn unused_helper() -> i32 { 42 }
1771+
| ^^^^^^^^^^^^^
1772+
|
1773+
= note: `#[warn(dead_code)]` on by default
1774+
1775+
error[E0308]: mismatched types
1776+
--> src/lib.rs:14:5
1777+
|
1778+
14 | "nope"
1779+
| ^^^^^^ expected `i32`, found `&str`
1780+
1781+
error: aborting due to 1 previous error
1782+
"#;
1783+
let result = filter_cargo_test(output);
1784+
assert!(
1785+
result.contains("E0308"),
1786+
"compile error must survive alongside warnings, got: {}",
1787+
result
1788+
);
1789+
assert!(
1790+
result.contains("unused_helper"),
1791+
"warning must still be reported, got: {}",
1792+
result
1793+
);
1794+
// The build fallback renders warning blocks itself — no double report.
1795+
assert_eq!(
1796+
result
1797+
.matches("warning: function `unused_helper` is never used")
1798+
.count(),
1799+
1,
1800+
"warning block must appear exactly once, got: {}",
1801+
result
1802+
);
1803+
}
1804+
1805+
#[test]
1806+
fn test_streamed_cargo_test_handler_does_not_repeat_streamed_warnings() {
1807+
use crate::core::stream::{BlockStreamFilter, StreamFilter};
1808+
// Warning blocks are streamed live, so the compile-error summary must not
1809+
// re-render them — otherwise a failed build reports every warning twice.
1810+
let raw = r#" Compiling failproj v0.1.0
1811+
warning: function `unused_helper` is never used
1812+
--> src/lib.rs:1:4
1813+
|
1814+
1 | fn unused_helper() -> i32 {
1815+
| ^^^^^^^^^^^^^
1816+
|
1817+
= note: `#[warn(dead_code)]` on by default
1818+
1819+
error[E0308]: mismatched types
1820+
--> src/lib.rs:14:5
1821+
|
1822+
14 | "nope"
1823+
| ^^^^^^ expected `i32`, found `&str`
1824+
1825+
error: aborting due to 1 previous error
1826+
"#;
1827+
let mut filter = BlockStreamFilter::new(CargoTestHandler::new());
1828+
let mut emitted = String::new();
1829+
for line in raw.lines() {
1830+
if let Some(out) = filter.feed_line(line) {
1831+
emitted.push_str(&out);
1832+
}
1833+
}
1834+
emitted.push_str(&filter.flush());
1835+
let summary = filter.on_exit(0, raw).unwrap_or_default();
1836+
let combined = format!("{}{}", emitted, summary);
1837+
1838+
assert_eq!(
1839+
combined
1840+
.matches("warning: function `unused_helper` is never used")
1841+
.count(),
1842+
1,
1843+
"streamed warning must not be repeated by the summary, got: {}",
1844+
combined
1845+
);
1846+
assert!(
1847+
combined.contains("E0308"),
1848+
"compile error must still be reported, got: {}",
1849+
combined
1850+
);
1851+
}
1852+
17391853
#[test]
17401854
fn test_streamed_cargo_test_handler_preserves_compiler_warnings() {
17411855
use crate::core::stream::{BlockStreamFilter, StreamFilter};

0 commit comments

Comments
 (0)