Don't lint let_unit_value when needed for type inferenece

Author: Jarcho

clippy_lints/src/unit_types/let_unit_value.rs

@@ -1,18 +1,38 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_with_macro_callsite;
+use core::ops::ControlFlow;
 use rustc_errors::Applicability;
-use rustc_hir::{Stmt, StmtKind};
+use rustc_hir::{Expr, ExprKind, PatKind, Stmt, StmtKind};
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::lint::in_external_macro;
+use rustc_middle::ty::{self, Ty, TypeFoldable, TypeVisitor};
 
 use super::LET_UNIT_VALUE;
 
 pub(super) fn check(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
-    if let StmtKind::Local(local) = stmt.kind {
-        if cx.typeck_results().pat_ty(local.pat).is_unit() {
-            if in_external_macro(cx.sess(), stmt.span) || local.pat.span.from_expansion() {
-                return;
+    if let StmtKind::Local(local) = stmt.kind
+        && !local.pat.span.from_expansion()
+        && !in_external_macro(cx.sess(), stmt.span)
+        && cx.typeck_results().pat_ty(local.pat).is_unit()
+    {
+        if local.init.map_or(false, |e| needs_inferred_result_ty(cx, e)) {
+            if !matches!(local.pat.kind, PatKind::Wild) {
+                span_lint_and_then(
+                    cx,
+                    LET_UNIT_VALUE,
+                    stmt.span,
+                    "this let-binding has unit value",
+                    |diag| {
+                            diag.span_suggestion(
+                                local.pat.span,
+                                "use a wild (`_`) binding",
+                                "_".into(),
+                                Applicability::MaybeIncorrect, // snippet
+                            );
+                    },
+                );
             }
+        } else {
             span_lint_and_then(
                 cx,
                 LET_UNIT_VALUE,
@@ -33,3 +53,45 @@ pub(super) fn check(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
         }
     }
 }
+
+fn needs_inferred_result_ty(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
+    let id = match e.kind {
+        ExprKind::Call(
+            Expr {
+                kind: ExprKind::Path(ref path),
+                hir_id,
+                ..
+            },
+            _,
+        ) => cx.qpath_res(path, *hir_id).opt_def_id(),
+        ExprKind::MethodCall(..) => cx.typeck_results().type_dependent_def_id(e.hir_id),
+        _ => return false,
+    };
+    if let Some(id) = id
+        && let sig = cx.tcx.fn_sig(id).skip_binder()
+        && let ty::Param(output_ty) = *sig.output().kind()
+    {
+        sig.inputs().iter().all(|&ty| !ty_contains_param(ty, output_ty.index))
+    } else {
+        false
+    }
+}
+
+fn ty_contains_param(ty: Ty<'_>, index: u32) -> bool {
+    struct Visitor(u32);
+    impl<'tcx> TypeVisitor<'tcx> for Visitor {
+        type BreakTy = ();
+        fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
+            if let ty::Param(ty) = *ty.kind() {
+                if ty.index == self.0 {
+                    ControlFlow::BREAK
+                } else {
+                    ControlFlow::CONTINUE
+                }
+            } else {
+                ty.super_visit_with(self)
+            }
+        }
+    }
+    ty.visit_with(&mut Visitor(index)).is_break()
+}

tests/ui/let_unit.fixed

@@ -61,3 +61,30 @@ fn multiline_sugg() {
 
 #[derive(Copy, Clone)]
 pub struct ContainsUnit(()); // should be fine
+
+fn _returns_generic() {
+    fn f<T>() -> T {
+        unimplemented!()
+    }
+    fn f2<T, U>(_: T) -> U {
+        unimplemented!()
+    }
+    fn f3<T>(x: T) -> T {
+        x
+    }
+    fn f4<T>(mut x: Vec<T>) -> T {
+        x.pop().unwrap()
+    }
+
+    let _: () = f(); // Ok
+    let _: () = f(); // Lint.
+
+    let _: () = f2(0i32); // Ok
+    let _: () = f2(0i32); // Lint.
+
+    f3(()); // Lint
+    f3(()); // Lint
+
+    f4(vec![()]); // Lint
+    f4(vec![()]); // Lint
+}

tests/ui/let_unit.rs

@@ -61,3 +61,30 @@ fn multiline_sugg() {
 
 #[derive(Copy, Clone)]
 pub struct ContainsUnit(()); // should be fine
+
+fn _returns_generic() {
+    fn f<T>() -> T {
+        unimplemented!()
+    }
+    fn f2<T, U>(_: T) -> U {
+        unimplemented!()
+    }
+    fn f3<T>(x: T) -> T {
+        x
+    }
+    fn f4<T>(mut x: Vec<T>) -> T {
+        x.pop().unwrap()
+    }
+
+    let _: () = f(); // Ok
+    let x: () = f(); // Lint.
+
+    let _: () = f2(0i32); // Ok
+    let x: () = f2(0i32); // Lint.
+
+    let _: () = f3(()); // Lint
+    let x: () = f3(()); // Lint
+
+    let _: () = f4(vec![()]); // Lint
+    let x: () = f4(vec![()]); // Lint
+}

tests/ui/let_unit.stderr

@@ -34,5 +34,45 @@ LL +         .map(|_| ())
 LL +         .next()
  ...
 
-error: aborting due to 3 previous errors
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:80:5
+   |
+LL |     let x: () = f(); // Lint.
+   |     ^^^^-^^^^^^^^^^^
+   |         |
+   |         help: use a wild (`_`) binding: `_`
+
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:83:5
+   |
+LL |     let x: () = f2(0i32); // Lint.
+   |     ^^^^-^^^^^^^^^^^^^^^^
+   |         |
+   |         help: use a wild (`_`) binding: `_`
+
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:85:5
+   |
+LL |     let _: () = f3(()); // Lint
+   |     ^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f3(());`
+
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:86:5
+   |
+LL |     let x: () = f3(()); // Lint
+   |     ^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f3(());`
+
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:88:5
+   |
+LL |     let _: () = f4(vec![()]); // Lint
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f4(vec![()]);`
+
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:89:5
+   |
+LL |     let x: () = f4(vec![()]); // Lint
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f4(vec![()]);`
+
+error: aborting due to 9 previous errors