Skip to content

Commit 91185fe

Browse files
y21y21
committed
add suggestion, reword diagnostic
1 parent a283b66 commit 91185fe

File tree

7 files changed

+162
-105
lines changed

7 files changed

+162
-105
lines changed

‎clippy_lints/src/zombie_processes.rs‎

Lines changed: 53 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::visitors::for_each_local_use_after_expr;
33
use clippy_utils::{fn_def_id, get_enclosing_block, match_any_def_paths, match_def_path, paths};
44
use rustc_ast::Mutability;
5-
use rustc_hir::def::DefKind;
5+
use rustc_errors::Applicability;
66
use rustc_hir::intravisit::{walk_expr, Visitor};
77
use rustc_hir::{Expr, ExprKind, HirId, Local, Node, PatKind, Stmt, StmtKind};
88
use rustc_lint::{LateContext, LateLintPass};
9-
use rustc_session::{declare_lint_pass, declare_tool_lint};
10-
use rustc_span::{sym, Span};
9+
use rustc_session::declare_lint_pass;
10+
use rustc_span::sym;
1111
use std::ops::ControlFlow;
1212

1313
declare_clippy_lint! {
@@ -41,19 +41,6 @@ declare_clippy_lint! {
4141
}
4242
declare_lint_pass!(ZombieProcesses => [ZOMBIE_PROCESSES]);
4343

44-
fn emit_lint(cx: &LateContext<'_>, span: Span) {
45-
span_lint_and_then(
46-
cx,
47-
ZOMBIE_PROCESSES,
48-
span,
49-
"spawned process is never `wait()`-ed on and leaves behind a zombie process",
50-
|diag| {
51-
diag.help("consider calling `.wait()`")
52-
.note("also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning");
53-
},
54-
);
55-
}
56-
5744
impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
5845
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
5946
if let ExprKind::Call(..) | ExprKind::MethodCall(..) = expr.kind
@@ -62,7 +49,6 @@ impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
6249
{
6350
match cx.tcx.hir().get_parent(expr.hir_id) {
6451
Node::Local(local) if let PatKind::Binding(_, local_id, ..) = local.pat.kind => {
65-
6652
// If the `Child` is assigned to a variable, we want to check if the code never calls `.wait()`
6753
// on the variable, and lint if not.
6854
// This is difficult to do because expressions can be arbitrarily complex
@@ -73,46 +59,53 @@ impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
7359
// - calling `id` or `kill`
7460
// - no use at all (e.g. `let _x = child;`)
7561
// - taking a shared reference (`&`), `wait()` can't go through that
76-
// Neither of these is sufficient to prevent zombie processes
62+
// None of these are sufficient to prevent zombie processes
7763
// Doing it like this means more FNs, but FNs are better than FPs.
7864
let has_no_wait = for_each_local_use_after_expr(cx, local_id, expr.hir_id, |expr| {
7965
match cx.tcx.hir().get_parent(expr.hir_id) {
80-
Node::Stmt(Stmt { kind: StmtKind::Semi(_), .. }) => ControlFlow::Continue(()),
66+
Node::Stmt(Stmt {
67+
kind: StmtKind::Semi(_),
68+
..
69+
}) => ControlFlow::Continue(()),
8170
Node::Expr(expr) if let ExprKind::Field(..) = expr.kind => ControlFlow::Continue(()),
8271
Node::Expr(expr) if let ExprKind::AddrOf(_, Mutability::Not, _) = expr.kind => {
8372
ControlFlow::Continue(())
84-
}
73+
},
8574
Node::Expr(expr)
8675
if let Some(fn_did) = fn_def_id(cx, expr)
87-
&& match_any_def_paths(cx, fn_did, &[
88-
&paths::CHILD_ID,
89-
&paths::CHILD_KILL,
90-
]).is_some() =>
76+
&& match_any_def_paths(cx, fn_did, &[&paths::CHILD_ID, &paths::CHILD_KILL])
77+
.is_some() =>
9178
{
9279
ControlFlow::Continue(())
93-
}
80+
},
9481

9582
// Conservatively assume that all other kinds of nodes call `.wait()` somehow.
9683
_ => ControlFlow::Break(()),
9784
}
98-
}).is_continue();
85+
})
86+
.is_continue();
9987

