Allow more complex expressions in let_unit_value

Author: Jarcho

clippy_lints/src/unit_types/let_unit_value.rs

@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_with_macro_callsite;
+use clippy_utils::visitors::for_each_value_source;
 use core::ops::ControlFlow;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, PatKind, Stmt, StmtKind};
@@ -11,11 +12,18 @@ use super::LET_UNIT_VALUE;
 
 pub(super) fn check(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
     if let StmtKind::Local(local) = stmt.kind
+        && let Some(init) = local.init
         && !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)) {
+        let needs_inferred = for_each_value_source(init, &mut |e| if needs_inferred_result_ty(cx, e) {
+            ControlFlow::Continue(())
+        } else {
+            ControlFlow::Break(())
+        }).is_continue();
+
+        if needs_inferred {
             if !matches!(local.pat.kind, PatKind::Wild) {
                 span_lint_and_then(
                     cx,

clippy_utils/src/visitors.rs

@@ -1,4 +1,5 @@
 use crate::path_to_local_id;
+use core::ops::ControlFlow;
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor};
@@ -370,3 +371,36 @@ pub fn is_expr_unsafe<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
     v.visit_expr(e);
     v.is_unsafe
 }
+
+/// Runs the given function for each sub expression producing the final value consumed the parent of
+/// the give expression.
+///
+/// e.g. for the following expression
+/// ```rust,ignore
+/// if foo {
+///     f(0)
+/// } else {
+///     1 + 1
+/// }
+/// ```
+/// this will pass both `f(0)` and `1+1` to the given function.
+pub fn for_each_value_source<'tcx, B>(
+    e: &'tcx Expr<'tcx>,
+    f: &mut impl FnMut(&'tcx Expr<'tcx>) -> ControlFlow<B>,
+) -> ControlFlow<B> {
+    match e.kind {
+        ExprKind::Block(Block { expr: Some(e), .. }, _) => for_each_value_source(e, f),
+        ExprKind::Match(_, arms, _) => {
+            for arm in arms {
+                for_each_value_source(arm.body, f)?;
+            }
+            ControlFlow::Continue(())
+        },
+        ExprKind::If(_, if_expr, Some(else_expr)) => {
+            for_each_value_source(if_expr, f)?;
+            for_each_value_source(else_expr, f)
+        },
+        ExprKind::DropTemps(e) => for_each_value_source(e, f),
+        _ => f(e),
+    }
+}

tests/ui/let_unit.fixed

@@ -87,4 +87,29 @@ fn _returns_generic() {
 
     f4(vec![()]); // Lint
     f4(vec![()]); // Lint
+
+    // Ok
+    let _: () = {
+        let x = 5;
+        f2(x)
+    };
+
+    let _: () = if true { f() } else { f2(0) }; // Ok
+    let _: () = if true { f() } else { f2(0) }; // Lint
+
+    // Ok
+    let _: () = match Some(0) {
+        None => f2(1),
+        Some(0) => f(),
+        Some(1) => f2(3),
+        Some(_) => f2('x'),
+    };
+
+    // Lint
+    match Some(0) {
+        None => f2(1),
+        Some(0) => f(),
+        Some(1) => f2(3),
+        Some(_) => (),
+    };
 }

tests/ui/let_unit.rs

@@ -87,4 +87,29 @@ fn _returns_generic() {
 
     let _: () = f4(vec![()]); // Lint
     let x: () = f4(vec![()]); // Lint
+
+    // Ok
+    let _: () = {
+        let x = 5;
+        f2(x)
+    };
+
+    let _: () = if true { f() } else { f2(0) }; // Ok
+    let x: () = if true { f() } else { f2(0) }; // Lint
+
+    // Ok
+    let _: () = match Some(0) {
+        None => f2(1),
+        Some(0) => f(),
+        Some(1) => f2(3),
+        Some(_) => f2('x'),
+    };
+
+    // Lint
+    let _: () = match Some(0) {
+        None => f2(1),
+        Some(0) => f(),
+        Some(1) => f2(3),
+        Some(_) => (),
+    };
 }

tests/ui/let_unit.stderr

@@ -74,5 +74,34 @@ error: this let-binding has unit value
 LL |     let x: () = f4(vec![()]); // Lint
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `f4(vec![()]);`
 
-error: aborting due to 9 previous errors
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:98:5
+   |
+LL |     let x: () = if true { f() } else { f2(0) }; // Lint
+   |     ^^^^-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         |
+   |         help: use a wild (`_`) binding: `_`
+
+error: this let-binding has unit value
+  --> $DIR/let_unit.rs:109:5
+   |
+LL | /     let _: () = match Some(0) {
+LL | |         None => f2(1),
+LL | |         Some(0) => f(),
+LL | |         Some(1) => f2(3),
+LL | |         Some(_) => (),
+LL | |     };
+   | |______^
+   |
+help: omit the `let` binding
+   |
+LL ~     match Some(0) {
+LL +         None => f2(1),
+LL +         Some(0) => f(),
+LL +         Some(1) => f2(3),
+LL +         Some(_) => (),
+LL +     };
+   |
+
+error: aborting due to 11 previous errors
 

tests/ui/panicking_macros.rs

@@ -1,4 +1,4 @@
-#![allow(clippy::assertions_on_constants, clippy::eq_op)]
+#![allow(clippy::assertions_on_constants, clippy::eq_op, clippy::let_unit_value)]
 #![feature(inline_const)]
 #![warn(clippy::unimplemented, clippy::unreachable, clippy::todo, clippy::panic)]