diff --git a/clippy_lints/src/unit_types/let_unit_value.rs b/clippy_lints/src/unit_types/let_unit_value.rs index b25a6e3375bb..39352b3ee476 100644 --- a/clippy_lints/src/unit_types/let_unit_value.rs +++ b/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 { + 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() +} diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed index f398edc23cb5..5fee742b1924 100644 --- a/tests/ui/let_unit.fixed +++ b/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 { + unimplemented!() + } + fn f2(_: T) -> U { + unimplemented!() + } + fn f3(x: T) -> T { + x + } + fn f4(mut x: Vec) -> 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 +} diff --git a/tests/ui/let_unit.rs b/tests/ui/let_unit.rs index af5b1fb2ac7e..505e4a7d8fdd 100644 --- a/tests/ui/let_unit.rs +++ b/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 { + unimplemented!() + } + fn f2(_: T) -> U { + unimplemented!() + } + fn f3(x: T) -> T { + x + } + fn f4(mut x: Vec) -> 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 +} diff --git a/tests/ui/let_unit.stderr b/tests/ui/let_unit.stderr index f2600c6c2288..c4a766bb9744 100644 --- a/tests/ui/let_unit.stderr +++ b/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