Skip to content

Commit f0e7af4

Browse files
Make suggest_deref_or_ref return a multipart suggestion
1 parent 1ca3bdf commit f0e7af4

10 files changed

+235
-176
lines changed

compiler/rustc_hir_typeck/src/demand.rs

+62-59
Original file line numberDiff line numberDiff line change
@@ -1087,7 +1087,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10871087
/// ```ignore (illustrative)
10881088
/// opt.map(|param| { takes_ref(param) });
10891089
/// ```
1090-
fn can_use_as_ref(&self, expr: &hir::Expr<'_>) -> Option<(Span, &'static str, String)> {
1090+
fn can_use_as_ref(&self, expr: &hir::Expr<'_>) -> Option<(Vec<(Span, String)>, &'static str)> {
10911091
let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = expr.kind else {
10921092
return None;
10931093
};
@@ -1133,12 +1133,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11331133
}
11341134
_ => false,
11351135
};
1136-
match (is_as_ref_able, self.sess().source_map().span_to_snippet(method_path.ident.span)) {
1137-
(true, Ok(src)) => {
1138-
let suggestion = format!("as_ref().{}", src);
1139-
Some((method_path.ident.span, "consider using `as_ref` instead", suggestion))
1140-
}
1141-
_ => None,
1136+
if is_as_ref_able {
1137+
Some((
1138+
vec![(method_path.ident.span.shrink_to_lo(), "as_ref().".to_string())],
1139+
"consider using `as_ref` instead",
1140+
))
1141+
} else {
1142+
None
11421143
}
11431144
}
11441145

@@ -1223,8 +1224,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12231224
checked_ty: Ty<'tcx>,
12241225
expected: Ty<'tcx>,
12251226
) -> Option<(
1226-
Span,
1227-
String,
1227+
Vec<(Span, String)>,
12281228
String,
12291229
Applicability,
12301230
bool, /* verbose */
@@ -1254,30 +1254,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12541254
&& let Ok(src) = sm.span_to_snippet(sp)
12551255
&& replace_prefix(&src, "b\"", "\"").is_some()
12561256
{
1257-
let pos = sp.lo() + BytePos(1);
1258-
return Some((
1259-
sp.with_hi(pos),
1260-
"consider removing the leading `b`".to_string(),
1261-
String::new(),
1262-
Applicability::MachineApplicable,
1263-
true,
1264-
false,
1265-
));
1266-
}
1267-
}
1257+
let pos = sp.lo() + BytePos(1);
1258+
return Some((
1259+
vec![(sp.with_hi(pos), String::new())],
1260+
"consider removing the leading `b`".to_string(),
1261+
Applicability::MachineApplicable,
1262+
true,
1263+
false,
1264+
));
1265+
}
1266+
}
12681267
(&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => {
12691268
if let hir::ExprKind::Lit(_) = expr.kind
12701269
&& let Ok(src) = sm.span_to_snippet(sp)
12711270
&& replace_prefix(&src, "\"", "b\"").is_some()
12721271
{
1273-
return Some((
1274-
sp.shrink_to_lo(),
1275-
"consider adding a leading `b`".to_string(),
1276-
"b".to_string(),
1277-
Applicability::MachineApplicable,
1278-
true,
1279-
false,
1280-
));
1272+
return Some((
1273+
vec![(sp.shrink_to_lo(), "b".to_string())],
1274+
"consider adding a leading `b`".to_string(),
1275+
Applicability::MachineApplicable,
1276+
true,
1277+
false,
1278+
));
12811279
}
12821280
}
12831281
_ => {}
@@ -1320,14 +1318,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13201318
}
13211319

