Skip to content

Commit

Permalink
Allow more complex expressions in let_unit_value
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed Apr 5, 2022
1 parent bb03a78 commit 0c0d3e8
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 2 deletions.
10 changes: 9 additions & 1 deletion clippy_lints/src/unit_types/let_unit_value.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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,
Expand Down
34 changes: 34 additions & 0 deletions clippy_utils/src/visitors.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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),
}
}
25 changes: 25 additions & 0 deletions tests/ui/let_unit.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) => (),
};
}
25 changes: 25 additions & 0 deletions tests/ui/let_unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) => (),
};
}
31 changes: 30 additions & 1 deletion tests/ui/let_unit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 0c0d3e8

Please sign in to comment.