Do not use a suggestion to change a binding's name to a type

rust-lang · bors · Jun 29, 2022 · Jun 6, 2022 · Jun 26, 2022 · Jun 26, 2022
commit 8fd73560b3ba9d970887b36e335de256ca293c80
diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -434,16 +434,16 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
                 match self.local_names[local] {
                     Some(name) if !local_decl.from_compiler_desugaring() => {
-                        let label = match local_decl.local_info.as_ref().unwrap() {
-                            box LocalInfo::User(ClearCrossCrate::Set(
+                        let label = match local_decl.local_info.as_deref().unwrap() {
+                            LocalInfo::User(ClearCrossCrate::Set(
                                 mir::BindingForm::ImplicitSelf(_),
                             )) => {
                                 let (span, suggestion) =
                                     suggest_ampmut_self(self.infcx.tcx, local_decl);
                                 Some((true, span, suggestion))
                             }
 
-                            box LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
+                            LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
                                 mir::VarBindingForm {
                                     binding_mode: ty::BindingMode::BindByValue(_),
                                     opt_ty_info,
@@ -473,20 +473,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                     // on for loops, RHS points to the iterator part
                                     Some(DesugaringKind::ForLoop) => {
                                         self.suggest_similar_mut_method_for_for_loop(&mut err);
-                                        Some((
-                                            false,
-                                            opt_assignment_rhs_span.unwrap(),
-                                            format!(
-                                                "this iterator yields `{SIGIL}` {DESC}s",
-                                                SIGIL = pointer_sigil,
-                                                DESC = pointer_desc
-                                            ),
-                                        ))
+                                        err.span_label(opt_assignment_rhs_span.unwrap(), format!(
+                                            "this iterator yields `{pointer_sigil}` {pointer_desc}s",
+                                        ));
+                                        None
                                     }
                                     // don't create labels for compiler-generated spans
                                     Some(_) => None,
                                     None => {
-                                        let (span, suggestion) = if name != kw::SelfLower {
+                                        let label = if name != kw::SelfLower {
                                             suggest_ampmut(
                                                 self.infcx.tcx,
                                                 local_decl,
@@ -501,7 +496,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                                         ..
                                                     }),
                                                 ))) => {
                                                    suggest_ampmut_self(self.infcx.tcx, local_decl)
+                                                    let (span, sugg) = suggest_ampmut_self(
+                                                        self.infcx.tcx,
+                                                        local_decl,
+                                                    );
+                                                    (true, span, sugg)
                                                 }
                                                 // explicit self (eg `self: &'a Self`)
                                                 _ => suggest_ampmut(
@@ -512,12 +511,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                                 ),
                                             }
                                         };
-                                        Some((true, span, suggestion))
+                                        Some(label)
                                     }
                                 }
                             }
 
-                            box LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
+                            LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
                                 mir::VarBindingForm {
                                     binding_mode: ty::BindingMode::BindByReference(_),
                                     ..
@@ -528,7 +527,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                     .map(|replacement| (true, pattern_span, replacement))
                             }
 
-                            box LocalInfo::User(ClearCrossCrate::Clear) => {
+                            LocalInfo::User(ClearCrossCrate::Clear) => {
                                 bug!("saw cleared local state")
                             }
 
@@ -559,7 +558,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                 }
                             }
                             Some((false, err_label_span, message)) => {
-                                err.span_label(err_label_span, &message);
+                                err.span_label(
+                                    err_label_span,
+                                    &format!(
+                                        "consider changing this binding's type to be: `{message}`"
+                                    ),
+                                );
                             }
                             None => {}
                         }
@@ -1004,7 +1008,7 @@ fn suggest_ampmut<'tcx>(
     local_decl: &mir::LocalDecl<'tcx>,
     opt_assignment_rhs_span: Option<Span>,
     opt_ty_info: Option<Span>,
-) -> (Span, String) {
+) -> (bool, Span, String) {
     if let Some(assignment_rhs_span) = opt_assignment_rhs_span
         && let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span)
     {
@@ -1028,37 +1032,38 @@ fn suggest_ampmut<'tcx>(
             let lt_name = &src[1..ws_pos];
             let ty = src[ws_pos..].trim_start();
             if !is_mutbl(ty) {
-                return (assignment_rhs_span, format!("&{lt_name} mut {ty}"));
+                return (true, assignment_rhs_span, format!("&{lt_name} mut {ty}"));
             }
         } else if let Some(stripped) = src.strip_prefix('&') {
             let stripped = stripped.trim_start();
             if !is_mutbl(stripped) {
-                return (assignment_rhs_span, format!("&mut {stripped}"));
+                return (true, assignment_rhs_span, format!("&mut {stripped}"));
             }
         }
     }
 