10088
// If it does have a `wait()` call, we're done. Don't lint.
10189
if !has_no_wait {
10290
return;
10391
}
10492

105-
check(cx, expr, local.hir_id);
93+
// Don't emit a suggestion since the binding is used later
94+
check(cx, expr, local.hir_id, false);
10695
},
10796
Node::Local(&Local { pat, hir_id, .. }) if let PatKind::Wild = pat.kind => {
10897
// `let _ = child;`, also dropped immediately without `wait()`ing
109-
check(cx, expr, hir_id);
110-
}
111-
Node::Stmt(&Stmt { kind: StmtKind::Semi(_), hir_id, .. }) => {
98+
check(cx, expr, hir_id, true);
99+
},
100+
Node::Stmt(&Stmt {
101+
kind: StmtKind::Semi(_),
102+
hir_id,
103+
..
104+
}) => {
112105
// Immediately dropped. E.g. `std::process::Command::new("echo").spawn().unwrap();`
113-
check(cx, expr, hir_id);
114-
}
115-
_ => {}
106+
check(cx, expr, hir_id, true);
107+
},
108+
_ => {},
116109
}
117110
}
118111
}
@@ -126,7 +119,7 @@ impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
126119
///
127120
/// This checks if the program doesn't unconditionally exit after the spawn expression and that it
128121
/// isn't the last statement of the program.
129-
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: HirId) {
122+
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: HirId, emit_suggestion: bool) {
130123
let Some(block) = get_enclosing_block(cx, spawn_expr.hir_id) else {
131124
return;
132125
};
@@ -148,7 +141,27 @@ fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: Hi
148141
return;
149142
}
150143

151-
emit_lint(cx, spawn_expr.span);
144+
span_lint_and_then(
145+
cx,
146+
ZOMBIE_PROCESSES,
147+
spawn_expr.span,
148+
"spawned process is never `wait()`ed on",
149+
|diag| {
150+
if emit_suggestion {
151+
diag.span_suggestion(
152+
spawn_expr.span.shrink_to_hi(),
153+
"try",
154+
".wait()",
155+
Applicability::MaybeIncorrect,
156+
);
157+
} else {
158+
diag.note("consider calling `.wait()`");
159+
}
160+
161+
diag.note("not doing so might leave behind zombie processes")
162+
.note("see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning");
163+
},
164+
);
152165
}
153166

