Skip to content

Commit 7529029

Browse files
estebankestebank
committed
Provide structured suggestion for hashmap[idx] = val 
1 parent 76531be commit 7529029

File tree

6 files changed

+149
-13
lines changed

6 files changed

+149
-13
lines changed

‎compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

+113-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
1+
use rustc_errors::{
2+
Applicability, Diagnostic, DiagnosticBuilder, EmissionGuarantee, ErrorGuaranteed,
3+
};
24
use rustc_hir as hir;
5+
use rustc_hir::intravisit::Visitor;
36
use rustc_hir::Node;
47
use rustc_middle::hir::map::Map;
58
use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
@@ -614,7 +617,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
614617
"trait `IndexMut` is required to modify indexed content, \
615618
but it is not implemented for `{ty}`",
616619
));
617-
self.suggest_map_index_mut_alternatives(ty, &mut err);
620+
self.suggest_map_index_mut_alternatives(ty, &mut err, span);
618621
}
619622
_ => (),
620623
}
@@ -632,13 +635,120 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
632635
&self,
633636
ty: Ty<'_>,
634637
err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
638+
span: Span,
635639
) {
636640
let Some(adt) = ty.ty_adt_def() else { return };
637641
let did = adt.did();
638642
if self.infcx.tcx.is_diagnostic_item(sym::HashMap, did)
639643
|| self.infcx.tcx.is_diagnostic_item(sym::BTreeMap, did)
640644
{
641-
err.help(format!("to modify a `{ty}`, use `.get_mut()`, `.insert()` or the entry API"));
645+
struct V<'a, 'b, 'tcx, G: EmissionGuarantee> {
646+
assign_span: Span,
647+
err: &'a mut DiagnosticBuilder<'b, G>,
648+
ty: Ty<'tcx>,
649+
suggested: bool,
650+
}
651+
impl<'a, 'b: 'a, 'hir, 'tcx, G: EmissionGuarantee> Visitor<'hir> for V<'a, 'b, 'tcx, G> {
652+
fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'hir>) {
653+
hir::intravisit::walk_stmt(self, stmt);
654+
let expr = match stmt.kind {
655+
hir::StmtKind::Semi(expr) | hir::StmtKind::Expr(expr) => expr,
656+
hir::StmtKind::Local(hir::Local { init: Some(expr), .. }) => expr,
657+
_ => {
658+
return;
659+
}
660+
};
661+
if let hir::ExprKind::Assign(place, rv, _sp) = expr.kind
662+
&& let hir::ExprKind::Index(val, index) = place.kind
663+
&& (expr.span == self.assign_span || place.span == self.assign_span)
664+
{
665+
// val[index] = rv;
666+
// ---------- place
667+
self.err.multipart_suggestions(
668+
&format!(
669+
"to modify a `{}`, use `.get_mut()`, `.insert()` or the entry API",
670+
self.ty,
671+
),
672+
vec![
673+
vec![ // val.insert(index, rv);
674+
(
675+
val.span.shrink_to_hi().with_hi(index.span.lo()),
676+
".insert(".to_string(),
677+
),
678+
(
679+
index.span.shrink_to_hi().with_hi(rv.span.lo()),
680+
", ".to_string(),
681+
),
682+
(rv.span.shrink_to_hi(), ")".to_string()),
683+
],
684+
vec![ // val.get_mut(index).map(|v| { *v = rv; });
685+
(
686+
val.span.shrink_to_hi().with_hi(index.span.lo()),
687+
".get_mut(".to_string(),
688+
),
689+
(
690+
index.span.shrink_to_hi().with_hi(place.span.hi()),
691+
").map(|val| { *val".to_string(),
692+
),
693+
(
694+
rv.span.shrink_to_hi(),
695+
"; })".to_string(),
696+
),
697+
],
698+
vec![ // let x = val.entry(index).or_insert(rv);
699+
(val.span.shrink_to_lo(), "let val = ".to_string()),
700+
(
701+
val.span.shrink_to_hi().with_hi(index.span.lo()),
702+
".entry(".to_string(),
703+
),
704+
(
705+
index.span.shrink_to_hi().with_hi(rv.span.lo()),
706+
").or_insert(".to_string(),
707+
),
708+
(rv.span.shrink_to_hi(), ")".to_string()),
709+
],
710+
].into_iter(),
711+
Applicability::MachineApplicable,
712+
);
713+
self.suggested = true;
714+
} else if let hir::ExprKind::MethodCall(_path, args @ [_, ..], sp) = expr.kind
715+
&& let hir::ExprKind::Index(val, index) = args[0].kind
716+
&& expr.span == self.assign_span
717+
{
718+
// val[index].path(args..);
719+
self.err.multipart_suggestion(
720+
&format!("to modify a `{}` use `.get_mut()`", self.ty),
721+
vec![
722+
(
723+
val.span.shrink_to_hi().with_hi(index.span.lo()),
724+
".get_mut(".to_string(),
725+
),
726+
(
727+
index.span.shrink_to_hi().with_hi(args[0].span.hi()),
728+
").map(|val| val".to_string(),
729+
),
730+
(sp.shrink_to_hi(), ")".to_string()),
731+
],
732+
Applicability::MachineApplicable,
733+
);
734+
self.suggested = true;
735+
}
736+
}
737+
}
738+
let hir_map = self.infcx.tcx.hir();
739+
let def_id = self.body.source.def_id();
740+
let hir_id = hir_map.local_def_id_to_hir_id(def_id.as_local().unwrap());
741+
let node = hir_map.find(hir_id);
742+
let Some(hir::Node::Item(item)) = node else { return; };
743+
let hir::ItemKind::Fn(.., body_id) = item.kind else { return; };
744+
let body = self.infcx.tcx.hir().body(body_id);
745+
let mut v = V { assign_span: span, err, ty, suggested: false };
746+
v.visit_body(body);
747+
if !v.suggested {
748+
err.help(&format!(
749+
"to modify a `{ty}`, use `.get_mut()`, `.insert()` or the entry API",
750+
));
751+
}
642752
}
643753
}
644754

