From b994124778907b5732f5453484b8b7a6812cf0f7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 31 Dec 2024 01:03:18 +0000 Subject: [PATCH 1/3] Explain how to mutate a HashMap/BTreeMap with more nuance --- compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs | 2 +- tests/ui/borrowck/index-mut-help.stderr | 2 +- tests/ui/btreemap/btreemap-index-mut-2.stderr | 2 +- tests/ui/btreemap/btreemap-index-mut.stderr | 2 +- tests/ui/hashmap/hashmap-index-mut.stderr | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 2484f817a06f8..4f82dfeb18bc9 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -575,7 +575,7 @@ 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![ diff --git a/tests/ui/borrowck/index-mut-help.stderr b/tests/ui/borrowck/index-mut-help.stderr index fde2b5dc07626..449fe42353a65 100644 --- a/tests/ui/borrowck/index-mut-help.stderr +++ b/tests/ui/borrowck/index-mut-help.stderr @@ -14,7 +14,7 @@ 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()); | ~~~~~~~~ ~ + diff --git a/tests/ui/btreemap/btreemap-index-mut-2.stderr b/tests/ui/btreemap/btreemap-index-mut-2.stderr index 0b8c77cb9e104..4589416b31963 100644 --- a/tests/ui/btreemap/btreemap-index-mut-2.stderr +++ b/tests/ui/btreemap/btreemap-index-mut-2.stderr @@ -5,7 +5,7 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap` -help: to modify a `BTreeMap`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `BTreeMap`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert(&0, 1); | ~~~~~~~~ ~ + diff --git a/tests/ui/btreemap/btreemap-index-mut.stderr b/tests/ui/btreemap/btreemap-index-mut.stderr index cc465fbf3def8..2f53296d63025 100644 --- a/tests/ui/btreemap/btreemap-index-mut.stderr +++ b/tests/ui/btreemap/btreemap-index-mut.stderr @@ -5,7 +5,7 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap` -help: to modify a `BTreeMap`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `BTreeMap`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert(&0, 1); | ~~~~~~~~ ~ + diff --git a/tests/ui/hashmap/hashmap-index-mut.stderr b/tests/ui/hashmap/hashmap-index-mut.stderr index 2381b8ecb9692..6b294823f6f95 100644 --- a/tests/ui/hashmap/hashmap-index-mut.stderr +++ b/tests/ui/hashmap/hashmap-index-mut.stderr @@ -5,7 +5,7 @@ LL | map[&0] = 1; | ^^^^^^^^^^^ cannot assign | = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap` -help: to modify a `HashMap`, use `.get_mut()`, `.insert()` or the entry API +help: use `.insert()` to insert a value into a `HashMap`, `.get_mut()` to modify it, or the entry API for more flexibility | LL | map.insert(&0, 1); | ~~~~~~~~ ~ + From 6a3474e653cbd77941f540852027f136c1d741c4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 31 Dec 2024 01:11:38 +0000 Subject: [PATCH 2/3] Use if-let in structured suggestion instead of Option::map --- .../src/diagnostics/mutability_errors.rs | 15 +++++++++------ tests/ui/borrowck/index-mut-help.stderr | 4 ++-- tests/ui/btreemap/btreemap-index-mut-2.stderr | 4 ++-- tests/ui/btreemap/btreemap-index-mut.stderr | 4 ++-- tests/ui/hashmap/hashmap-index-mut.stderr | 4 ++-- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 4f82dfeb18bc9..109ec096417fe 100644 --- 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!( - "use `.insert()` to insert a value into a `{}`, `.get_mut()` to modify it, or the entry API for more flexibility", + "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); @@ -628,15 +630,16 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { 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 index 449fe42353a65..8766ff4a6c41a 100644 --- a/tests/ui/borrowck/index-mut-help.stderr +++ b/tests/ui/borrowck/index-mut-help.stderr @@ -18,8 +18,8 @@ help: use `.insert()` to insert a value into a `HashMap<&str, String>`, `.get_mu | 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 index 4589416b31963..c42462ee1eb53 100644 --- a/tests/ui/btreemap/btreemap-index-mut-2.stderr +++ b/tests/ui/btreemap/btreemap-index-mut-2.stderr @@ -9,8 +9,8 @@ help: use `.insert()` to insert a value into a `BTreeMap`, `.get_mut() | 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 index 2f53296d63025..f402f503c1578 100644 --- a/tests/ui/btreemap/btreemap-index-mut.stderr +++ b/tests/ui/btreemap/btreemap-index-mut.stderr @@ -9,8 +9,8 @@ help: use `.insert()` to insert a value into a `BTreeMap`, `.get_mut() | 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 index 6b294823f6f95..ad33c6f9b1540 100644 --- a/tests/ui/hashmap/hashmap-index-mut.stderr +++ b/tests/ui/hashmap/hashmap-index-mut.stderr @@ -9,8 +9,8 @@ help: use `.insert()` to insert a value into a `HashMap`, `.get_mut()` | 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); | +++++++++ ~~~~~~~ ~~~~~~~~~~~~ + From f28e13b0552ab7b95e0cc764c5285ebac23fcaa2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 31 Dec 2024 01:20:45 +0000 Subject: [PATCH 3/3] Fix span for IndexMut method call on HashMap/BTreeMap --- compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs | 4 ++-- tests/ui/borrowck/index-mut-help.stderr | 5 ++++- tests/ui/issues/issue-41726.stderr | 5 ++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 109ec096417fe..c690789b587f6 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -624,7 +624,7 @@ 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( @@ -639,7 +639,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { index.span.shrink_to_hi().with_hi(receiver.span.hi()), ") { 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 index 8766ff4a6c41a..c4c9c1c531399 100644 --- 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 diff --git a/tests/ui/issues/issue-41726.stderr b/tests/ui/issues/issue-41726.stderr index fe7d4df70676f..250bba222bfdb 100644 --- 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>` - = help: to modify a `HashMap>`, use `.get_mut()`, `.insert()` or the entry API +help: to modify a `HashMap>` use `.get_mut()` + | +LL | if let Some(val) = things.get_mut(src.as_str()) { val.sort(); }; + | ++++++++++++++++++ ~~~~~~~~~ ~~~~~~~ +++ error: aborting due to 1 previous error