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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jul 8, 2025
Merged

skip exit late lint pass on tests #15222

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2b93d2c
skip exit late lint pass on tests
Jared-Prime Jul 7, 2025
1f36d4d
run `cargo dev fmt` on changes
Jared-Prime Jul 7, 2025
c000a01
fix: check against 'main' function name instead of entrypoint function
sesgoe Jul 7, 2025
5853e6f
chore: add tests to check against the --test compile flag
sesgoe Jul 7, 2025
52aa383
Merge pull request #1 from sesgoe/ses/update-clippy-fix-13518
Jared-Prime Jul 7, 2025
1c7fb0f
do not completely suppress warning with test code
Jared-Prime Jul 7, 2025
9580d61
remove no longer used import
Jared-Prime Jul 7, 2025
81dd4e6
use preinterned symbol
Jared-Prime Jul 7, 2025
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
6 changes: 5 additions & 1 deletion clippy_lints/src/exit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::declare_lint_pass;
use rustc_span::sym;

Expand Down Expand Up @@ -43,6 +43,10 @@ declare_lint_pass!(Exit => [EXIT]);

impl<'tcx> LateLintPass<'tcx> for Exit {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if cx.sess().is_test_crate() {
return;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suppresses the lint entirely in --test sessions and will not emit a lint anywhere in the crate, even if we aren't actually in a main fn and we should emit a warning, right?

I think that would interact confusingly with #[expect] and create a situation similar to #13358, where if you have something like:

fn foo() {
  #[expect(clippy::exit)]
  std::process::exit();
}

and run cargo clippy --all-targets, then you get an unfulfilled lint expectation, but if you remove the #[expect], you do get the warning there.

--all-targets runs clippy twice, once without --test and again with --test, so only sometimes emitting a warning dependent on the session (like is done here) is a bit confusing for users IMO

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if, instead of using !is_entrypoint_fn at the end of the condition chain, which I guess is also dependent on the session and different for the test harness, we could just change that to check if the enclosing function's name is not literally "main". That seems closer to what we actually want and would be more consistent between compiler sessions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@y21 I had the same thought after reading your first comment, and upon consideration, this seems like a good way to go

I saw the if cx.sess().is_test_crate() { return; } pattern elsewhere, which is why it seemed reasonable to slot it in here too -- the only real thought was: in a test context, is this lint relevant?

The confusing interaction with --all-targets is worth fixing, and checking that the name is literally main makes it clear in both compilation contexts/sessions, so that feels like a good solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @y21 . I added the changes and documentation updates recommended by @sesgoe then removed the blanket skip for test crates. Let me know if anything else required here!

if let ExprKind::Call(path_expr, [_]) = e.kind
&& let ExprKind::Path(ref path) = path_expr.kind
&& let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id()
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/exit4.rs
View file
Open in desktop
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)
}