Add lint for functions which never return

Author: GnomedDev

CHANGELOG.md

@@ -5761,6 +5761,7 @@ Released 2018-09-13
 [`neg_multiply`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_multiply
 [`negative_feature_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#negative_feature_names
 [`never_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#never_loop
+[`never_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#never_returns
 [`new_ret_no_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_ret_no_self
 [`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default
 [`new_without_default_derive`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default_derive

README.md

@@ -5,7 +5,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
 You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.

book/src/README.md

@@ -6,7 +6,7 @@
 A collection of lints to catch common mistakes and improve your
 [Rust](https://github.com/rust-lang/rust) code.
 
-[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 Lints are divided into categories, each with a default [lint
 level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how

book/src/lint_configuration.md

@@ -349,6 +349,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
 * [`large_types_passed_by_value`](https://rust-lang.github.io/rust-clippy/master/index.html#large_types_passed_by_value)
 * [`linkedlist`](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
 * [`needless_pass_by_ref_mut`](https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut)
+* [`never_returns`](https://rust-lang.github.io/rust-clippy/master/index.html#never_returns)
 * [`option_option`](https://rust-lang.github.io/rust-clippy/master/index.html#option_option)
 * [`rc_buffer`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer)
 * [`rc_mutex`](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)

clippy_config/src/conf.rs

@@ -398,6 +398,7 @@ define_Conf! {
         large_types_passed_by_value,
         linkedlist,
         needless_pass_by_ref_mut,
+        never_returns,
         option_option,
         rc_buffer,
         rc_mutex,

clippy_lints/src/declared_lints.rs

@@ -546,6 +546,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::needless_update::NEEDLESS_UPDATE_INFO,
     crate::neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD_INFO,
     crate::neg_multiply::NEG_MULTIPLY_INFO,
+    crate::never_returns::NEVER_RETURNS_INFO,
     crate::new_without_default::NEW_WITHOUT_DEFAULT_INFO,
     crate::no_effect::NO_EFFECT_INFO,
     crate::no_effect::NO_EFFECT_UNDERSCORE_BINDING_INFO,

clippy_lints/src/lib.rs

@@ -268,6 +268,7 @@ mod needless_question_mark;
 mod needless_update;
 mod neg_cmp_op_on_partial_ord;
 mod neg_multiply;
+mod never_returns;
 mod new_without_default;
 mod no_effect;
 mod no_mangle_with_rust_abi;
@@ -951,5 +952,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
     store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
     store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
+    store.register_late_pass(|_| Box::new(never_returns::NeverReturns::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/never_returns.rs

@@ -0,0 +1,181 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_opt;
+use clippy_utils::{ReturnType, ReturnVisitor, is_entrypoint_fn, visit_returns};
+use rustc_errors::Applicability;
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::{BodyId, FnRetTy, FnSig, ImplItem, ImplItemKind, Item, ItemKind, TyKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::TypeckResults;
+use rustc_middle::ty::adjustment::{Adjust, Adjustment};
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Detects functions that do not return, but do not have `!` as their return type.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Returning `!` is a more accurate API for your callers, and allows for optimisations/further linting.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn run() {
+    ///     loop {
+    ///         do_thing()
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn run() -> ! {
+    ///     loop {
+    ///         do_thing()
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.83.0"]
+    pub NEVER_RETURNS,
+    pedantic,
+    "functions that never return, but are typed to"
+}
+
+#[derive(Clone, Copy)]
+pub(crate) struct NeverReturns {
+    avoid_breaking_exported_api: bool,
+}
+
+impl_lint_pass!(NeverReturns => [NEVER_RETURNS]);
+
+impl NeverReturns {
+    pub fn new(conf: &Conf) -> Self {
+        Self {
+            avoid_breaking_exported_api: conf.avoid_breaking_exported_api,
+        }
+    }
+
+    fn check_item_fn(self, cx: &LateContext<'_>, sig: FnSig<'_>, def_id: LocalDefId, body_id: BodyId) {
+        let returns_unit = if let FnRetTy::Return(ret_ty) = sig.decl.output {
+            if let TyKind::Never = ret_ty.kind {
+                return;
+            }
+
+            matches!(ret_ty.kind, TyKind::Tup([]))
+        } else {
+            true
+        };
+
+        if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) {
+            return;
+        }
+
+        // We shouldn't try to change the signature of a lang item!
+        if cx.tcx.lang_items().from_def_id(def_id.to_def_id()).is_some() {
+            return;
+        }
+
+        let body = cx.tcx.hir().body(body_id);
+        let typeck_results = cx.tcx.typeck_body(body_id);
+        let mut visitor = NeverReturnVisitor {
+            typeck_results,
+            returns_unit,
+            found_implicit_return: false,
+        };
+
+        if visit_returns(&mut visitor, body.value).is_continue() && visitor.found_implicit_return {
+            let mut applicability = Applicability::MachineApplicable;
+            let (lint_span, mut snippet, sugg) = match sig.decl.output {
+                FnRetTy::DefaultReturn(span) => (span, String::new(), " -> !"),
+                FnRetTy::Return(ret_ty) => {
+                    let snippet = if let Some(snippet) = snippet_opt(cx, ret_ty.span) {
+                        format!(" a `{snippet}`")
+                    } else {
+                        applicability = Applicability::HasPlaceholders;
+                        String::new()
+                    };
+
+                    (ret_ty.span, snippet, "!")
+                },
+            };
+
+            snippet.insert_str(0, "function never returns, but is typed to return");
+            span_lint_and_sugg(
+                cx,
+                NEVER_RETURNS,
+                lint_span,
+                snippet,
+                "replace with",
+                sugg.into(),
+                applicability,
+            );
+        }
+    }
+}
+
+impl LateLintPass<'_> for NeverReturns {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        if let ItemKind::Fn(sig, _, body_id) = item.kind {
+            let local_def_id = item.owner_id.def_id;
+            if is_entrypoint_fn(cx, local_def_id.to_def_id()) {
+                return;
+            }
+
+            self.check_item_fn(cx, sig, local_def_id, body_id);
+        }
+    }
+
+    fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) {
+        if let ImplItemKind::Fn(sig, body_id) = item.kind {
+            let local_def_id = item.owner_id.def_id;
+            self.check_item_fn(cx, sig, local_def_id, body_id);
+        }
+    }
+}
+
+struct NeverReturnVisitor<'tcx> {
+    typeck_results: &'tcx TypeckResults<'tcx>,
+    found_implicit_return: bool,
+    returns_unit: bool,
+}
+
+impl ReturnVisitor for &mut NeverReturnVisitor<'_> {
+    type Result = std::ops::ControlFlow<()>;
+
+    fn visit_return(&mut self, kind: ReturnType<'_>) -> Self::Result {
+        let expression = match kind {
+            ReturnType::Explicit(expr) => expr,
+            ReturnType::UnitReturnExplicit(_) => {
+                return Self::Result::Break(());
+            },
+            ReturnType::Implicit(expr) | ReturnType::MissingElseImplicit(expr) => {
+                self.found_implicit_return = true;
+                expr
+            },
+            ReturnType::DivergingImplicit(_) => {
+                // If this function returns unit, a diverging implicit may just
+                // be an implicit unit return, in which case we should not lint.
+                return if self.returns_unit {
+                    Self::Result::Break(())
+                } else {
+                    Self::Result::Continue(())
+                };
+            },
+        };
+
+        if expression.span.from_expansion() {
+            return Self::Result::Break(());
+        }
+
+        let adjustments = self.typeck_results.expr_adjustments(expression);
+        if adjustments.iter().any(is_never_to_any) {
+            Self::Result::Continue(())
+        } else {
+            Self::Result::Break(())
+        }
+    }
+}
+
+fn is_never_to_any(adjustment: &Adjustment<'_>) -> bool {
+    matches!(adjustment.kind, Adjust::NeverToAny)
+}

lintcheck/src/driver.rs

@@ -54,7 +54,7 @@ fn run_clippy(addr: &str) -> Option<i32> {
     }
 }
 
-pub fn drive(addr: &str) {
+pub fn drive(addr: &str) -> ! {
     process::exit(run_clippy(addr).unwrap_or_else(|| {
         Command::new("rustc")
             .args(env::args_os().skip(2))

src/driver.rs

@@ -181,7 +181,7 @@ const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/ne
 
 #[allow(clippy::too_many_lines)]
 #[allow(clippy::ignored_unit_patterns)]
-pub fn main() {
+pub fn main() -> ! {
     let early_dcx = EarlyDiagCtxt::new(ErrorOutputType::default());
 
     rustc_driver::init_rustc_env_logger(&early_dcx);

tests/compile-test.rs

@@ -583,7 +583,7 @@ impl LintMetadata {
         }
     }
 
-    fn applicability_str(&self) -> &str {
+    fn applicability_str(&self) -> &'static str {
         match self.applicability {
             Applicability::MachineApplicable => "MachineApplicable",
             Applicability::HasPlaceholders => "HasPlaceholders",

tests/ui-toml/panic/panic.rs

@@ -13,7 +13,7 @@ fn main() {
     }
 }
 
-fn issue_13292() {
+fn issue_13292() -> ! {
     panic_any("should lint")
 }
 

tests/ui/empty_loop.rs

@@ -5,7 +5,7 @@
 extern crate proc_macros;
 use proc_macros::{external, inline_macros};
 
-fn should_trigger() {
+fn should_trigger() -> ! {
     loop {}
     #[allow(clippy::never_loop)]
     loop {

tests/ui/implicit_return.fixed

@@ -118,6 +118,7 @@ fn loop_macro_test() -> bool {
     }
 }
 
+#[expect(clippy::never_returns)]
 fn divergent_test() -> bool {
     fn diverge() -> ! {
         panic!()

tests/ui/implicit_return.rs

@@ -118,6 +118,7 @@ fn loop_macro_test() -> bool {
     }
 }
 
+#[expect(clippy::never_returns)]
 fn divergent_test() -> bool {
     fn diverge() -> ! {
         panic!()

tests/ui/implicit_return.stderr

@@ -170,7 +170,7 @@ LL +     }
    |
 
 error: missing `return` statement
-  --> tests/ui/implicit_return.rs:130:5
+  --> tests/ui/implicit_return.rs:131:5
    |
 LL |     true
    |     ^^^^

tests/ui/infinite_loops.rs

@@ -1,7 +1,7 @@
 //@no-rustfix
 //@aux-build:proc_macros.rs
 
-#![allow(clippy::never_loop)]
+#![allow(clippy::never_loop, clippy::never_returns)]
 #![warn(clippy::infinite_loop)]
 
 extern crate proc_macros;

tests/ui/needless_continue.rs

@@ -55,14 +55,14 @@ fn main() {
     }
 }
 
-fn simple_loop() {
+fn simple_loop() -> ! {
     loop {
         continue;
         //~^ ERROR: this `continue` expression is redundant
     }
 }
 
-fn simple_loop2() {
+fn simple_loop2() -> ! {
     loop {
         println!("bleh");
         continue;
@@ -71,15 +71,15 @@ fn simple_loop2() {
 }
 
 #[rustfmt::skip]
-fn simple_loop3() {
+fn simple_loop3() -> ! {
     loop {
         continue
         //~^ ERROR: this `continue` expression is redundant
     }
 }
 
 #[rustfmt::skip]
-fn simple_loop4() {
+fn simple_loop4() -> ! {
     loop {
         println!("bleh");
         continue
@@ -94,7 +94,7 @@ mod issue_2329 {
     fn update_condition() {}
 
     // only the outer loop has a label
-    fn foo() {
+    fn foo() -> ! {
         'outer: loop {
             println!("Entry");
             while condition() {
@@ -118,7 +118,7 @@ mod issue_2329 {
     }
 
     // both loops have labels
-    fn bar() {
+    fn bar() -> ! {
         'outer: loop {
             println!("Entry");
             'inner: while condition() {