unused_unit: don't lint on closure return types (rust-lang#15549)

As discussed in [#clippy >
rust-lang#15035](https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/.2315035)

Fixes rust-lang#15035

changelog: [`unused_unit`] don't lint on closure return types

Author: samueltardieu

clippy_lints/src/unused_unit.rs

@@ -6,8 +6,8 @@ use rustc_errors::Applicability;
 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,
+    AssocItemConstraintKind, Body, Expr, ExprKind, FnDecl, FnRetTy, GenericArgsParentheses, PolyTraitRef, Term, Ty,
+    TyKind,
 };
 use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
@@ -49,19 +49,22 @@ impl<'tcx> LateLintPass<'tcx> for UnusedUnit {
         decl: &'tcx FnDecl<'tcx>,
         body: &'tcx Body<'tcx>,
         span: Span,
-        def_id: LocalDefId,
+        _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 = 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()
-            {
+            // The explicit `-> ()` in the closure signature might be necessary for multiple reasons:
+            // - Implicit types in closure signatures are forbidden when `for<...>` is present
+            // - If the closure body ends with a function call, and that function's return type is generic, the
+            //   `-> ()` could be required for it to be inferred
+            //
+            // There could be more reasons to have it, and, in general, we shouldn't discourage the users from
+            // writing more type annotations than strictly necessary, because it can help readability and
+            // maintainability
+            if let FnKind::Closure = kind {
                 return;
             }
 

tests/ui/unused_unit.edition2021.fixed

@@ -127,14 +127,10 @@ mod issue14577 {
     trait Unit {}
     impl Unit for () {}
 
-    fn run<R: Unit>(f: impl FnOnce() -> R) {
-        f();
-    }
-
     #[allow(dependency_on_unit_never_type_fallback)]
     fn bar() {
-        run(|| { todo!() }); 
         //~[edition2021]^ unused_unit
+        panic!()
     }
 
     struct UnitStruct;
@@ -150,3 +146,24 @@ mod pr14962 {
     type UnusedParensButNoUnit = Box<dyn (Fn())>;
 }
 
+
+mod issue15035 {
+
+    trait Convert<T> {
+        fn from(value: T) -> Self;
+    }
+
+    impl Convert<u64> for () {
+        fn from(_value: u64) -> Self {}
+    }
+
+    fn handle<T: Convert<u64>>(value: u64) -> T {
+        Convert::from(value)
+    }
+
+    pub fn f() -> Option<bool> {
+        let result: Result<bool, u64> = Err(42);
+        // the `-> ()` is required for the inference of `handle`'s return type
+        result.map_err(|err| -> () { handle(err) }).ok()
+    }
+}

tests/ui/unused_unit.edition2021.stderr

@@ -119,10 +119,10 @@ LL | fn test3()-> (){}
    |           ^^^^^ help: remove the `-> ()`
 
 error: unneeded unit return type
-  --> tests/ui/unused_unit.rs:136:15
+  --> tests/ui/unused_unit.rs:131:13
    |
-LL |         run(|| -> () { todo!() }); 
-   |               ^^^^^^ help: remove the `-> ()`
+LL |     fn bar() -> () {
+   |             ^^^^^^ help: remove the `-> ()`
 
 error: aborting due to 20 previous errors
 

tests/ui/unused_unit.edition2024.fixed

@@ -127,14 +127,10 @@ mod issue14577 {
     trait Unit {}
     impl Unit for () {}
 
-    fn run<R: Unit>(f: impl FnOnce() -> R) {
-        f();
-    }
-
     #[allow(dependency_on_unit_never_type_fallback)]
-    fn bar() {
-        run(|| -> () { todo!() }); 
+    fn bar() -> () {
         //~[edition2021]^ unused_unit
+        panic!()
     }
 
     struct UnitStruct;
@@ -150,3 +146,24 @@ mod pr14962 {
     type UnusedParensButNoUnit = Box<dyn (Fn())>;
 }
 
+
+mod issue15035 {
+
+    trait Convert<T> {
+        fn from(value: T) -> Self;
+    }
+
+    impl Convert<u64> for () {
+        fn from(_value: u64) -> Self {}
+    }
+
+    fn handle<T: Convert<u64>>(value: u64) -> T {
+        Convert::from(value)
+    }
+
+    pub fn f() -> Option<bool> {
+        let result: Result<bool, u64> = Err(42);
+        // the `-> ()` is required for the inference of `handle`'s return type
+        result.map_err(|err| -> () { handle(err) }).ok()
+    }
+}

tests/ui/unused_unit.rs

@@ -127,14 +127,10 @@ mod issue14577 {
     trait Unit {}
     impl Unit for () {}
 
-    fn run<R: Unit>(f: impl FnOnce() -> R) {
-        f();
-    }
-
     #[allow(dependency_on_unit_never_type_fallback)]
-    fn bar() {
-        run(|| -> () { todo!() }); 
+    fn bar() -> () {
         //~[edition2021]^ unused_unit
+        panic!()
     }
 
     struct UnitStruct;
@@ -150,3 +146,24 @@ mod pr14962 {
     type UnusedParensButNoUnit = Box<dyn (Fn())>;
 }
 
+
+mod issue15035 {
+
+    trait Convert<T> {
+        fn from(value: T) -> Self;
+    }
+
+    impl Convert<u64> for () {
+        fn from(_value: u64) -> Self {}
+    }
+
+    fn handle<T: Convert<u64>>(value: u64) -> T {
+        Convert::from(value)
+    }
+
+    pub fn f() -> Option<bool> {
+        let result: Result<bool, u64> = Err(42);
+        // the `-> ()` is required for the inference of `handle`'s return type
+        result.map_err(|err| -> () { handle(err) }).ok()
+    }
+}