]> git.lizzy.rs Git - rust.git/commitdiff
Allow more complex expressions in `let_unit_value`
authorJason Newcomb <jsnewcomb@pm.me>
Tue, 5 Apr 2022 01:54:34 +0000 (21:54 -0400)
committerJason Newcomb <jsnewcomb@pm.me>
Fri, 15 Apr 2022 01:34:33 +0000 (21:34 -0400)
clippy_lints/src/unit_types/let_unit_value.rs
clippy_utils/src/visitors.rs
tests/ui/cast_alignment.rs
tests/ui/let_unit.fixed
tests/ui/let_unit.rs
tests/ui/let_unit.stderr
tests/ui/or_then_unwrap.fixed
tests/ui/or_then_unwrap.rs
tests/ui/panicking_macros.rs

index 39352b3ee4769ac8dd2dcc5650ef4b8686808bc4..96858fc7f274955d4bbeced6f0a9a6dbb9ec0a1e 100644 (file)
@@ -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};
 
 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,
index 3db64b2535398634e26a9824533a35fab18e0dd5..c00bc2bd213f9a4fd3c8469fe16f0273167b1191 100644 (file)
@@ -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};
@@ -402,3 +403,36 @@ fn visit_block(&mut self, b: &'tcx Block<'_>) {
     v.visit_expr(e);
     v.found_unsafe
 }
+
+/// Runs the given function for each sub-expression producing the final value consumed by 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),
+    }
+}
index e4e7290a30e9e711c911c34d96a1239af8e0d19a..95bb883df1bf10877db0fff7a242606284dc55a8 100644 (file)
@@ -44,8 +44,8 @@ fn main() {
         let _ = core::ptr::read_unaligned(ptr as *const u16);
         let _ = core::intrinsics::unaligned_volatile_load(ptr as *const u16);
         let ptr = &mut data as *mut [u8; 2] as *mut u8;
-        let _ = (ptr as *mut u16).write_unaligned(0);
-        let _ = core::ptr::write_unaligned(ptr as *mut u16, 0);
-        let _ = core::intrinsics::unaligned_volatile_store(ptr as *mut u16, 0);
+        (ptr as *mut u16).write_unaligned(0);
+        core::ptr::write_unaligned(ptr as *mut u16, 0);
+        core::intrinsics::unaligned_volatile_store(ptr as *mut u16, 0);
     }
 }
index 5fee742b19248fbf9d8386a097ca7e0dd7bd661d..e72b746232551b87861e2783cf5ce3d7d76dfd21 100644 (file)
@@ -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(_) => (),
+    };
 }
index 505e4a7d8fdd575a7e40088c4ef1de0adf1a9368..47ee0a76724792445b8da0bd6465355b43fafb73 100644 (file)
@@ -87,4 +87,29 @@ fn f4<T>(mut x: Vec<T>) -> T {
 
     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(_) => (),
+    };
 }
index c4a766bb974403d60277db4449c326ec924c6779..13ec11a6d33e9a9c12cc06f9a8e37facaab385f3 100644 (file)
@@ -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
 
index 6e0d5a87f6807b012fcc55428b5b0aee61882301..844cc4b7a09281ebaa23604078f83735625b17c7 100644 (file)
@@ -1,7 +1,7 @@
 // run-rustfix
 
 #![warn(clippy::or_then_unwrap)]
-#![allow(clippy::map_identity)]
+#![allow(clippy::map_identity, clippy::let_unit_value)]
 
 struct SomeStruct;
 impl SomeStruct {
index e406a71d46d00d377024971cee471851e0c3a063..1528ef9be964de41ba522ee5fef9930ca3a159bb 100644 (file)
@@ -1,7 +1,7 @@
 // run-rustfix
 
 #![warn(clippy::or_then_unwrap)]
-#![allow(clippy::map_identity)]
+#![allow(clippy::map_identity, clippy::let_unit_value)]
 
 struct SomeStruct;
 impl SomeStruct {
index 12a0c776ae2fd6feb90371be57be32ddd59379de..041ef17fa6834240c97e3234601e8432f3f0ff0a 100644 (file)
@@ -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)]