fix: unused_unit suggests wrongly when unit never type fallback

profetia · profetia · commit a51cadf10632 · 2025-04-17T20:54:17.000+08:00
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -771,7 +771,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_early_pass(|| Box::new(formatting::Formatting));
     store.register_early_pass(|| Box::new(misc_early::MiscEarlyLints));
     store.register_late_pass(|_| Box::new(redundant_closure_call::RedundantClosureCall));
-    store.register_early_pass(|| Box::new(unused_unit::UnusedUnit));
+    store.register_early_pass(move || Box::new(unused_unit::UnusedUnit::new(conf)));
+    store.register_late_pass(move |_| Box::new(unused_unit::UnusedUnit::new(conf)));
     store.register_late_pass(|_| Box::new(returns::Return));
     store.register_late_pass(move |tcx| Box::new(collapsible_if::CollapsibleIf::new(tcx, conf)));
     store.register_late_pass(|_| Box::new(items_after_statements::ItemsAfterStatements));
diff --git a/clippy_lints/src/unused_unit.rs b/clippy_lints/src/unused_unit.rs
@@ -1,11 +1,19 @@
+use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::source::{SpanRangeExt, position_before_rarrow};
-use rustc_ast::visit::FnKind;
-use rustc_ast::{ClosureBinder, ast};
+use clippy_utils::{is_never_expr, is_unit_expr};
+use rustc_ast::{Block, StmtKind};
 use rustc_errors::Applicability;
-use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::declare_lint_pass;
-use rustc_span::{BytePos, Span};
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{
+    AssocItemConstraintKind, Body, Expr, ExprKind, FnDecl, FnRetTy, GenericArgsParentheses, Node, PolyTraitRef, Term,
+    Ty, TyKind,
+};
+use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+use rustc_span::{BytePos, Span, sym};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -32,29 +40,101 @@ declare_clippy_lint! {
     "needless unit expression"
 }
 
-declare_lint_pass!(UnusedUnit => [UNUSED_UNIT]);
+pub struct UnusedUnit {
+    msrv: Msrv,
+}
 
