Skip to content

Commit d964e55

Browse files
authored
skip exit late lint pass on tests (#15222)
changelog: [`exit`]: When using the `--test` or `--all-targets` flag, the exit lint should not fail on the main function. Fixes #13518 With help from @sesgoe
2 parents c943f4c + 81dd4e6 commit d964e55

File tree

7 files changed

+99
-9
lines changed

7 files changed

+99
-9
lines changed

clippy_lints/src/exit.rs

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,38 @@
11
use clippy_utils::diagnostics::span_lint;
2-
use clippy_utils::is_entrypoint_fn;
32
use rustc_hir::{Expr, ExprKind, Item, ItemKind, OwnerNode};
43
use rustc_lint::{LateContext, LateLintPass};
54
use rustc_session::declare_lint_pass;
65
use rustc_span::sym;
76

87
declare_clippy_lint! {
98
/// ### What it does
10-
/// Detects calls to the `exit()` function which terminates the program.
9+
/// Detects calls to the `exit()` function that are not in the `main` function. Calls to `exit()`
10+
/// immediately terminate the program.
1111
///
1212
/// ### Why restrict this?
1313
/// `exit()` immediately terminates the program with no information other than an exit code.
1414
/// This provides no means to troubleshoot a problem, and may be an unexpected side effect.
1515
///
1616
/// Codebases may use this lint to require that all exits are performed either by panicking
1717
/// (which produces a message, a code location, and optionally a backtrace)
18-
/// or by returning from `main()` (which is a single place to look).
18+
/// or by calling `exit()` from `main()` (which is a single place to look).
1919
///
20-
/// ### Example
20+
/// ### Good example
2121
/// ```no_run
22-
/// std::process::exit(0)
22+
/// fn main() {
23+
/// std::process::exit(0);
24+
/// }
25+
/// ```
26+
///
27+
/// ### Bad example
28+
/// ```no_run
29+
/// fn main() {
30+
/// other_function();
31+
/// }
32+
///
33+
/// fn other_function() {
34+
/// std::process::exit(0);
35+
/// }
2336
/// ```
2437
///
2538
/// Use instead:
@@ -36,7 +49,7 @@ declare_clippy_lint! {
3649
#[clippy::version = "1.41.0"]
3750
pub EXIT,
3851
restriction,
39-
"detects `std::process::exit` calls"
52+
"detects `std::process::exit` calls outside of `main`"
4053
}
4154

4255
declare_lint_pass!(Exit => [EXIT]);
@@ -48,10 +61,14 @@ impl<'tcx> LateLintPass<'tcx> for Exit {
4861
&& let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id()
4962
&& cx.tcx.is_diagnostic_item(sym::process_exit, def_id)
5063
&& let parent = cx.tcx.hir_get_parent_item(e.hir_id)
51-
&& let OwnerNode::Item(Item{kind: ItemKind::Fn{ .. }, ..}) = cx.tcx.hir_owner_node(parent)
52-
// If the next item up is a function we check if it is an entry point
64+
&& let OwnerNode::Item(Item{kind: ItemKind::Fn{ ident, .. }, ..}) = cx.tcx.hir_owner_node(parent)
65+
// If the next item up is a function we check if it isn't named "main"
5366
// and only then emit a linter warning
54-
&& !is_entrypoint_fn(cx, parent.to_def_id())
67+
68+
// if you instead check for the parent of the `exit()` call being the entrypoint function, as this worked before,
69+
// in compilation contexts like --all-targets (which include --tests), you get false positives
70+
// because in a test context, main is not the entrypoint function
71+
&& ident.name != sym::main
5572
{
5673
span_lint(cx, EXIT, e.span, "usage of `process::exit`");
5774
}

tests/ui/exit1_compile_flag_test.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@compile-flags: --test
2+
#![warn(clippy::exit)]
3+
4+
fn not_main() {
5+
if true {
6+
std::process::exit(4);
7+
//~^ exit
8+
}
9+
}
10+
11+
fn main() {
12+
if true {
13+
std::process::exit(2);
14+
};
15+
not_main();
16+
std::process::exit(1);
17+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error: usage of `process::exit`
2+
--> tests/ui/exit1_compile_flag_test.rs:6:9
3+
|
4+
LL | std::process::exit(4);
5+
| ^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::exit` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::exit)]`
9+
10+
error: aborting due to 1 previous error
11+

tests/ui/exit2_compile_flag_test.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//@compile-flags: --test
2+
#![warn(clippy::exit)]
3+
4+
fn also_not_main() {
5+
std::process::exit(3);
6+
//~^ exit
7+
}
8+
9+
fn main() {
10+
if true {
11+
std::process::exit(2);
12+
};
13+
also_not_main();
14+
std::process::exit(1);
15+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error: usage of `process::exit`
2+
--> tests/ui/exit2_compile_flag_test.rs:5:5
3+
|
4+
LL | std::process::exit(3);
5+
| ^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::exit` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::exit)]`
9+
10+
error: aborting due to 1 previous error
11+

tests/ui/exit3_compile_flag_test.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//@ check-pass
2+
//@compile-flags: --test
3+
4+
#![warn(clippy::exit)]
5+
6+
fn main() {
7+
if true {
8+
std::process::exit(2);
9+
};
10+
std::process::exit(1);
11+
}

tests/ui/exit4.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//@ check-pass
2+
//@compile-flags: --test
3+
4+
#![warn(clippy::exit)]
5+
6+
fn main() {
7+
std::process::exit(0)
8+
}

0 commit comments

Comments
 (0)