‎src/test/ui/borrowck/index-mut-help.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
// When mutably indexing a type that implements `Index` but not `IndexMut`, a
22
// special 'help' message is added to the output.
3+
use std::collections::HashMap;
34

45

56
fn main() {
6-
use std::collections::HashMap;
7-
87
let mut map = HashMap::new();
98
map.insert("peter", "23".to_string());
109

‎src/test/ui/borrowck/index-mut-help.stderr

+15-5
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,33 @@
11
error[E0596]: cannot borrow data in an index of `HashMap<&str, String>` as mutable
2-
--> $DIR/index-mut-help.rs:11:5
2+
--> $DIR/index-mut-help.rs:10:5
33
|
44
LL | map["peter"].clear();
55
| ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>`
8-
= help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: to modify a `HashMap<&str, String>` use `.get_mut()`
9+
|
10+
LL | map.get_mut("peter").map(|val| val.clear());
11+
| ~~~~~~~~~ ~~~~~~~~~~~~~~~ +
912

1013
error[E0594]: cannot assign to data in an index of `HashMap<&str, String>`
11-
--> $DIR/index-mut-help.rs:12:5
14+
--> $DIR/index-mut-help.rs:11:5
1215
|
1316
LL | map["peter"] = "0".to_string();
1417
| ^^^^^^^^^^^^ cannot assign
1518
|
1619
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<&str, String>`
17-
= help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
20+
help: to modify a `HashMap<&str, String>`, use `.get_mut()`, `.insert()` or the entry API
21+
|
22+
LL | map.insert("peter", "0".to_string());
23+
| ~~~~~~~~ ~ +
24+
LL | map.get_mut("peter").map(|val| { *val = "0".to_string(); });
25+
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
26+
LL | let val = map.entry("peter").or_insert("0".to_string());
27+
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
1828

1929
error[E0596]: cannot borrow data in an index of `HashMap<&str, String>` as mutable
20-
--> $DIR/index-mut-help.rs:13:13
30+
--> $DIR/index-mut-help.rs:12:13
2131
|
2232
LL | let _ = &mut map["peter"];
2333
| ^^^^^^^^^^^^^^^^^ cannot borrow as mutable

‎src/test/ui/btreemap/btreemap-index-mut.stderr

+8-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,14 @@ LL | map[&0] = 1;
55
| ^^^^^^^^^^^ cannot assign
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `BTreeMap<u32, u32>`
8-
= help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: to modify a `BTreeMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
9+
|
10+
LL | map.insert(&0, 1);
11+
| ~~~~~~~~ ~ +
12+
LL | map.get_mut(&0).map(|val| { *val = 1; });
13+
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
14+
LL | let val = map.entry(&0).or_insert(1);
15+
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
916

1017
error: aborting due to previous error
1118

‎src/test/ui/hashmap/hashmap-index-mut.stderr

+8-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,14 @@ LL | map[&0] = 1;
55
| ^^^^^^^^^^^ cannot assign
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<u32, u32>`
8-
= help: to modify a `HashMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: to modify a `HashMap<u32, u32>`, use `.get_mut()`, `.insert()` or the entry API
9+
|
10+
LL | map.insert(&0, 1);
11+
| ~~~~~~~~ ~ +
12+
LL | map.get_mut(&0).map(|val| { *val = 1; });
13+
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~ ++++
14+
LL | let val = map.entry(&0).or_insert(1);
15+
| +++++++++ ~~~~~~~ ~~~~~~~~~~~~ +
916

1017
error: aborting due to previous error
1118

‎src/test/ui/issues/issue-41726.stderr

+4-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ LL | things[src.as_str()].sort();
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
66
|
77
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `HashMap<String, Vec<String>>`
8-
= help: to modify a `HashMap<String, Vec<String>>`, use `.get_mut()`, `.insert()` or the entry API
8+
help: to modify a `HashMap<String, Vec<String>>` use `.get_mut()`
9+
|
10+
LL | things.get_mut(src.as_str()).map(|val| val.sort());
11+
| ~~~~~~~~~ ~~~~~~~~~~~~~~~ +
912

1013
error: aborting due to previous error
1114

0 commit comments

Comments
 (0)