Skip to content

Commit 3220bde

Browse files
committed
fix(unify): eliminate multi-target false positives in unused dependency
detection
1 parent 3bad1f7 commit 3220bde

2 files changed

Lines changed: 127 additions & 11 deletions

File tree

src/cargo/unify/unused_finder.rs

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -304,31 +304,71 @@ fn run_unused_crate_dependency_check(
304304
.output()
305305
.with_context(|| format!("running cargo check in {}", workspace_root.display()))?;
306306

307-
let mut by_member: HashMap<String, HashSet<String>> = HashMap::new();
307+
let mut member_targets: HashMap<String, HashSet<String>> = HashMap::new();
308+
let mut member_dep_unused_targets: HashMap<String, HashMap<String, HashSet<String>>> = HashMap::new();
309+
308310
for line in String::from_utf8_lossy(&output.stdout).lines() {
309-
let Ok(message) = serde_json::from_str::<CargoCompilerMessage>(line) else {
311+
let Ok(message) = serde_json::from_str::<CargoEvent>(line) else {
310312
continue;
311313
};
312-
if message.reason != "compiler-message" {
313-
continue;
314-
}
315-
if message.message.code.as_ref().and_then(|c| c.code.as_deref()) != Some("unused_crate_dependencies") {
314+
if message.reason != "compiler-message" && message.reason != "compiler-artifact" {
316315
continue;
317316
}
317+
318318
let Some(manifest_path) = message.manifest_path.as_deref() else {
319319
continue;
320320
};
321321
let Some(member_name) = resolve_member_name(manifest_path, manifest_to_member) else {
322322
continue;
323323
};
324-
let Some(crate_name) = parse_unused_crate_name(&message.message.message) else {
324+
325+
let Some(target) = message.target.as_ref() else {
326+
continue;
327+
};
328+
if !is_relevant_target(target) {
329+
continue;
330+
}
331+
let target_id = target.identifier();
332+
member_targets
333+
.entry(member_name.to_string())
334+
.or_default()
335+
.insert(target_id.clone());
336+
337+
if message.reason != "compiler-message" {
338+
continue;
339+
}
340+
let Some(diagnostic) = message.message.as_ref() else {
341+
continue;
342+
};
343+
if diagnostic.code.as_ref().and_then(|c| c.code.as_deref()) != Some("unused_crate_dependencies") {
344+
continue;
345+
}
346+
let Some(crate_name) = parse_unused_crate_name(&diagnostic.message) else {
325347
continue;
326348
};
327349

328-
by_member
350+
member_dep_unused_targets
329351
.entry(member_name.to_string())
330352
.or_default()
331-
.insert(crate_name.replace('-', "_"));
353+
.entry(crate_name.replace('-', "_"))
354+
.or_default()
355+
.insert(target_id);
356+
}
357+
358+
let mut by_member: HashMap<String, HashSet<String>> = HashMap::new();
359+
for (member, dep_targets) in member_dep_unused_targets {
360+
let Some(all_targets) = member_targets.get(&member) else {
361+
continue;
362+
};
363+
if all_targets.is_empty() {
364+
continue;
365+
}
366+
367+
for (dep_name, warned_targets) in dep_targets {
368+
if all_targets.iter().all(|target| warned_targets.contains(target)) {
369+
by_member.entry(member.clone()).or_default().insert(dep_name);
370+
}
371+
}
332372
}
333373

334374
Ok(by_member)
@@ -347,10 +387,27 @@ fn parse_unused_crate_name(message: &str) -> Option<&str> {
347387
}
348388

349389
#[derive(Debug, Deserialize)]
350-
struct CargoCompilerMessage {
390+
struct CargoEvent {
351391
reason: String,
352392
manifest_path: Option<String>,
353-
message: CargoDiagnostic,
393+
target: Option<CargoTarget>,
394+
message: Option<CargoDiagnostic>,
395+
}
396+
397+
#[derive(Debug, Deserialize)]
398+
struct CargoTarget {
399+
kind: Vec<String>,
400+
name: String,
401+
src_path: Option<String>,
402+
}
403+
404+
impl CargoTarget {
405+
fn identifier(&self) -> String {
406+
match &self.src_path {
407+
Some(src_path) => src_path.clone(),
408+
None => format!("{}:{}", self.kind.join(","), self.name),
409+
}
410+
}
354411
}
355412

356413
#[derive(Debug, Deserialize)]
@@ -364,6 +421,10 @@ struct CargoDiagnosticCode {
364421
code: Option<String>,
365422
}
366423

424+
fn is_relevant_target(target: &CargoTarget) -> bool {
425+
!target.kind.iter().any(|kind| kind == "custom-build")
426+
}
427+
367428
/// Check if a target constraint (cfg expression) matches any configured target
368429
///
369430
/// This is a heuristic check - we look for common patterns in the cfg string

tests/integration/test_unused_detection.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,61 @@ serialization = ["serde"]
133133
Ok(())
134134
}
135135

136+
#[test]
137+
fn test_unused_detection_multi_target_does_not_union_false_positive() -> Result<()> {
138+
// Regression: rustc's unused-crate-dependencies lint is target-local.
139+
// A dep used in lib but not in bin must NOT be treated as package-unused.
140+
let workspace = create_workspace_with_unused_detection()?;
141+
142+
add_crate_with_manifest(
143+
&workspace,
144+
"test-crate",
145+
r#"[package]
146+
name = "test-crate"
147+
version = "0.1.0"
148+
edition = "2021"
149+
150+
[dependencies]
151+
once_cell = "1.0"
152+
"#,
153+
)?;
154+
155+
fs::write(
156+
workspace.path.join("crates/test-crate/src/lib.rs"),
157+
r#"//! test crate
158+
use once_cell::sync::Lazy;
159+
160+
static CELL: Lazy<u8> = Lazy::new(|| 7);
161+
162+
pub fn hello() -> u8 {
163+
*CELL
164+
}
165+
"#,
166+
)?;
167+
fs::write(
168+
workspace.path.join("crates/test-crate/src/main.rs"),
169+
r#"fn main() {
170+
println!("bin target");
171+
}
172+
"#,
173+
)?;
174+
175+
workspace.commit("Add lib+bin crate using dependency only in lib")?;
176+
177+
let output = run_cargo_rail(&workspace.path, &["rail", "unify", "--check"])?;
178+
let stdout = String::from_utf8_lossy(&output.stdout);
179+
let stderr = String::from_utf8_lossy(&output.stderr);
180+
181+
assert!(
182+
!stdout.contains("once_cell") || !stdout.contains("Unused"),
183+
"once_cell should NOT be flagged as unused when used by lib target\nstdout:\n{}\nstderr:\n{}",
184+
stdout,
185+
stderr
186+
);
187+
188+
Ok(())
189+
}
190+
136191
#[test]
137192
fn test_unused_detection_optional_dep_with_dep_syntax_not_flagged() -> Result<()> {
138193
// Test the `dep:name` syntax (Rust 2021+) for optional deps

0 commit comments

Comments
 (0)