about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2024-12-31 17:33:45 +0000
committerEsteban Küber <esteban@kuber.com.ar>2025-01-11 01:34:23 +0000
commitec98df4bb6839d017911444499557ecb73e39a00 (patch)
tree0fa36a3dd70adf4ba55e383de9a7b3ca6cfce3cc
parentc2ae386c851b9f85439375dc788a506e2482ca60 (diff)
downloadrust-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.ftl3
-rw-r--r--compiler/rustc_passes/src/errors.rs19
-rw-r--r--compiler/rustc_passes/src/liveness.rs81
-rw-r--r--tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr7
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