Skip to content

Commit

Permalink
remove last statement special casing
Browse files Browse the repository at this point in the history
  • Loading branch information
y21 committed Jul 3, 2024
1 parent a089c00 commit e3ccab4
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 47 deletions.
40 changes: 6 additions & 34 deletions clippy_lints/src/zombie_processes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,18 @@ impl<'tcx> LateLintPass<'tcx> for ZombieProcesses {
}

// Don't emit a suggestion since the binding is used later
check(cx, expr, local.hir_id, false);
check(cx, expr, false);
},
Node::LetStmt(&LetStmt { pat, hir_id, .. }) if let PatKind::Wild = pat.kind => {
Node::LetStmt(&LetStmt { pat, .. }) if let PatKind::Wild = pat.kind => {
// `let _ = child;`, also dropped immediately without `wait()`ing
check(cx, expr, hir_id, true);
check(cx, expr, true);
},
Node::Stmt(&Stmt {
kind: StmtKind::Semi(_),
hir_id,
..
}) => {
// Immediately dropped. E.g. `std::process::Command::new("echo").spawn().unwrap();`
check(cx, expr, hir_id, true);
check(cx, expr, true);
},
_ => {},
}
Expand Down Expand Up @@ -212,9 +211,8 @@ impl<'a, 'tcx> Visitor<'tcx> for WaitFinder<'a, 'tcx> {
/// such as checking that the binding is not used in certain ways, which isn't necessary for
/// `let _ = <expr that spawns child>;`.
///
/// This checks if the program doesn't unconditionally exit after the spawn expression and that it
/// isn't the last statement of the program.
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: HirId, emit_suggestion: bool) {
/// This checks if the program doesn't unconditionally exit after the spawn expression.
fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, emit_suggestion: bool) {
let Some(block) = get_enclosing_block(cx, spawn_expr.hir_id) else {
return;
};
Expand All @@ -228,12 +226,6 @@ fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: Hi
return;
}

// This might be the last effective node of the program (main function).
// There's no need to lint in that case either, as this is basically equivalent to calling `exit()`
if is_last_node_in_main(cx, node_id) {
return;
}

span_lint_and_then(
cx,
ZOMBIE_PROCESSES,
Expand All @@ -257,26 +249,6 @@ fn check<'tcx>(cx: &LateContext<'tcx>, spawn_expr: &'tcx Expr<'tcx>, node_id: Hi
);
}

/// The hir id id may either correspond to a `Local` or `Stmt`, depending on how we got here.
/// This function gets the enclosing function, checks if it's `main` and if so,
/// check if the last statement modulo blocks is the given id.
fn is_last_node_in_main(cx: &LateContext<'_>, id: HirId) -> bool {
let hir = cx.tcx.hir();
let body_owner = hir.enclosing_body_owner(id);
let enclosing_body = hir.body_owned_by(body_owner);

if let Some((main_def_id, _)) = cx.tcx.entry_fn(())
&& main_def_id == body_owner.to_def_id()
&& let ExprKind::Block(block, _) = &enclosing_body.value.peel_blocks().kind
&& let [.., stmt] = block.stmts
{
matches!(stmt.kind, StmtKind::Let(local) if local.hir_id == id)
|| matches!(stmt.kind, StmtKind::Semi(..) if stmt.hir_id == id)
} else {
false
}
}

/// Checks if the given expression exits the process.
fn is_exit_expression(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
fn_def_id(cx, expr).is_some_and(|fn_did| {
Expand Down
9 changes: 0 additions & 9 deletions tests/ui/zombie_processes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,6 @@ fn main() {
}
x.wait().unwrap();
}

// Checking that it won't lint if spawn is the last statement of a main function.
// IMPORTANT: this case must always be at the very end so it always tests the right thing.
// Don't move this.
{
{
Command::new("").spawn().unwrap();
}
}
}

fn process_child(c: Child) {
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/zombie_processes_fixable.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::zombie_processes)]
#![allow(clippy::needless_return)]

use std::process::{Child, Command};

Expand All @@ -19,3 +20,7 @@ fn not_main() {
fn spawn_proc() -> Child {
Command::new("").spawn().unwrap()
}

fn spawn_proc_2() -> Child {
return Command::new("").spawn().unwrap();
}
1 change: 1 addition & 0 deletions tests/ui/zombie_processes_fixable.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![warn(clippy::zombie_processes)]
#![allow(clippy::needless_return)]

use std::process::{Child, Command};

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/zombie_processes_fixable.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: spawned process is never `wait()`ed on
--> tests/ui/zombie_processes_fixable.rs:6:13
--> tests/ui/zombie_processes_fixable.rs:7:13
|
LL | let _ = Command::new("").spawn().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try: `.wait()`
Expand All @@ -10,7 +10,7 @@ LL | let _ = Command::new("").spawn().unwrap();
= help: to override `-D warnings` add `#[allow(clippy::zombie_processes)]`

error: spawned process is never `wait()`ed on
--> tests/ui/zombie_processes_fixable.rs:8:5
--> tests/ui/zombie_processes_fixable.rs:9:5
|
LL | Command::new("").spawn().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try: `.wait()`
Expand All @@ -19,7 +19,7 @@ LL | Command::new("").spawn().unwrap();
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning

error: spawned process is never `wait()`ed on
--> tests/ui/zombie_processes_fixable.rs:10:5
--> tests/ui/zombie_processes_fixable.rs:11:5
|
LL | spawn_proc();
| ^^^^^^^^^^^^- help: try: `.wait()`
Expand All @@ -28,7 +28,7 @@ LL | spawn_proc();
= note: see https://doc.rust-lang.org/stable/std/process/struct.Child.html#warning

error: spawned process is never `wait()`ed on
--> tests/ui/zombie_processes_fixable.rs:16:5
--> tests/ui/zombie_processes_fixable.rs:17:5
|
LL | Command::new("").spawn().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try: `.wait()`
Expand Down

0 comments on commit e3ccab4

Please sign in to comment.