Add lint for functions which never return

CHANGELOG.md

@@ -5800,6 +5800,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

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

@@ -428,6 +428,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

@@ -552,6 +552,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

@@ -266,6 +266,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(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
     store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
     store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf)));
+    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,190 @@
+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, 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 do_thing() {}
+    /// fn run() {
+    ///     loop {
+    ///         do_thing();
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # fn do_thing() {}
+    /// 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);
+        } else if let ItemKind::Impl(impl_) = item.kind {
+            // Do not lint trait impls
+            if impl_.of_trait.is_some() {
+                return;
+            }
+
+            for impl_item in impl_.items {
+                let ImplItemKind::Fn(sig, body_id) = cx.tcx.hir().impl_item(impl_item.id).kind else {
+                    continue;
+                };
+
+                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)
+}

clippy_lints/src/unnecessary_literal_bound.rs

@@ -1,10 +1,10 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::path_res;
+use clippy_utils::{ReturnType, ReturnVisitor, path_res, visit_returns};
 use rustc_ast::ast::LitKind;
 use rustc_errors::Applicability;
 use rustc_hir::def::Res;
-use rustc_hir::intravisit::{FnKind, Visitor};
-use rustc_hir::{Body, Expr, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind, intravisit};
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{Body, ExprKind, FnDecl, FnRetTy, Lit, MutTy, Mutability, PrimTy, Ty, TyKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::Span;
@@ -67,49 +67,36 @@ fn extract_anonymous_ref<'tcx>(hir_ty: &Ty<'tcx>) -> Option<&'tcx Ty<'tcx>> {
     Some(ty)
 }
 
-fn is_str_literal(expr: &Expr<'_>) -> bool {
-    matches!(
-        expr.kind,
-        ExprKind::Lit(Lit {
-            node: LitKind::Str(..),
-            ..
-        }),
-    )
-}
-
-struct FindNonLiteralReturn;
+struct LiteralReturnVisitor;
 
-impl<'hir> Visitor<'hir> for FindNonLiteralReturn {
+impl ReturnVisitor for LiteralReturnVisitor {
     type Result = std::ops::ControlFlow<()>;
-    type NestedFilter = intravisit::nested_filter::None;
+    fn visit_return(&mut self, kind: ReturnType<'_>) -> Self::Result {
+        let expr = match kind {
+            ReturnType::Implicit(e) | ReturnType::Explicit(e) => e,
+            ReturnType::UnitReturnExplicit(_) | ReturnType::MissingElseImplicit(_) => {
+                panic!("Function which returns `&str` has a unit return!")
+            },
+            ReturnType::DivergingImplicit(_) => {
+                // If this block is implicitly returning `!`, it can return `&'static str`.
+                return Self::Result::Continue(());
+            },
+        };
 
-    fn visit_expr(&mut self, expr: &'hir Expr<'hir>) -> Self::Result {
-        if let ExprKind::Ret(Some(ret_val_expr)) = expr.kind
-            && !is_str_literal(ret_val_expr)
-        {
-            Self::Result::Break(())
+        if matches!(
+            expr.kind,
+            ExprKind::Lit(Lit {
+                node: LitKind::Str(..),
+                ..
+            })
+        ) {
+            Self::Result::Continue(())
         } else {
-            intravisit::walk_expr(self, expr)
+            Self::Result::Break(())
         }
     }
 }
 
-fn check_implicit_returns_static_str(body: &Body<'_>) -> bool {
-    // TODO: Improve this to the same complexity as the Visitor to catch more implicit return cases.
-    if let ExprKind::Block(block, _) = body.value.kind
-        && let Some(implicit_ret) = block.expr
-    {
-        return is_str_literal(implicit_ret);
-    }
-
-    false
-}
-
-fn check_explicit_returns_static_str(expr: &Expr<'_>) -> bool {
-    let mut visitor = FindNonLiteralReturn;
-    visitor.visit_expr(expr).is_continue()
-}
-
 impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
     fn check_fn(
         &mut self,
@@ -143,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryLiteralBound {
         }
 
         // Check for all return statements returning literals
-        if check_explicit_returns_static_str(body.value) && check_implicit_returns_static_str(body) {
+        if visit_returns(LiteralReturnVisitor, body.value).is_continue() {
             span_lint_and_sugg(
                 cx,
                 UNNECESSARY_LITERAL_BOUND,

clippy_utils/src/lib.rs

@@ -10,6 +10,7 @@
 #![feature(assert_matches)]
 #![feature(unwrap_infallible)]
 #![feature(array_windows)]
+#![feature(associated_type_defaults)]
 #![recursion_limit = "512"]
 #![allow(
     clippy::missing_errors_doc,
@@ -30,6 +31,7 @@
 // FIXME: switch to something more ergonomic here, once available.
 // (Currently there is no way to opt into sysroot crates without `extern crate`.)
 extern crate rustc_ast;
+extern crate rustc_ast_ir;
 extern crate rustc_ast_pretty;
 extern crate rustc_attr;
 extern crate rustc_const_eval;
@@ -69,6 +71,7 @@ pub mod numeric_literal;
 pub mod paths;
 pub mod ptr;
 pub mod qualify_min_const_fn;
+mod returns;
 pub mod source;
 pub mod str_utils;
 pub mod sugg;
@@ -81,6 +84,7 @@ pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match};
 pub use self::hir_utils::{
     HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over,
 };
+pub use returns::{ReturnType, ReturnVisitor, visit_returns};
 
 use core::mem;
 use core::ops::ControlFlow;