13221320
if let hir::ExprKind::Unary(hir::UnOp::Deref, ref inner) = expr.kind
1323-
&& let Some(1) = self.deref_steps(expected, checked_ty) {
1321+
&& let Some(1) = self.deref_steps(expected, checked_ty)
1322+
{
13241323
// We have `*&T`, check if what was expected was `&T`.
13251324
// If so, we may want to suggest removing a `*`.
13261325
sugg_sp = sugg_sp.with_hi(inner.span.lo());
13271326
return Some((
1328-
sugg_sp,
1327+
vec![(sugg_sp, String::new())],
13291328
"consider removing deref here".to_string(),
1330-
"".to_string(),
13311329
Applicability::MachineApplicable,
13321330
true,
13331331
false,
@@ -1342,13 +1340,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13421340
_ => false,
13431341
};
13441342

1345-
if let Some(sugg) = self.can_use_as_ref(expr) {
1343+
if let Some((sugg, msg)) = self.can_use_as_ref(expr) {
13461344
return Some((
1347-
sugg.0,
1348-
sugg.1.to_string(),
1349-
sugg.2,
1345+
sugg,
1346+
msg.to_string(),
13501347
Applicability::MachineApplicable,
1351-
false,
1348+
true,
13521349
false,
13531350
));
13541351
}
@@ -1369,16 +1366,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
13691366
}
13701367
}
13711368

1372-
let (sp, sugg_expr, verbose) = if needs_parens {
1373-
let src = sm.span_to_snippet(sugg_sp).ok()?;
1374-
(sp, format!("({src})"), false)
1369+
let sugg = mutability.ref_prefix_str();
1370+
let (sugg, verbose) = if needs_parens {
1371+
(
1372+
vec![
1373+
(sp.shrink_to_lo(), format!("{prefix}{sugg}(")),
1374+
(sp.shrink_to_hi(), ")".to_string()),
1375+
],
1376+
false,
1377+
)
13751378
} else {
1376-
(sp.shrink_to_lo(), "".to_string(), true)
1379+
(vec![(sp.shrink_to_lo(), format!("{prefix}{sugg}"))], true)
13771380
};
13781381
return Some((
1379-
sp,
1382+
sugg,
13801383
format!("consider {}borrowing here", mutability.mutably_str()),
1381-
format!("{prefix}{}{sugg_expr}", mutability.ref_prefix_str()),
13821384
Applicability::MachineApplicable,
13831385
verbose,
13841386
false,
@@ -1404,23 +1406,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14041406
&& sm.is_span_accessible(call_span)
14051407
{
14061408
return Some((
1407-
sp.with_hi(call_span.lo()),
1409+
vec![(sp.with_hi(call_span.lo()), String::new())],
14081410
"consider removing the borrow".to_string(),
1409-
String::new(),
14101411
Applicability::MachineApplicable,
14111412
true,
1412-
true
1413+
true,
14131414
));
14141415
}
14151416
return None;
14161417
}
1417-
if sp.contains(expr.span)
1418-
&& sm.is_span_accessible(expr.span)
1419-
{
1418+
if sp.contains(expr.span) && sm.is_span_accessible(expr.span) {
14201419
return Some((
1421-
sp.with_hi(expr.span.lo()),
1420+
vec![(sp.with_hi(expr.span.lo()), String::new())],
14221421
"consider removing the borrow".to_string(),
1423-
String::new(),
14241422
Applicability::MachineApplicable,
14251423
true,
14261424
true,
@@ -1444,23 +1442,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14441442

14451443
let suggestion = replace_prefix(&src, old_prefix, &new_prefix).map(|_| {
14461444
// skip `&` or `&mut ` if both mutabilities are mutable
1447-
let lo = sp.lo() + BytePos(min(old_prefix.len(), mutbl_b.ref_prefix_str().len()) as _);
1445+
let lo = sp.lo()
1446+
+ BytePos(min(old_prefix.len(), mutbl_b.ref_prefix_str().len()) as _);
14481447
// skip `&` or `&mut `
14491448
let hi = sp.lo() + BytePos(old_prefix.len() as _);
14501449
let sp = sp.with_lo(lo).with_hi(hi);
14511450

14521451
(
14531452
sp,
1454-
format!("{}{derefs}", if mutbl_a != mutbl_b { mutbl_b.prefix_str() } else { "" }),
1455-
if mutbl_b <= mutbl_a { Applicability::MachineApplicable } else { Applicability::MaybeIncorrect }
1453+
format!(
1454+
"{}{derefs}",
1455+
if mutbl_a != mutbl_b { mutbl_b.prefix_str() } else { "" }
1456+
),
1457+
if mutbl_b <= mutbl_a {
1458+
Applicability::MachineApplicable
1459+
} else {
1460+
Applicability::MaybeIncorrect
1461+
},
14561462
)
14571463
});
14581464

14591465
if let Some((span, src, applicability)) = suggestion {
14601466
return Some((
1461-
span,
1467+
vec![(span, src)],
14621468
"consider dereferencing".to_string(),
1463-
src,
14641469
applicability,
14651470
true,
14661471
false,
@@ -1489,9 +1494,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
14891494
// If we've reached our target type with just removing `&`, then just print now.
14901495
if steps == 0 && !remove.trim().is_empty() {
14911496
return Some((
1492-
prefix_span,
1497+
vec![(prefix_span, String::new())],
14931498
format!("consider removing the `{}`", remove.trim()),
1494-
String::new(),
14951499
// Do not remove `&&` to get to bool, because it might be something like
14961500
// { a } && b, which we have a separate fixup suggestion that is more
14971501
// likely correct...
@@ -1557,9 +1561,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15571561
}
15581562

15591563
return Some((
1560-
span,
1564+
vec![(span, suggestion)],
15611565
message,
1562-
suggestion,
15631566
Applicability::MachineApplicable,
15641567
true,
15651568
false,

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
274274
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
275275
) -> bool {
276276
let expr = expr.peel_blocks();
277-
if let Some((sp, msg, suggestion, applicability, verbose, annotation)) =
277+
if let Some((suggestion, msg, applicability, verbose, annotation)) =
278278
self.suggest_deref_or_ref(expr, found, expected)
279279
{
280280
if verbose {
281-
err.span_suggestion_verbose(sp, &msg, suggestion, applicability);
281+
err.multipart_suggestion_verbose(&msg, suggestion, applicability);
282282
} else {
283-
err.span_suggestion(sp, &msg, suggestion, applicability);
283+
err.multipart_suggestion(&msg, suggestion, applicability);
284284
}
285285
if annotation {
286286
let suggest_annotation = match expr.peel_drop_temps().kind {

tests/ui/issues/issue-46756-consider-borrowing-cast-or-binexpr.stderr

+12-8
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,37 @@ error[E0308]: mismatched types
22
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:12:42
33
|
44
LL | light_flows_our_war_of_mocking_words(behold as usize);
5-
| ------------------------------------ ^^^^^^^^^^^^^^^
6-
| | |
7-
| | expected `&usize`, found `usize`
8-
| | help: consider borrowing here: `&(behold as usize)`
5+
| ------------------------------------ ^^^^^^^^^^^^^^^ expected `&usize`, found `usize`
6+
| |
97
| arguments to this function are incorrect
108
|
119
note: function defined here
1210
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:5:4
1311
|
1412
LL | fn light_flows_our_war_of_mocking_words(and_yet: &usize) -> usize {
1513
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------
14+
help: consider borrowing here
15+
|
16+
LL | light_flows_our_war_of_mocking_words(&(behold as usize));
17+
| ++ +
1618

1719
error[E0308]: mismatched types
1820
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:14:42
1921
|
2022
LL | light_flows_our_war_of_mocking_words(with_tears + 4);
21-
| ------------------------------------ ^^^^^^^^^^^^^^
22-
| | |
23-
| | expected `&usize`, found `usize`
24-
| | help: consider borrowing here: `&(with_tears + 4)`
23+
| ------------------------------------ ^^^^^^^^^^^^^^ expected `&usize`, found `usize`
24+
| |
2525
| arguments to this function are incorrect
2626
|
2727
note: function defined here
2828
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:5:4
2929
|
3030
LL | fn light_flows_our_war_of_mocking_words(and_yet: &usize) -> usize {
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------
32+
help: consider borrowing here
33+
|
34+
LL | light_flows_our_war_of_mocking_words(&(with_tears + 4));
35+
| ++ +
3236

3337
error: aborting due to 2 previous errors
3438

tests/ui/range/issue-54505-no-std.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -29,30 +29,30 @@ fn main() {
2929
take_range(0..1);
3030
//~^ ERROR mismatched types [E0308]
3131
//~| HELP consider borrowing here
32-
//~| SUGGESTION &(0..1)
32+
//~| SUGGESTION &(
3333

3434
take_range(1..);
3535
//~^ ERROR mismatched types [E0308]
3636
//~| HELP consider borrowing here
37-
//~| SUGGESTION &(1..)
37+
//~| SUGGESTION &(
3838

3939
take_range(..);
4040
//~^ ERROR mismatched types [E0308]
4141
//~| HELP consider borrowing here
42-
//~| SUGGESTION &(..)
42+
//~| SUGGESTION &(
4343

4444
take_range(0..=1);
4545
//~^ ERROR mismatched types [E0308]
4646
//~| HELP consider borrowing here
47-
//~| SUGGESTION &(0..=1)
47+
//~| SUGGESTION &(
4848

4949
take_range(..5);
5050
//~^ ERROR mismatched types [E0308]
5151
//~| HELP consider borrowing here
52-
//~| SUGGESTION &(..5)
52+
//~| SUGGESTION &(
5353

5454
take_range(..=42);
5555
//~^ ERROR mismatched types [E0308]
5656
//~| HELP consider borrowing here
57-
//~| SUGGESTION &(..=42)
57+
//~| SUGGESTION &(
5858
}

0 commit comments

Comments
 (0)