-impl EarlyLintPass for UnusedUnit {
-    fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: ast::NodeId) {
-        if let ast::FnRetTy::Ty(ref ty) = kind.decl().output
-            && let ast::TyKind::Tup(ref vals) = ty.kind
-            && vals.is_empty()
-            && !ty.span.from_expansion()
-            && get_def(span) == get_def(ty.span)
+impl_lint_pass!(UnusedUnit => [UNUSED_UNIT]);
+
+impl UnusedUnit {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for UnusedUnit {
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        kind: FnKind<'tcx>,
+        decl: &'tcx FnDecl<'tcx>,
+        body: &'tcx Body<'tcx>,
+        span: Span,
+        def_id: LocalDefId,
+    ) {
+        if let FnRetTy::Return(hir_ty) = decl.output
+            && is_unit_ty(hir_ty)
+            && !hir_ty.span.from_expansion()
+            && get_def(span) == get_def(hir_ty.span)
         {
             // implicit types in closure signatures are forbidden when `for<...>` is present
-            if let FnKind::Closure(&ClosureBinder::For { .. }, ..) = kind {
+            if let FnKind::Closure = kind
+                && let Node::Expr(expr) = cx.tcx.hir_node_by_def_id(def_id)
+                && let ExprKind::Closure(closure) = expr.kind
+                && !closure.bound_generic_params.is_empty()
+            {
                 return;
             }
 
-            lint_unneeded_unit_return(cx, ty, span);
+            // unit never type fallback is no longer supported since Rust 2024. For more information,
+            // see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/never-type-fallback.html>
+            if self.msrv.meets(cx, msrvs::UNIT_NEVER_TYPE_FALLBACK)
+                && let ExprKind::Block(block, _) = body.value.kind
+                && let Some(expr) = block.expr
+                && is_never_expr(cx, expr).is_some()
+            {
+                return;
+            }
+
+            lint_unneeded_unit_return(cx, hir_ty.span, span);
         }
     }
 
-    fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
-        if let Some(stmt) = block.stmts.last()
-            && let ast::StmtKind::Expr(ref expr) = stmt.kind
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if let ExprKind::Ret(Some(expr)) | ExprKind::Break(_, Some(expr)) = expr.kind
             && is_unit_expr(expr)
+            && !expr.span.from_expansion()
+        {
+            span_lint_and_sugg(
+                cx,
+                UNUSED_UNIT,
+                expr.span,
+                "unneeded `()`",
+                "remove the `()`",
+                String::new(),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+
+    fn check_poly_trait_ref(&mut self, cx: &LateContext<'tcx>, poly: &'tcx PolyTraitRef<'tcx>) {
+        let segments = &poly.trait_ref.path.segments;
+
+        if segments.len() == 1
+            && ["Fn", "FnMut", "FnOnce"].contains(&segments[0].ident.name.as_str())
+            && let Some(args) = segments[0].args
+            && args.parenthesized == GenericArgsParentheses::ParenSugar
+            && let constraints = &args.constraints
+            && constraints.len() == 1
+            && constraints[0].ident.name == sym::Output
+            && let AssocItemConstraintKind::Equality { term: Term::Ty(hir_ty) } = constraints[0].kind
+            && args.span_ext.hi() != poly.span.hi()
+            && !hir_ty.span.from_expansion()
+            && is_unit_ty(hir_ty)
+        {
+            lint_unneeded_unit_return(cx, hir_ty.span, poly.span);
+        }
+    }
+}
+
+impl EarlyLintPass for UnusedUnit {
+    /// Check for unit expressions in blocks. This is left in the early pass because some macros
+    /// expand its inputs as-is, making it invisible to the late pass. See #4076.
+    fn check_block(&mut self, cx: &EarlyContext<'_>, block: &Block) {
+        if let Some(stmt) = block.stmts.last()
+            && let StmtKind::Expr(expr) = &stmt.kind
+            && let rustc_ast::ExprKind::Tup(inner) = &expr.kind
+            && inner.is_empty()
             && let ctxt = block.span.ctxt()
             && stmt.span.ctxt() == ctxt
             && expr.span.ctxt() == ctxt
@@ -72,39 +152,10 @@ impl EarlyLintPass for UnusedUnit {
             );
         }
     }
+}
 
-    fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
-        match e.kind {
-            ast::ExprKind::Ret(Some(ref expr)) | ast::ExprKind::Break(_, Some(ref expr)) => {
-                if is_unit_expr(expr) && !expr.span.from_expansion() {
-                    span_lint_and_sugg(
-                        cx,
-                        UNUSED_UNIT,
-                        expr.span,
-                        "unneeded `()`",
-                        "remove the `()`",
-                        String::new(),
-                        Applicability::MachineApplicable,
-                    );
-                }
-            },
-            _ => (),
-        }
-    }
-
-    fn check_poly_trait_ref(&mut self, cx: &EarlyContext<'_>, poly: &ast::PolyTraitRef) {
-        let segments = &poly.trait_ref.path.segments;
-
-        if segments.len() == 1
-            && ["Fn", "FnMut", "FnOnce"].contains(&segments[0].ident.name.as_str())
-            && let Some(args) = &segments[0].args
-            && let ast::GenericArgs::Parenthesized(generic_args) = &**args
-            && let ast::FnRetTy::Ty(ty) = &generic_args.output
-            && ty.kind.is_unit()
-        {
-            lint_unneeded_unit_return(cx, ty, generic_args.span);
-        }
-    }
+fn is_unit_ty(ty: &Ty<'_>) -> bool {
+    matches!(ty.kind, TyKind::Tup([]))
 }
 
 // get the def site
@@ -117,24 +168,15 @@ fn get_def(span: Span) -> Option<Span> {
     }
 }
 
-// is this expr a `()` unit?
-fn is_unit_expr(expr: &ast::Expr) -> bool {
-    if let ast::ExprKind::Tup(ref vals) = expr.kind {
-        vals.is_empty()
-    } else {
-        false
-    }
-}
-
-fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) {
+fn lint_unneeded_unit_return(cx: &LateContext<'_>, ty_span: Span, span: Span) {
     let (ret_span, appl) =
-        span.with_hi(ty.span.hi())
+        span.with_hi(ty_span.hi())
             .get_source_text(cx)
-            .map_or((ty.span, Applicability::MaybeIncorrect), |src| {
-                position_before_rarrow(&src).map_or((ty.span, Applicability::MaybeIncorrect), |rpos| {
+            .map_or((ty_span, Applicability::MaybeIncorrect), |src| {
+                position_before_rarrow(&src).map_or((ty_span, Applicability::MaybeIncorrect), |rpos| {
                     (
                         #[expect(clippy::cast_possible_truncation)]
-                        ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)),
+                        ty_span.with_lo(BytePos(span.lo().0 + rpos as u32)),
                         Applicability::MachineApplicable,
                     )
                 })
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
@@ -23,7 +23,7 @@ macro_rules! msrv_aliases {
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
     1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT }
-    1,85,0 { UINT_FLOAT_MIDPOINT }
+    1,85,0 { UINT_FLOAT_MIDPOINT, UNIT_NEVER_TYPE_FALLBACK }
     1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR }
     1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
     1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
diff --git a/tests/ui/unused_unit.fixed b/tests/ui/unused_unit.fixed
@@ -120,3 +120,24 @@ mod issue9949 {
         ()
     }
 }
+
+#[clippy::msrv = "1.85"]
+mod issue14577 {
+    trait Unit {}
+    impl Unit for () {}
+
+    fn run<R: Unit>(f: impl FnOnce() -> R) {
+        f();
+    }
+
+    fn bar() {
+        run(|| -> () { todo!() });
+    }
+
+    struct UnitStruct;
+    impl UnitStruct {
+        fn apply<F: for<'c> Fn(&'c mut Self)>(&mut self, f: F) {
+            todo!()
+        }
+    }
+}
diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs
@@ -120,3 +120,24 @@ mod issue9949 {
         ()
     }
 }
+
+#[clippy::msrv = "1.85"]
+mod issue14577 {
+    trait Unit {}
+    impl Unit for () {}
+
+    fn run<R: Unit>(f: impl FnOnce() -> R) {
+        f();
+    }
+
+    fn bar() {
+        run(|| -> () { todo!() });
+    }
+
+    struct UnitStruct;
+    impl UnitStruct {
+        fn apply<F: for<'c> Fn(&'c mut Self)>(&mut self, f: F) {
+            todo!()
+        }
+    }
+}
diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr
@@ -1,15 +1,21 @@
-error: unneeded unit return type
-  --> tests/ui/unused_unit.rs:20:58
+error: unneeded unit expression
+  --> tests/ui/unused_unit.rs:35:9
    |
-LL |     pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
-   |                                                          ^^^^^^ help: remove the `-> ()`
+LL |         ()
+   |         ^^ help: remove the final `()`
    |
 note: the lint level is defined here
   --> tests/ui/unused_unit.rs:13:9
    |
 LL | #![deny(clippy::unused_unit)]
    |         ^^^^^^^^^^^^^^^^^^^
 
+error: unneeded unit expression
+  --> tests/ui/unused_unit.rs:60:26
+   |
+LL | fn return_unit() -> () { () }
+   |                          ^^ help: remove the final `()`
+
 error: unneeded unit return type
   --> tests/ui/unused_unit.rs:20:28
    |
@@ -22,6 +28,12 @@ error: unneeded unit return type
 LL |     where G: Fn() -> () {
    |                  ^^^^^^ help: remove the `-> ()`
 
+error: unneeded unit return type
+  --> tests/ui/unused_unit.rs:20:58
+   |
+LL |     pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) -> ()
+   |                                                          ^^^^^^ help: remove the `-> ()`
+
 error: unneeded unit return type
   --> tests/ui/unused_unit.rs:25:26
    |
@@ -34,12 +46,6 @@ error: unneeded unit return type
 LL |     fn into(self) -> () {
    |                  ^^^^^^ help: remove the `-> ()`
 
-error: unneeded unit expression
-  --> tests/ui/unused_unit.rs:35:9
-   |
-LL |         ()
-   |         ^^ help: remove the final `()`
-
 error: unneeded unit return type
   --> tests/ui/unused_unit.rs:41:29
    |
@@ -82,12 +88,6 @@ error: unneeded unit return type
 LL | fn return_unit() -> () { () }
    |                 ^^^^^^ help: remove the `-> ()`
 
-error: unneeded unit expression
-  --> tests/ui/unused_unit.rs:60:26
-   |
-LL | fn return_unit() -> () { () }
-   |                          ^^ help: remove the final `()`
-
 error: unneeded `()`
   --> tests/ui/unused_unit.rs:72:14
    |
