Skip to content

skip exit late lint pass on tests #15222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions clippy_lints/src/exit.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,38 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_entrypoint_fn;
use rustc_hir::{Expr, ExprKind, Item, ItemKind, OwnerNode};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
/// Detects calls to the `exit()` function which terminates the program.
/// Detects calls to the `exit()` function that are not in the `main` function. Calls to `exit()`
/// immediately terminate the program.
///
/// ### Why restrict this?
/// `exit()` immediately terminates the program with no information other than an exit code.
/// This provides no means to troubleshoot a problem, and may be an unexpected side effect.
///
/// Codebases may use this lint to require that all exits are performed either by panicking
/// (which produces a message, a code location, and optionally a backtrace)
/// or by returning from `main()` (which is a single place to look).
/// or by calling `exit()` from `main()` (which is a single place to look).
///
/// ### Example
/// ### Good example
/// ```no_run
/// std::process::exit(0)
/// fn main() {
/// std::process::exit(0);
/// }
/// ```
///
/// ### Bad example
/// ```no_run
/// fn main() {
/// other_function();
/// }
///
/// fn other_function() {
/// std::process::exit(0);
/// }
/// ```
///
/// Use instead:
Expand All @@ -36,7 +49,7 @@ declare_clippy_lint! {
#[clippy::version = "1.41.0"]
pub EXIT,
restriction,
"detects `std::process::exit` calls"
"detects `std::process::exit` calls outside of `main`"
}

declare_lint_pass!(Exit => [EXIT]);
Expand All @@ -48,10 +61,14 @@ impl<'tcx> LateLintPass<'tcx> for Exit {
&& let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::process_exit, def_id)
&& let parent = cx.tcx.hir_get_parent_item(e.hir_id)
&& let OwnerNode::Item(Item{kind: ItemKind::Fn{ .. }, ..}) = cx.tcx.hir_owner_node(parent)
// If the next item up is a function we check if it is an entry point
&& let OwnerNode::Item(Item{kind: ItemKind::Fn{ ident, .. }, ..}) = cx.tcx.hir_owner_node(parent)
// If the next item up is a function we check if it isn't named "main"
// and only then emit a linter warning
&& !is_entrypoint_fn(cx, parent.to_def_id())

// if you instead check for the parent of the `exit()` call being the entrypoint function, as this worked before,
// in compilation contexts like --all-targets (which include --tests), you get false positives
// because in a test context, main is not the entrypoint function
&& ident.name != sym::main
{
span_lint(cx, EXIT, e.span, "usage of `process::exit`");
}
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/exit1_compile_flag_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//@compile-flags: --test
#![warn(clippy::exit)]

fn not_main() {
if true {
std::process::exit(4);
//~^ exit
}
}

fn main() {
if true {
std::process::exit(2);
};
not_main();
std::process::exit(1);
}
11 changes: 11 additions & 0 deletions tests/ui/exit1_compile_flag_test.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: usage of `process::exit`
--> tests/ui/exit1_compile_flag_test.rs:6:9
|
LL | std::process::exit(4);
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::exit` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::exit)]`

error: aborting due to 1 previous error

15 changes: 15 additions & 0 deletions tests/ui/exit2_compile_flag_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@compile-flags: --test
#![warn(clippy::exit)]

fn also_not_main() {
std::process::exit(3);
//~^ exit
}

fn main() {
if true {
std::process::exit(2);
};
also_not_main();
std::process::exit(1);
}
11 changes: 11 additions & 0 deletions tests/ui/exit2_compile_flag_test.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: usage of `process::exit`
--> tests/ui/exit2_compile_flag_test.rs:5:5
|
LL | std::process::exit(3);
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::exit` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::exit)]`

error: aborting due to 1 previous error

11 changes: 11 additions & 0 deletions tests/ui/exit3_compile_flag_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ check-pass
//@compile-flags: --test

#![warn(clippy::exit)]

fn main() {
if true {
std::process::exit(2);
};
std::process::exit(1);
}
8 changes: 8 additions & 0 deletions tests/ui/exit4.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//@ check-pass
//@compile-flags: --test

#![warn(clippy::exit)]

fn main() {
std::process::exit(0)
}