Replace elided variable in let_unit with () when used

Situation: `let_unit` lints when an expression binds a unit (`()`)
to a variable. In some cases this binding may be passed down to
another function. Currently, the lint removes the binding without
considering usage.

Change: All usages of the elided variable are now replaced with `()`.

fixes: #12594
This commit is contained in:
Quinn Sinclair 2024-03-31 17:53:13 +02:00
parent 124e68bef8
commit eee4db928f
4 changed files with 100 additions and 3 deletions

View file

@ -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<rustc_span::Span>,
}
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.