From 57f9f8f883b2621296549d82669299aecd00cc78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 31 Dec 2024 17:03:03 +0000 Subject: [PATCH 1/4] Add test for `mut arg: &Ty` meant to be `arg: &mut Ty` This is a mistake I've seen newcomers make where they want to express an "out" argument. --- ...owed-type-meant-to-be-arg-of-mut-borrow.rs | 20 +++++++ ...-type-meant-to-be-arg-of-mut-borrow.stderr | 60 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs create mode 100644 tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs new file mode 100644 index 0000000000000..eff4745e4633c --- /dev/null +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs @@ -0,0 +1,20 @@ +#![deny(unused_assignments, unused_variables)] +struct Object; + +fn change_object(mut object: &Object) { + let object2 = Object; + object = object2; //~ ERROR mismatched types +} + +fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned to, but never used + let object2 = Object; + object = &object2; + //~^ ERROR `object2` does not live long enough + //~| ERROR value assigned to `object` is never read +} + +fn main() { + let object = Object; + change_object(&object); + change_object2(&object); +} 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 new file mode 100644 index 0000000000000..5ac5a8fa40db6 --- /dev/null +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr @@ -0,0 +1,60 @@ +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: consider borrowing here + | +LL | object = &object2; + | + + +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; + | ^^^^^^ + | + = 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)] + | ^^^^^^^^^^^^^^^^^^ + +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 + | +LL | fn change_object2(mut object: &Object) { + | ^^^^^^ + | + = note: consider using `_object` instead +note: the lint level is defined here + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:29 + | +LL | #![deny(unused_assignments, unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error[E0597]: `object2` does not live long enough + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:14 + | +LL | fn change_object2(mut object: &Object) { + | - let's call the lifetime of this reference `'1` +LL | let object2 = Object; + | ------- binding `object2` declared here +LL | object = &object2; + | ---------^^^^^^^^ + | | | + | | borrowed value does not live long enough + | assignment requires that `object2` is borrowed for `'1` +... +LL | } + | - `object2` dropped here while still borrowed + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0308, E0597. +For more information about an error, try `rustc --explain E0308`. From c2ae386c851b9f85439375dc788a506e2482ca60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 31 Dec 2024 17:13:11 +0000 Subject: [PATCH 2/4] 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. --- compiler/rustc_hir_typeck/src/demand.rs | 105 ++++++++++++++++++ ...-type-meant-to-be-arg-of-mut-borrow.stderr | 8 +- 2 files changed, 110 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 56f7a2c1150ae..557e532ca8c6b 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>, + ) -> 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<'_>, 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 5ac5a8fa40db6..f4e1b599c5ab9 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 @@ -7,10 +7,12 @@ LL | let object2 = Object; LL | object = object2; | ^^^^^^^ expected `&Object`, found `Object` | -help: consider borrowing here +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; | -LL | object = &object2; - | + error: value assigned to `object` is never read --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 From ec98df4bb6839d017911444499557ecb73e39a00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 31 Dec 2024 17:33:45 +0000 Subject: [PATCH 3/4] 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. --- compiler/rustc_passes/messages.ftl | 3 + compiler/rustc_passes/src/errors.rs | 19 ++++- compiler/rustc_passes/src/liveness.rs | 81 ++++++++++++++++++- ...-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 9df396cbf7a8c..3ed600a717f53 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 13da021c6143a..c3043ac60aa69 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, + #[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 b85a987c64119..426899a4d5c6d 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 { + 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, hir_id: HirId, ln: LiveNode, var: Variable) { + fn warn_about_dead_assign( + &self, + spans: Vec, + hir_id: HirId, + ln: LiveNode, + var: Variable, + suggestion: Option, + ) { 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 f4e1b599c5ab9..edb1525c0d597 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 From 4438b3211f32f85145a9d30fcd107206899d722c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 11 Jan 2025 01:58:32 +0000 Subject: [PATCH 4/4] review comments and make test `run-rustfix` --- compiler/rustc_hir_typeck/src/demand.rs | 138 ++++++++---------- ...d-type-meant-to-be-arg-of-mut-borrow.fixed | 22 +++ ...owed-type-meant-to-be-arg-of-mut-borrow.rs | 10 +- ...-type-meant-to-be-arg-of-mut-borrow.stderr | 14 +- 4 files changed, 100 insertions(+), 84 deletions(-) create mode 100644 tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 557e532ca8c6b..367e7c6de9532 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -822,82 +822,72 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &hir::Expr<'_>, error: Option>, ) -> 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 }; + if let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error + && let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind() - // We have a mutable binding in the argument. - let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind else { - return false; - }; + // The difference between the expected and found values is one level of borrowing. + && self.can_eq(self.param_env, *inner, found) - // 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; + // 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) + && rhs.hir_id == expr.hir_id + + // 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 + && let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id) + + // The pattern we have is an fn argument. + && let hir::Node::Param(hir::Param { ty_span, .. }) = + self.tcx.parent_hir_node(pat.hir_id) + && 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() + + // We have a mutable binding in the argument. + && let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind + + // 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, x) = ty.kind + { + // `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty` + Some(( + x.ty.span.shrink_to_lo(), + format!( + "{}mut ", + if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " } + ), + )) + } else { + None + } + }) + .next() + { + 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; + } + false } fn annotate_alternative_method_deref( diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed new file mode 100644 index 0000000000000..914ca1f3a065b --- /dev/null +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed @@ -0,0 +1,22 @@ +//@ run-rustfix +#![deny(unused_assignments, unused_variables)] +struct Object; + +fn change_object(object: &mut Object) { //~ HELP you might have meant to mutate + let object2 = Object; + *object = object2; //~ ERROR mismatched types +} + +fn change_object2(object: &mut Object) { //~ ERROR variable `object` is assigned to, but never used + //~^ HELP you might have meant to mutate + let object2 = Object; + *object = object2; + //~^ ERROR `object2` does not live long enough + //~| ERROR value assigned to `object` is never read +} + +fn main() { + let mut object = Object; + change_object(&mut object); + change_object2(&mut object); +} diff --git a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs index eff4745e4633c..331359a98d147 100644 --- a/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs +++ b/tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs @@ -1,12 +1,14 @@ +//@ run-rustfix #![deny(unused_assignments, unused_variables)] struct Object; -fn change_object(mut object: &Object) { +fn change_object(mut object: &Object) { //~ HELP you might have meant to mutate let object2 = Object; object = object2; //~ ERROR mismatched types } fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned to, but never used + //~^ HELP you might have meant to mutate let object2 = Object; object = &object2; //~^ ERROR `object2` does not live long enough @@ -14,7 +16,7 @@ fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned } fn main() { - let object = Object; - change_object(&object); - change_object2(&object); + let mut object = Object; + change_object(&mut object); + change_object2(&mut object); } 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 edb1525c0d597..e7e4003936a11 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 @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:6:14 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:7:14 | LL | fn change_object(mut object: &Object) { | ------- expected due to this parameter type @@ -15,41 +15,43 @@ LL ~ *object = object2; | error: value assigned to `object` is never read - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:13: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 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:2: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 | 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 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:10:23 | LL | fn change_object2(mut object: &Object) { | ^^^^^^ | = note: consider using `_object` instead note: the lint level is defined here - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:29 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:2:29 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^ error[E0597]: `object2` does not live long enough - --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:14 + --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:13:14 | LL | fn change_object2(mut object: &Object) { | - let's call the lifetime of this reference `'1` +LL | LL | let object2 = Object; | ------- binding `object2` declared here LL | object = &object2;