-    let highlight_span = match opt_ty_info {
+    let (suggestability, highlight_span) = match opt_ty_info {
         // if this is a variable binding with an explicit type,
         // try to highlight that for the suggestion.
-        Some(ty_span) => ty_span,
+        Some(ty_span) => (true, ty_span),
 
         // otherwise, just highlight the span associated with
         // the (MIR) LocalDecl.
-        None => local_decl.source_info.span,
+        None => (false, local_decl.source_info.span),
     };

     if let Ok(src) = tcx.sess.source_map().span_to_snippet(highlight_span)
         && let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(char::is_whitespace))
     {
         let lt_name = &src[1..ws_pos];
         let ty = &src[ws_pos..];
-        return (highlight_span, format!("&{} mut{}", lt_name, ty));
+        return (true, highlight_span, format!("&{} mut{}", lt_name, ty));
     }
 
     let ty_mut = local_decl.ty.builtin_deref(true).unwrap();
     assert_eq!(ty_mut.mutbl, hir::Mutability::Not);
     (
+        suggestability,
         highlight_span,
         if local_decl.ty.is_region_ptr() {
             format!("&mut {}", ty_mut.ty)

diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs
@@ -162,7 +162,11 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
                 let opt_ty_info;
                 let self_arg;
                 if let Some(ref fn_decl) = tcx.hir().fn_decl_by_hir_id(owner_id) {
-                    opt_ty_info = fn_decl.inputs.get(index).map(|ty| ty.span);
+                    opt_ty_info = fn_decl
+                        .inputs
+                        .get(index)
+                        // Make sure that inferred closure args have no type span
+                        .and_then(|ty| if arg.pat.span != ty.span { Some(ty.span) } else { None });
                     self_arg = if index == 0 && fn_decl.implicit_self.has_implicit_self() {
                         match fn_decl.implicit_self {
                             hir::ImplicitSelfKind::Imm => Some(ImplicitSelfKind::Imm),

diff --git a/src/test/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr b/src/test/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr
@@ -2,7 +2,7 @@ error[E0594]: cannot assign to `**t1`, which is behind a `&` reference
   --> $DIR/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs:9:5
    |
 LL |     let t1 = t0;
-   |         -- help: consider changing this to be a mutable reference: `&mut &mut isize`
+   |         -- consider changing this binding's type to be: `&mut &mut isize`
 LL |     let p: &isize = &**t0;
 LL |     **t1 = 22;
    |     ^^^^^^^^^ `t1` is a `&` reference, so the data it refers to cannot be written

diff --git a/src/test/ui/borrowck/issue-85765.rs b/src/test/ui/borrowck/issue-85765.rs
@@ -1,7 +1,7 @@
 fn main() {
     let mut test = Vec::new();
     let rofl: &Vec<Vec<i32>> = &mut test;
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     rofl.push(Vec::new());
     //~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
     //~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@@ -15,14 +15,14 @@ fn main() {
 
     #[rustfmt::skip]
     let x: &usize = &mut{0};
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     *x = 1;
     //~^ ERROR cannot assign to `*x`, which is behind a `&` reference
     //~| NOTE `x` is a `&` reference, so the data it refers to cannot be written
 
     #[rustfmt::skip]
     let y: &usize = &mut(0);
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     *y = 1;
     //~^ ERROR cannot assign to `*y`, which is behind a `&` reference
     //~| NOTE `y` is a `&` reference, so the data it refers to cannot be written

diff --git a/src/test/ui/borrowck/issue-85765.stderr b/src/test/ui/borrowck/issue-85765.stderr
@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
   --> $DIR/issue-85765.rs:5:5
    |
 LL |     let rofl: &Vec<Vec<i32>> = &mut test;
-   |         ---- help: consider changing this to be a mutable reference: `&mut Vec<Vec<i32>>`
+   |         ---- consider changing this binding's type to be: `&mut Vec<Vec<i32>>`
 LL |
 LL |     rofl.push(Vec::new());
    |     ^^^^^^^^^^^^^^^^^^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@@ -20,7 +20,7 @@ error[E0594]: cannot assign to `*x`, which is behind a `&` reference
   --> $DIR/issue-85765.rs:19:5
    |
 LL |     let x: &usize = &mut{0};
-   |         - help: consider changing this to be a mutable reference: `&mut usize`
+   |         - consider changing this binding's type to be: `&mut usize`
 LL |
 LL |     *x = 1;
    |     ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@@ -29,7 +29,7 @@ error[E0594]: cannot assign to `*y`, which is behind a `&` reference
   --> $DIR/issue-85765.rs:26:5
    |
 LL |     let y: &usize = &mut(0);
-   |         - help: consider changing this to be a mutable reference: `&mut usize`
+   |         - consider changing this binding's type to be: `&mut usize`
 LL |
 LL |     *y = 1;
    |     ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written

diff --git a/src/test/ui/borrowck/issue-91206.rs b/src/test/ui/borrowck/issue-91206.rs
@@ -9,7 +9,8 @@ impl TestClient {
 fn main() {
     let client = TestClient;
     let inner = client.get_inner_ref();
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     inner.clear();
     //~^ ERROR cannot borrow `*inner` as mutable, as it is behind a `&` reference [E0596]
+    //~| NOTE `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
 }
diff --git a/src/test/ui/borrowck/issue-91206.stderr b/src/test/ui/borrowck/issue-91206.stderr
@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference
   --> $DIR/issue-91206.rs:13:5
    |
 LL |     let inner = client.get_inner_ref();
-   |         ----- help: consider changing this to be a mutable reference: `&mut Vec<usize>`
+   |         ----- consider changing this binding's type to be: `&mut Vec<usize>`
 LL |
 LL |     inner.clear();
    |     ^^^^^^^^^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable

diff --git a/src/test/ui/borrowck/issue-92015.stderr b/src/test/ui/borrowck/issue-92015.stderr
@@ -2,7 +2,7 @@ error[E0594]: cannot assign to `*foo`, which is behind a `&` reference
   --> $DIR/issue-92015.rs:6:5
    |
 LL |     let foo = Some(&0).unwrap();
-   |         --- help: consider changing this to be a mutable reference: `&mut i32`
+   |         --- consider changing this binding's type to be: `&mut i32`
 LL |     *foo = 1;
    |     ^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written
 

diff --git a/src/test/ui/borrowck/suggest-as-ref-on-mut-closure.rs b/src/test/ui/borrowck/suggest-as-ref-on-mut-closure.rs
@@ -1,7 +1,7 @@
 // This is not exactly right, yet.
 
 // Ideally we should be suggesting `as_mut` for the first case,
-//and suggesting to change `as_ref` to `as_mut` in the second.
+// and suggesting to change `as_ref` to `as_mut` in the second.
 
 fn x(cb: &mut Option<&mut dyn FnMut()>) {
     cb.map(|cb| cb());

diff --git a/src/test/ui/borrowck/suggest-as-ref-on-mut-closure.stderr b/src/test/ui/borrowck/suggest-as-ref-on-mut-closure.stderr
@@ -23,7 +23,7 @@ error[E0596]: cannot borrow `*cb` as mutable, as it is behind a `&` reference
 LL |     cb.as_ref().map(|cb| cb());
    |                      --  ^^ `cb` is a `&` reference, so the data it refers to cannot be borrowed as mutable
    |                      |
-   |                      help: consider changing this to be a mutable reference: `&mut &mut dyn FnMut()`
+   |                      consider changing this binding's type to be: `&mut &mut dyn FnMut()`
 
 error: aborting due to 2 previous errors
 

diff --git a/src/test/ui/issues/issue-51515.rs b/src/test/ui/issues/issue-51515.rs
@@ -5,8 +5,6 @@ fn main() {
     *foo = 32;
     //~^ ERROR cannot assign to `*foo`, which is behind a `&` reference
     let bar = foo;
-    //~^ HELP consider changing this to be a mutable reference
-    //~| SUGGESTION &mut i32
     *bar = 64;
     //~^ ERROR cannot assign to `*bar`, which is behind a `&` reference
 }
diff --git a/src/test/ui/issues/issue-51515.stderr b/src/test/ui/issues/issue-51515.stderr
@@ -8,11 +8,10 @@ LL |     *foo = 32;
    |     ^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written
 
 error[E0594]: cannot assign to `*bar`, which is behind a `&` reference
-  --> $DIR/issue-51515.rs:10:5
+  --> $DIR/issue-51515.rs:8:5
    |
 LL |     let bar = foo;
-   |         --- help: consider changing this to be a mutable reference: `&mut i32`
-...
+   |         --- consider changing this binding's type to be: `&mut i32`
 LL |     *bar = 64;
    |     ^^^^^^^^^ `bar` is a `&` reference, so the data it refers to cannot be written