diff --git a/clippy_lints/src/unit_types/let_unit_value.rs b/clippy_lints/src/unit_types/let_unit_value.rs index 9406d1607695..d7ab40c4b665 100644 --- a/clippy_lints/src/unit_types/let_unit_value.rs +++ b/clippy_lints/src/unit_types/let_unit_value.rs @@ -1,9 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_with_context; -use clippy_utils::visitors::{for_each_local_assignment, for_each_value_source}; +use clippy_utils::visitors::{for_each_local_assignment, for_each_value_source, is_local_used}; use core::ops::ControlFlow; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; +use rustc_hir::intravisit::{walk_body, Visitor}; use rustc_hir::{Expr, ExprKind, HirId, HirIdSet, Local, MatchSource, Node, PatKind, QPath, TyKind}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::{in_external_macro, is_from_async_await}; @@ -11,7 +12,7 @@ use rustc_middle::ty; use super::LET_UNIT_VALUE; -pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { // skip `let () = { ... }` if let PatKind::Tuple(fields, ..) = local.pat.kind && fields.is_empty() @@ -75,12 +76,53 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>) { let snip = snippet_with_context(cx, expr.span, local.span.ctxt(), "()", &mut app).0; diag.span_suggestion(local.span, "omit the `let` binding", format!("{snip};"), app); } + + if let PatKind::Binding(_, binding_hir_id, ident, ..) = local.pat.kind + && let Some(body_id) = cx.enclosing_body.as_ref() + && let body = cx.tcx.hir().body(*body_id) + && is_local_used(cx, body, binding_hir_id) + { + let identifier = ident.as_str(); + let mut visitor = UnitVariableCollector::new(binding_hir_id); + walk_body(&mut visitor, body); + visitor.spans.into_iter().for_each(|span| { + let msg = + format!("variable `{identifier}` of type `()` can be replaced with explicit `()`"); + diag.span_suggestion(span, msg, "()", Applicability::MachineApplicable); + }); + } }, ); } } } +struct UnitVariableCollector { + id: HirId, + spans: Vec, +} + +impl UnitVariableCollector { + fn new(id: HirId) -> Self { + Self { id, spans: vec![] } + } +} + +/** + * Collect all instances where a variable is used based on its `HirId`. + */ +impl<'tcx> Visitor<'tcx> for UnitVariableCollector { + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> Self::Result { + if let ExprKind::Path(QPath::Resolved(None, path)) = ex.kind + && let Res::Local(id) = path.res + && id == self.id + { + self.spans.push(path.span); + } + rustc_hir::intravisit::walk_expr(self, ex); + } +} + /// Checks sub-expressions which create the value returned by the given expression for whether /// return value inference is needed. This checks through locals to see if they also need inference /// at this point. diff --git a/tests/ui/let_unit.fixed b/tests/ui/let_unit.fixed index 4d41b5e5e503..20940daffa7a 100644 --- a/tests/ui/let_unit.fixed +++ b/tests/ui/let_unit.fixed @@ -177,3 +177,21 @@ async fn issue10433() { } pub async fn issue11502(a: ()) {} + +pub fn issue12594() { + fn returns_unit() {} + + fn returns_result(res: T) -> Result { + Ok(res) + } + + fn actual_test() { + // create first a unit value'd value + returns_unit(); + returns_result(()).unwrap(); + returns_result(()).unwrap(); + // make sure we replace only the first variable + let res = 1; + returns_result(res).unwrap(); + } +} diff --git a/tests/ui/let_unit.rs b/tests/ui/let_unit.rs index daa660be25e6..dca66f2e3edb 100644 --- a/tests/ui/let_unit.rs +++ b/tests/ui/let_unit.rs @@ -177,3 +177,21 @@ async fn issue10433() { } pub async fn issue11502(a: ()) {} + +pub fn issue12594() { + fn returns_unit() {} + + fn returns_result(res: T) -> Result { + Ok(res) + } + + fn actual_test() { + // create first a unit value'd value + let res = returns_unit(); + returns_result(res).unwrap(); + returns_result(res).unwrap(); + // make sure we replace only the first variable + let res = 1; + returns_result(res).unwrap(); + } +} diff --git a/tests/ui/let_unit.stderr b/tests/ui/let_unit.stderr index 0f1f3d782234..aafb77bcd0d6 100644 --- a/tests/ui/let_unit.stderr +++ b/tests/ui/let_unit.stderr @@ -51,5 +51,24 @@ LL + Some(_) => (), LL + }; | -error: aborting due to 3 previous errors +error: this let-binding has unit value + --> tests/ui/let_unit.rs:190:9 + | +LL | let res = returns_unit(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: omit the `let` binding + | +LL | returns_unit(); + | +help: variable `res` of type `()` can be replaced with explicit `()` + | +LL | returns_result(()).unwrap(); + | ~~ +help: variable `res` of type `()` can be replaced with explicit `()` + | +LL | returns_result(()).unwrap(); + | ~~ + +error: aborting due to 4 previous errors