From 97bf7b934e740b8b94aaf0e75a17f995444e44b6 Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Sat, 4 Dec 2021 18:57:31 +1100 Subject: [PATCH] Improve suggestion to change struct field to &mut --- .../src/diagnostics/mutability_errors.rs | 39 +++++++------------ src/test/ui/did_you_mean/issue-38147-2.rs | 7 +++- src/test/ui/did_you_mean/issue-38147-2.stderr | 23 ++++++++--- src/test/ui/did_you_mean/issue-38147-3.stderr | 8 ++-- 4 files changed, 42 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 0434c1ba59a..86e577fdf1d 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -229,15 +229,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } => { err.span_label(span, format!("cannot {ACT}", ACT = act)); - if let Some((span, message)) = annotate_struct_field( + if let Some(span) = get_mut_span_in_struct_field( self.infcx.tcx, Place::ty_from(local, proj_base, self.body, self.infcx.tcx).ty, field, ) { - err.span_suggestion( + err.span_suggestion_verbose( span, "consider changing this to be mutable", - message, + "mut ".into(), Applicability::MaybeIncorrect, ); } @@ -1059,18 +1059,18 @@ fn is_closure_or_generator(ty: Ty<'_>) -> bool { ty.is_closure() || ty.is_generator() } -/// Adds a suggestion to a struct definition given a field access to a local. -/// This function expects the local to be a reference to a struct in order to produce a suggestion. +/// Given a field that needs to be mutuable, returns a span where the mut could go. +/// This function expects the local to be a reference to a struct in order to produce a span. /// /// ```text /// LL | s: &'a String -/// | ---------- use `&'a mut String` here to make mutable +/// | ^ returns a span pointing here /// ``` -fn annotate_struct_field<'tcx>( +fn get_mut_span_in_struct_field<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, field: &mir::Field, -) -> Option<(Span, String)> { +) -> Option { // Expect our local to be a reference to a struct of some kind. if let ty::Ref(_, ty, _) = ty.kind() { if let ty::Adt(def, _) = ty.kind() { @@ -1081,25 +1081,12 @@ fn annotate_struct_field<'tcx>( // Now we're dealing with the actual struct that we're going to suggest a change to, // we can expect a field that is an immutable reference to a type. if let hir::Node::Field(field) = node { - if let hir::TyKind::Rptr( - lifetime, - hir::MutTy { mutbl: hir::Mutability::Not, ref ty }, - ) = field.ty.kind + if let hir::TyKind::Rptr(lifetime, hir::MutTy { mutbl: hir::Mutability::Not, .. }) = + field.ty.kind { - // Get the snippets in two parts - the named lifetime (if there is one) and - // type being referenced, that way we can reconstruct the snippet without loss - // of detail. - let type_snippet = tcx.sess.source_map().span_to_snippet(ty.span).ok()?; - let lifetime_snippet = if !lifetime.is_elided() { - format!("{} ", tcx.sess.source_map().span_to_snippet(lifetime.span).ok()?) - } else { - String::new() - }; - - return Some(( - field.ty.span, - format!("&{}mut {}", lifetime_snippet, &*type_snippet,), - )); + return Some( + lifetime.span.with_hi(lifetime.span.hi() + BytePos(1)).shrink_to_hi(), + ); } } } diff --git a/src/test/ui/did_you_mean/issue-38147-2.rs b/src/test/ui/did_you_mean/issue-38147-2.rs index fe2634d88ab..154b149b720 100644 --- a/src/test/ui/did_you_mean/issue-38147-2.rs +++ b/src/test/ui/did_you_mean/issue-38147-2.rs @@ -1,11 +1,16 @@ struct Bar<'a> { - s: &'a String + s: &'a String, + // use wonky spaces to ensure we are creating the span correctly + longer_name: & 'a Vec } impl<'a> Bar<'a> { fn f(&mut self) { self.s.push('x'); //~^ ERROR cannot borrow `*self.s` as mutable, as it is behind a `&` reference + + self.longer_name.push(13); + //~^ ERROR cannot borrow `*self.longer_name` as mutable, as it is behind a `&` reference } } diff --git a/src/test/ui/did_you_mean/issue-38147-2.stderr b/src/test/ui/did_you_mean/issue-38147-2.stderr index 8bf5c76977d..5ff97eacc23 100644 --- a/src/test/ui/did_you_mean/issue-38147-2.stderr +++ b/src/test/ui/did_you_mean/issue-38147-2.stderr @@ -1,12 +1,25 @@ error[E0596]: cannot borrow `*self.s` as mutable, as it is behind a `&` reference - --> $DIR/issue-38147-2.rs:7:9 + --> $DIR/issue-38147-2.rs:9:9 | -LL | s: &'a String - | ---------- help: consider changing this to be mutable: `&'a mut String` -... LL | self.s.push('x'); | ^^^^^^^^^^^^^^^^ cannot borrow as mutable + | +help: consider changing this to be mutable + | +LL | s: &'a mut String, + | +++ -error: aborting due to previous error +error[E0596]: cannot borrow `*self.longer_name` as mutable, as it is behind a `&` reference + --> $DIR/issue-38147-2.rs:12:9 + | +LL | self.longer_name.push(13); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable + | +help: consider changing this to be mutable + | +LL | longer_name: & 'a mut Vec + | +++ + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/did_you_mean/issue-38147-3.stderr b/src/test/ui/did_you_mean/issue-38147-3.stderr index 0e1e42261c4..94ffe17f101 100644 --- a/src/test/ui/did_you_mean/issue-38147-3.stderr +++ b/src/test/ui/did_you_mean/issue-38147-3.stderr @@ -1,11 +1,13 @@ error[E0596]: cannot borrow `*self.s` as mutable, as it is behind a `&` reference --> $DIR/issue-38147-3.rs:7:9 | -LL | s: &'a String - | ---------- help: consider changing this to be mutable: `&'a mut String` -... LL | self.s.push('x'); | ^^^^^^^^^^^^^^^^ cannot borrow as mutable + | +help: consider changing this to be mutable + | +LL | s: &'a mut String + | +++ error: aborting due to previous error