Merge pull request #1 from sesgoe/ses/update-clippy-fix-13518

fix: check against 'main' function name instead of entrypoint function

Author: Jared-Prime

clippy_lints/src/exit.rs

@@ -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, LintContext};
 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:
@@ -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]);
@@ -52,10 +65,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.as_str() != "main"
         {
             span_lint(cx, EXIT, e.span, "usage of `process::exit`");
         }

tests/ui/exit1_compile_flag_test.rs

@@ -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);
+}

tests/ui/exit1_compile_flag_test.stderr

@@ -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
+

tests/ui/exit2_compile_flag_test.rs

@@ -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);
+}

tests/ui/exit2_compile_flag_test.stderr

@@ -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
+

tests/ui/exit3_compile_flag_test.rs

@@ -0,0 +1,11 @@
+//@ check-pass
+//@compile-flags: --test
+
+#![warn(clippy::exit)]
+
+fn main() {
+    if true {
+        std::process::exit(2);
+    };
+    std::process::exit(1);
+}