diff options
| author | Esteban Küber <esteban@kuber.com.ar> | 2024-12-31 17:33:45 +0000 |
|---|---|---|
| committer | Esteban Küber <esteban@kuber.com.ar> | 2025-01-11 01:34:23 +0000 |
| commit | ec98df4bb6839d017911444499557ecb73e39a00 (patch) | |
| tree | 0fa36a3dd70adf4ba55e383de9a7b3ca6cfce3cc | |
| parent | c2ae386c851b9f85439375dc788a506e2482ca60 (diff) | |
| download | rust-ec98df4bb6839d017911444499557ecb73e39a00.tar.gz rust-ec98df4bb6839d017911444499557ecb73e39a00.zip | |
On unused assign lint, detect `mut arg: &Ty` meant to be `arg: &mut Ty`
```
error: value assigned to `object` is never read
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5
|
LL | object = &object2;
| ^^^^^^
|
note: the lint level is defined here
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9
|
LL | #![deny(unused_assignments, unused_variables)]
| ^^^^^^^^^^^^^^^^^^
help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding
|
LL ~ fn change_object2(object: &mut Object) {
LL | let object2 = Object;
LL ~ *object = object2;
|
```
This might be the first thing someone tries to write to mutate the value *behind* an argument, trying to avoid an E0308.
| -rw-r--r-- | compiler/rustc_passes/messages.ftl | 3 | ||||
| -rw-r--r-- | compiler/rustc_passes/src/errors.rs | 19 | ||||
| -rw-r--r-- | compiler/rustc_passes/src/liveness.rs | 81 | ||||
| -rw-r--r-- | tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr | 7 |
4 files changed, 104 insertions, 6 deletions
diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 9df396cbf7a..3ed600a717f 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -799,6 +799,9 @@ passes_unused_assign = value assigned to `{$name}` is never read passes_unused_assign_passed = value passed to `{$name}` is never read .help = maybe it is overwritten before being read? +passes_unused_assign_suggestion = + you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding + passes_unused_capture_maybe_capture_ref = value captured by `{$name}` is never read .help = did you mean to capture by reference instead? diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 13da021c614..c3043ac60aa 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1787,9 +1787,26 @@ pub(crate) struct IneffectiveUnstableImpl; #[derive(LintDiagnostic)] #[diag(passes_unused_assign)] -#[help] pub(crate) struct UnusedAssign { pub name: String, + #[subdiagnostic] + pub suggestion: Option<UnusedAssignSuggestion>, + #[help] + pub help: bool, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(passes_unused_assign_suggestion, applicability = "maybe-incorrect")] +pub(crate) struct UnusedAssignSuggestion { + pub pre: &'static str, + #[suggestion_part(code = "{pre}mut ")] + pub ty_span: Span, + #[suggestion_part(code = "")] + pub ty_ref_span: Span, + #[suggestion_part(code = "*")] + pub ident_span: Span, + #[suggestion_part(code = "")] + pub expr_ref_span: Span, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index b85a987c641..426899a4d5c 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -1360,7 +1360,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { fn visit_local(&mut self, local: &'tcx hir::LetStmt<'tcx>) { self.check_unused_vars_in_pat(local.pat, None, None, |spans, hir_id, ln, var| { if local.init.is_some() { - self.warn_about_dead_assign(spans, hir_id, ln, var); + self.warn_about_dead_assign(spans, hir_id, ln, var, None); } }); @@ -1460,7 +1460,8 @@ impl<'tcx> Liveness<'_, 'tcx> { // as being used. let ln = self.live_node(expr.hir_id, expr.span); let var = self.variable(var_hid, expr.span); - self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var); + let sugg = self.annotate_mut_binding_to_immutable_binding(var_hid, expr); + self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var, sugg); } } _ => { @@ -1585,6 +1586,70 @@ impl<'tcx> Liveness<'_, 'tcx> { } } + /// Detect the following case + /// + /// ```text + /// fn change_object(mut a: &Ty) { + /// let a = Ty::new(); + /// b = &a; + /// } + /// ``` + /// + /// where the user likely meant to modify the value behind there reference, use `a` as an out + /// parameter, instead of mutating the local binding. When encountering this we suggest: + /// + /// ```text + /// fn change_object(a: &'_ mut Ty) { + /// let a = Ty::new(); + /// *b = a; + /// } + /// ``` + fn annotate_mut_binding_to_immutable_binding( + &self, + var_hid: HirId, + expr: &'tcx Expr<'tcx>, + ) -> Option<errors::UnusedAssignSuggestion> { + if let hir::Node::Expr(parent) = self.ir.tcx.parent_hir_node(expr.hir_id) + && let hir::ExprKind::Assign(_, rhs, _) = parent.kind + && let hir::ExprKind::AddrOf(borrow_kind, _mut, inner) = rhs.kind + && let hir::BorrowKind::Ref = borrow_kind + && let hir::Node::Pat(pat) = self.ir.tcx.hir_node(var_hid) + && let hir::Node::Param(hir::Param { ty_span, .. }) = + self.ir.tcx.parent_hir_node(pat.hir_id) + && let item_id = self.ir.tcx.hir().get_parent_item(pat.hir_id) + && let item = self.ir.tcx.hir_owner_node(item_id) + && let Some(fn_decl) = item.fn_decl() + && let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind + && let Some((ty_span, pre)) = fn_decl + .inputs + .iter() + .filter_map(|ty| { + if ty.span == *ty_span + && let hir::TyKind::Ref(lt, mut_ty) = ty.kind + { + // `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty` + Some(( + mut_ty.ty.span.shrink_to_lo(), + if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " }, + )) + } else { + None + } + }) + .next() + { + Some(errors::UnusedAssignSuggestion { + ty_span, + pre, + ty_ref_span: pat.span.until(ident.span), + ident_span: expr.span.shrink_to_lo(), + expr_ref_span: rhs.span.until(inner.span), + }) + } else { + None + } + } + #[instrument(skip(self), level = "INFO")] fn report_unused( &self, @@ -1738,15 +1803,23 @@ impl<'tcx> Liveness<'_, 'tcx> { suggs } - fn warn_about_dead_assign(&self, spans: Vec<Span>, hir_id: HirId, ln: LiveNode, var: Variable) { + fn warn_about_dead_assign( + &self, + spans: Vec<Span>, + hir_id: HirId, + ln: LiveNode, + var: Variable, + suggestion: Option<errors::UnusedAssignSuggestion>, + ) { if !self.live_on_exit(ln, var) && let Some(name) = self.should_warn(var) { + let help = suggestion.is_none(); self.ir.tcx.emit_node_span_lint( lint::builtin::UNUSED_ASSIGNMENTS, hir_id, spans, - errors::UnusedAssign { name }, + errors::UnusedAssign { name, suggestion, help }, ); } } diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr index f4e1b599c5a..edb1525c0d5 100644 --- a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr @@ -20,12 +20,17 @@ error: value assigned to `object` is never read LL | object = &object2; | ^^^^^^ | - = help: maybe it is overwritten before being read? note: the lint level is defined here --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^^^ +help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding + | +LL ~ fn change_object2(object: &mut Object) { +LL | let object2 = Object; +LL ~ *object = object2; + | error: variable `object` is assigned to, but never used --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:9:23 |