154167
/// The hir id id may either correspond to a `Local` or `Stmt`, depending on how we got here.
@@ -159,8 +172,8 @@ fn is_last_node_in_main(cx: &LateContext<'_>, id: HirId) -> bool {
159172
let body_owner = hir.enclosing_body_owner(id);
160173
let enclosing_body = hir.body(hir.body_owned_by(body_owner));
161174

162-
if cx.tcx.opt_def_kind(body_owner) == Some(DefKind::Fn)
163-
&& cx.tcx.opt_item_name(body_owner.to_def_id()) == Some(sym::main)
175+
if let Some((main_def_id, _)) = cx.tcx.entry_fn(())
176+
&& main_def_id == body_owner.to_def_id()
164177
&& let ExprKind::Block(block, _) = &enclosing_body.value.peel_blocks().kind
165178
&& let [.., stmt] = block.stmts
166179
{
@@ -173,11 +186,9 @@ fn is_last_node_in_main(cx: &LateContext<'_>, id: HirId) -> bool {
173186

174187
/// Checks if the given expression exits the process.
175188
fn is_exit_expression(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
176-
if let Some(fn_did) = fn_def_id(cx, expr) {
177-
match_any_def_paths(cx, fn_did, &[&paths::EXIT, &paths::ABORT]).is_some()
178-
} else {
179-
false
180-
}
189+
fn_def_id(cx, expr).is_some_and(|fn_did| {
190+
cx.tcx.is_diagnostic_item(sym::process_exit, fn_did) || match_def_path(cx, fn_did, &paths::ABORT)
191+
})
181192
}
182193

183194
#[derive(Debug)]

‎clippy_utils/src/paths.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ pub const CORE_RESULT_OK_METHOD: [&str; 4] = ["core", "result", "Result", "ok"];
2525
pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"];
2626
pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"];
2727
pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"];
28-
pub const EXIT: [&str; 3] = ["std", "process", "exit"];
2928
pub const CHILD: [&str; 3] = ["std", "process", "Child"];
3029
pub const CHILD_ID: [&str; 4] = ["std", "process", "Child", "id"];
3130
pub const CHILD_KILL: [&str; 4] = ["std", "process", "Child", "kill"];

‎tests/ui/zombie_processes.rs‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,9 @@
33
use std::process::{Child, Command};
44

55
fn main() {
6-
let _ = Command::new("").spawn().unwrap();
7-
//~^ ERROR: spawned process is never `wait()`-ed on
8-
Command::new("").spawn().unwrap();
9-
//~^ ERROR: spawned process is never `wait()`-ed on
10-
spawn_proc();
11-
//~^ ERROR: spawned process is never `wait()`-ed on
12-
spawn_proc().wait().unwrap(); // OK
13-
146
{
157
let mut x = Command::new("").spawn().unwrap();
16-
//~^ ERROR: spawned process is never `wait()`-ed on
8+
//~^ ERROR: spawned process is never `wait()`ed on
179
x.kill();
1810
x.id();
1911
}
@@ -40,7 +32,7 @@ fn main() {
4032
}
4133
{
4234
let mut x = Command::new("").spawn().unwrap();
43-
//~^ ERROR: spawned process is never `wait()`-ed on
35+
//~^ ERROR: spawned process is never `wait()`ed on
4436
let v = &x;
4537
// (allow shared refs is fine because one cannot call `.wait()` through that)
4638
}
@@ -65,14 +57,14 @@ fn main() {
6557
// It should assume that it might not exit and still lint
6658
{
6759
let mut x = Command::new("").spawn().unwrap();
68-
//~^ ERROR: spawned process is never `wait()`-ed on
60+
//~^ ERROR: spawned process is never `wait()`ed on
6961
if true {
7062
std::process::exit(0);
7163
}
7264
}
7365
{
7466
let mut x = Command::new("").spawn().unwrap();
75-
//~^ ERROR: spawned process is never `wait()`-ed on
67+
//~^ ERROR: spawned process is never `wait()`ed on
7668
if true {
7769
while false {}
7870
// Calling `exit()` after leaving a while loop should still be linted.
@@ -90,10 +82,6 @@ fn main() {
9082
}
9183
}
9284

93-
fn spawn_proc() -> Child {
94-
todo!()
95-
}
96-
9785
fn process_child(c: Child) {
9886
todo!()
9987
}

‎tests/ui/zombie_processes.stderr‎

Lines changed: 23 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,44 @@
1-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
2-
--> $DIR/zombie_processes.rs:6:13
3-
|
4-
LL | let _ = Command::new("").spawn().unwrap();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6-
|
7-
= help: consider calling `.wait()`
8-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
9-
= note: `-D clippy::zombie-processes` implied by `-D warnings`
10-
= help: to override `-D warnings` add `#[allow(clippy::zombie_processes)]`
11-
12-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
13-
--> $DIR/zombie_processes.rs:8:5
14-
|
15-
LL | Command::new("").spawn().unwrap();
16-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17-
|
18-
= help: consider calling `.wait()`
19-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
20-
21-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
22-
--> $DIR/zombie_processes.rs:10:5
23-
|
24-
LL | spawn_proc();
25-
| ^^^^^^^^^^^^
26-
|
27-
= help: consider calling `.wait()`
28-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
29-
30-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
31-
--> $DIR/zombie_processes.rs:15:21
1+
error: spawned process is never `wait()`ed on
2+
--> $DIR/zombie_processes.rs:7:21
323
|
334
LL | let mut x = Command::new("").spawn().unwrap();
345
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
356
|
36-
= help: consider calling `.wait()`
37-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
7+
= note: consider calling `.wait()`
8+
= note: not doing so might leave behind zombie processes
9+
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
10+
= note: `-D clippy::zombie-processes` implied by `-D warnings`
11+
= help: to override `-D warnings` add `#[allow(clippy::zombie_processes)]`
3812

39-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
40-
--> $DIR/zombie_processes.rs:42:21
13+
error: spawned process is never `wait()`ed on
14+
--> $DIR/zombie_processes.rs:34:21
4115
|
4216
LL | let mut x = Command::new("").spawn().unwrap();
4317
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4418
|
45-
= help: consider calling `.wait()`
46-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
19+
= note: consider calling `.wait()`
20+
= note: not doing so might leave behind zombie processes
21+
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
4722

48-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
49-
--> $DIR/zombie_processes.rs:67:21
23+
error: spawned process is never `wait()`ed on
24+
--> $DIR/zombie_processes.rs:59:21
5025
|
5126
LL | let mut x = Command::new("").spawn().unwrap();
5227
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5328
|
54-
= help: consider calling `.wait()`
55-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
29+
= note: consider calling `.wait()`
30+
= note: not doing so might leave behind zombie processes
31+
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
5632

57-
error: spawned process is never `wait()`-ed on and leaves behind a zombie process
58-
--> $DIR/zombie_processes.rs:74:21
33+
error: spawned process is never `wait()`ed on
34+
--> $DIR/zombie_processes.rs:66:21
5935
|
6036
LL | let mut x = Command::new("").spawn().unwrap();
6137
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6238
|
63-
= help: consider calling `.wait()`
64-
= note: also see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
39+
= note: consider calling `.wait()`
40+
= note: not doing so might leave behind zombie processes
41+
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning
6542

66-
error: aborting due to 7 previous errors
43+
error: aborting due to 4 previous errors
6744

‎tests/ui/zombie_processes_fixable.fixed‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![warn(clippy::zombie_processes)]
2+
3+
use std::process::{Child, Command};
4+
5+
fn main() {
6+
let _ = Command::new("").spawn().unwrap().wait();
7+
//~^ ERROR: spawned process is never `wait()`ed on
8+
Command::new("").spawn().unwrap().wait();
9+
//~^ ERROR: spawned process is never `wait()`ed on
10+
spawn_proc().wait();
11+
//~^ ERROR: spawned process is never `wait()`ed on
12+
spawn_proc().wait().unwrap(); // OK
13+
}
14+
15+
fn not_main() {
16+
Command::new("").spawn().unwrap().wait();
17+
}
18+
19+
fn spawn_proc() -> Child {
20+
todo!()
21+
}

‎tests/ui/zombie_processes_fixable.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#![warn(clippy::zombie_processes)]
2+
3+
use std::process::{Child, Command};
4+
5+
fn main() {
6+
let _ = Command::new("").spawn().unwrap();
7+
//~^ ERROR: spawned process is never `wait()`ed on
8+
Command::new("").spawn().unwrap();
9+
//~^ ERROR: spawned process is never `wait()`ed on
10+
spawn_proc();
11+
//~^ ERROR: spawned process is never `wait()`ed on
12+
spawn_proc().wait().unwrap(); // OK
13+
}
14+
15+
fn not_main() {
16+
Command::new("").spawn().unwrap();
17+
}
18+
19+
fn spawn_proc() -> Child {
20+
todo!()
21+
}

0 commit comments

Comments
 (0)