Rollup merge of #134945 - compiler-errors:map-mutate-nits, r=estebank

Some small nits to the borrowck suggestions for mutating a map through index 1. Suggesting users to either use `.insert` or `.get_mut` (which do totally different things) can be a bit of a footgun, so let's make that a bit more nuanced. 2. I find the suggestion of `.get_mut(|val| { *val = whatever; })` to be a bit awkward. I changed this to be an if-let instead. 3. Fix a bug which was suppressing the structured suggestion for some mutations via the index operator on `HashMap`/`BTreeMap`. r? estebank or reassign
rust-lang · Jan 1, 2025 · f91bfd9 · f91bfd9
2 parents 1ea1db5 + f28e13b
commit f91bfd9
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 21 deletions.
diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -575,7 +575,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         // ---------- place
                         self.err.multipart_suggestions(
                             format!(
-                                "to modify a `{}`, use `.get_mut()`, `.insert()` or the entry API",
+                                "use `.insert()` to insert a value into a `{}`, `.get_mut()` \
+                                to modify it, or the entry API for more flexibility",
                                 self.ty,
                             ),
                             vec![
@@ -592,16 +593,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                                     (rv.span.shrink_to_hi(), ")".to_string()),
                                 ],
                                 vec![
-                                    // val.get_mut(index).map(|v| { *v = rv; });
+                                    // if let Some(v) = val.get_mut(index) { *v = rv; }
+                                    (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()),
                                     (
                                         val.span.shrink_to_hi().with_hi(index.span.lo()),
                                         ".get_mut(".to_string(),
                                     ),
                                     (
                                         index.span.shrink_to_hi().with_hi(place.span.hi()),
-                                        ").map(|val| { *val".to_string(),
+                                        ") { *val".to_string(),
                                     ),
-                                    (rv.span.shrink_to_hi(), "; })".to_string()),
+                                    (rv.span.shrink_to_hi(), "; }".to_string()),
                                 ],
                                 vec![
                                     // let x = val.entry(index).or_insert(rv);
@@ -622,21 +624,22 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                         self.suggested = true;
                     } else if let hir::ExprKind::MethodCall(_path, receiver, _, sp) = expr.kind
                         && let hir::ExprKind::Index(val, index, _) = receiver.kind
-                        && expr.span == self.assign_span
+                        && receiver.span == self.assign_span
                     {
                         // val[index].path(args..);
                         self.err.multipart_suggestion(
                             format!("to modify a `{}` use `.get_mut()`", self.ty),
                             vec![
+                                (val.span.shrink_to_lo(), "if let Some(val) = ".to_string()),
                                 (
                                     val.span.shrink_to_hi().with_hi(index.span.lo()),
                                     ".get_mut(".to_string(),
                                 ),
                                 (
                                     index.span.shrink_to_hi().with_hi(receiver.span.hi()),
-                                    ").map(|val| val".to_string(),
+                                    ") { val".to_string(),
                                 ),
-                                (sp.shrink_to_hi(), ")".to_string()),
+                                (sp.shrink_to_hi(), "; }".to_string()),
                             ],
                             Applicability::MachineApplicable,
                         );

diff --git a/tests/ui/borrowck/index-mut-help.stderr b/tests/ui/borrowck/index-mut-help.stderr
@@ -5,7 +5,10 @@ LL |     map["peter"].clear();
    |     ^^^^^^^^^^^^ cannot borrow as mutable
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>`
-   = help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
+help: to modify a `HashMap<&str, String>` use `.get_mut()`
+   |
+LL |     if let Some(val) = map.get_mut("peter") { val.clear(); };
+   |     ++++++++++++++++++    ~~~~~~~~~       ~~~~~~~        +++
 
 error[E0594]: cannot assign to data in an index of `HashMap<&str, String>`
   --> $DIR/index-mut-help.rs:11:5
@@ -14,12 +17,12 @@ LL |     map["peter"] = "0".to_string();
    |     ^^^^^^^^^^^^ cannot assign
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>`
-help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
+help: use `.insert()` to insert a value into a `HashMap<&str, String>`, `.get_mut()` to modify it, or the entry API for more flexibility
    |
 LL |     map.insert("peter", "0".to_string());
    |        ~~~~~~~~       ~                +
-LL |     map.get_mut("peter").map(|val| { *val = "0".to_string(); });
-   |        ~~~~~~~~~       ~~~~~~~~~~~~~~~~~~                  ++++
+LL |     if let Some(val) = map.get_mut("peter") { *val = "0".to_string(); };
+   |     ++++++++++++++++++    ~~~~~~~~~       ~~~~~~~~                  +++
 LL |     let val = map.entry("peter").or_insert("0".to_string());
    |     +++++++++    ~~~~~~~       ~~~~~~~~~~~~               +
 

diff --git a/tests/ui/btreemap/btreemap-index-mut-2.stderr b/tests/ui/btreemap/btreemap-index-mut-2.stderr
@@ -5,12 +5,12 @@ LL |         map[&0] = 1;
    |         ^^^^^^^^^^^ cannot assign
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
-help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
+help: use `.insert()` to insert a value into a `BTreeMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility
    |
 LL |         map.insert(&0, 1);
    |            ~~~~~~~~  ~  +
-LL |         map.get_mut(&0).map(|val| { *val = 1; });
-   |            ~~~~~~~~~  ~~~~~~~~~~~~~~~~~~    ++++
+LL |         if let Some(val) = map.get_mut(&0) { *val = 1; };
+   |         ++++++++++++++++++    ~~~~~~~~~  ~~~~~~~~    +++
 LL |         let val = map.entry(&0).or_insert(1);
    |         +++++++++    ~~~~~~~  ~~~~~~~~~~~~ +
 

diff --git a/tests/ui/btreemap/btreemap-index-mut.stderr b/tests/ui/btreemap/btreemap-index-mut.stderr
@@ -5,12 +5,12 @@ LL |     map[&0] = 1;
    |     ^^^^^^^^^^^ cannot assign
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
-help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
+help: use `.insert()` to insert a value into a `BTreeMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility
    |
 LL |     map.insert(&0, 1);
    |        ~~~~~~~~  ~  +
-LL |     map.get_mut(&0).map(|val| { *val = 1; });
-   |        ~~~~~~~~~  ~~~~~~~~~~~~~~~~~~    ++++
+LL |     if let Some(val) = map.get_mut(&0) { *val = 1; };
+   |     ++++++++++++++++++    ~~~~~~~~~  ~~~~~~~~    +++
 LL |     let val = map.entry(&0).or_insert(1);
    |     +++++++++    ~~~~~~~  ~~~~~~~~~~~~ +
 

diff --git a/tests/ui/hashmap/hashmap-index-mut.stderr b/tests/ui/hashmap/hashmap-index-mut.stderr
@@ -5,12 +5,12 @@ LL |     map[&0] = 1;
    |     ^^^^^^^^^^^ cannot assign
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<u32, u32>`
-help: to modify a `HashMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
+help: use `.insert()` to insert a value into a `HashMap<u32, u32>`, `.get_mut()` to modify it, or the entry API for more flexibility
    |
 LL |     map.insert(&0, 1);
    |        ~~~~~~~~  ~  +
-LL |     map.get_mut(&0).map(|val| { *val = 1; });
-   |        ~~~~~~~~~  ~~~~~~~~~~~~~~~~~~    ++++
+LL |     if let Some(val) = map.get_mut(&0) { *val = 1; };
+   |     ++++++++++++++++++    ~~~~~~~~~  ~~~~~~~~    +++
 LL |     let val = map.entry(&0).or_insert(1);
    |     +++++++++    ~~~~~~~  ~~~~~~~~~~~~ +
 

diff --git a/tests/ui/issues/issue-41726.stderr b/tests/ui/issues/issue-41726.stderr
@@ -5,7 +5,10 @@ LL |         things[src.as_str()].sort();
    |         ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
    |
    = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<String, Vec<String>>`
-   = help: to modify a `HashMap<String, Vec<String>>`, use `.get_mut()`, `.insert()` or the entry API
+help: to modify a `HashMap<String, Vec<String>>` use `.get_mut()`
+   |
+LL |         if let Some(val) = things.get_mut(src.as_str()) { val.sort(); };
+   |         ++++++++++++++++++       ~~~~~~~~~            ~~~~~~~       +++
 
 error: aborting due to 1 previous error