about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2024-12-31 17:13:11 +0000
committerEsteban Küber <esteban@kuber.com.ar>2025-01-11 01:34:23 +0000
commitc2ae386c851b9f85439375dc788a506e2482ca60 (patch)
tree4ad1ade571b635d09670aa0053d8fbd6ac5ae4ba /compiler
parent57f9f8f883b2621296549d82669299aecd00cc78 (diff)
downloadrust-c2ae386c851b9f85439375dc788a506e2482ca60.tar.gz
rust-c2ae386c851b9f85439375dc788a506e2482ca60.zip
On E0308, detect `mut arg: &Ty` meant to be `arg: &mut Ty`
```
error[E0308]: mismatched types
  --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:6:14
   |
LL | fn change_object(mut object: &Object) {
   |                              ------- expected due to this parameter type
LL |     let object2 = Object;
LL |     object = object2;
   |              ^^^^^^^ expected `&Object`, found `Object`
   |
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_object(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. We avoid suggesting `object = &object2;`, as that is less likely to be what was intended.
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_hir_typeck/src/demand.rs105
1 files changed, 105 insertions, 0 deletions
diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs
index 56f7a2c1150..557e532ca8c 100644
--- a/compiler/rustc_hir_typeck/src/demand.rs
+++ b/compiler/rustc_hir_typeck/src/demand.rs
@@ -85,6 +85,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         self.annotate_expected_due_to_let_ty(err, expr, error);
         self.annotate_loop_expected_due_to_inference(err, expr, error);
+        if self.annotate_mut_binding_to_immutable_binding(err, expr, error) {
+            return;
+        }
 
         // FIXME(#73154): For now, we do leak check when coercing function
         // pointers in typeck, instead of only during borrowck. This can lead
@@ -795,6 +798,108 @@ impl<'a, 'tcx> FnCtxt<'a, '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,
+        err: &mut Diag<'_>,
+        expr: &hir::Expr<'_>,
+        error: Option<TypeError<'tcx>>,
+    ) -> bool {
+        let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error else { return false };
+        let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind() else { return false };
+        if !self.can_eq(self.param_env, *inner, found) {
+            // The difference between the expected and found values isn't one level of borrowing.
+            return false;
+        }
+        // We have an `ident = expr;` assignment.
+        let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(lhs, rhs, _), .. }) =
+            self.tcx.parent_hir_node(expr.hir_id)
+        else {
+            return false;
+        };
+        if rhs.hir_id != expr.hir_id || expected.is_closure() {
+            return false;
+        }
+        // We are assigning to some binding.
+        let hir::ExprKind::Path(hir::QPath::Resolved(
+            None,
+            hir::Path { res: hir::def::Res::Local(hir_id), .. },
+        )) = lhs.kind
+        else {
+            return false;
+        };
+        let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id) else { return false };
+        // The pattern we have is an fn argument.
+        let hir::Node::Param(hir::Param { ty_span, .. }) = self.tcx.parent_hir_node(pat.hir_id)
+        else {
+            return false;
+        };
+        let item = self.tcx.hir().get_parent_item(pat.hir_id);
+        let item = self.tcx.hir_owner_node(item);
+        let Some(fn_decl) = item.fn_decl() else { return false };
+
+        // We have a mutable binding in the argument.
+        let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind else {
+            return false;
+        };
+
+        // Look for the type corresponding to the argument pattern we have in the argument list.
+        let Some(ty_sugg) = 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(),
+                        format!(
+                            "{}mut ",
+                            if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " }
+                        ),
+                    ))
+                } else {
+                    None
+                }
+            })
+            .next()
+        else {
+            return false;
+        };
+        let sugg = vec![
+            ty_sugg,
+            (pat.span.until(ident.span), String::new()),
+            (lhs.span.shrink_to_lo(), "*".to_string()),
+        ];
+        // We suggest changing the argument from `mut ident: &Ty` to `ident: &'_ mut Ty` and the
+        // assignment from `ident = val;` to `*ident = val;`.
+        err.multipart_suggestion_verbose(
+            "you might have meant to mutate the pointed at value being passed in, instead of \
+             changing the reference in the local binding",
+            sugg,
+            Applicability::MaybeIncorrect,
+        );
+        return true;
+    }
+
     fn annotate_alternative_method_deref(
         &self,
         err: &mut Diag<'_>,