Skip to content

Commit

Permalink
Make suggest_deref_or_ref return a multipart suggestion
Browse files Browse the repository at this point in the history
  • Loading branch information
@compiler-errors
compiler-errors committed Apr 18, 2023
1 parent ab99cdf commit cf24419
Show file tree
Hide file tree
Showing 11 changed files with 235 additions and 178 deletions.
121 changes: 62 additions & 59 deletions compiler/rustc_hir_typeck/src/demand.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// ```ignore (illustrative)
/// opt.map(|param| { takes_ref(param) });
/// ```
fn can_use_as_ref(&self, expr: &hir::Expr<'_>) -> Option<(Span, &'static str, String)> {
fn can_use_as_ref(&self, expr: &hir::Expr<'_>) -> Option<(Vec<(Span, String)>, &'static str)> {
let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = expr.kind else {
return None;
};
Expand Down Expand Up @@ -1133,12 +1133,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => false,
};
match (is_as_ref_able, self.sess().source_map().span_to_snippet(method_path.ident.span)) {
(true, Ok(src)) => {
let suggestion = format!("as_ref().{}", src);
Some((method_path.ident.span, "consider using `as_ref` instead", suggestion))
}
_ => None,
if is_as_ref_able {
Some((
vec![(method_path.ident.span.shrink_to_lo(), "as_ref().".to_string())],
"consider using `as_ref` instead",
))
} else {
None
}
}

Expand Down Expand Up @@ -1223,8 +1224,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
) -> Option<(
Span,
String,
Vec<(Span, String)>,
String,
Applicability,
bool, /* verbose */
Expand Down Expand Up @@ -1254,30 +1254,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& let Ok(src) = sm.span_to_snippet(sp)
&& replace_prefix(&src, "b\"", "\"").is_some()
{
let pos = sp.lo() + BytePos(1);
return Some((
sp.with_hi(pos),
"consider removing the leading `b`".to_string(),
String::new(),
Applicability::MachineApplicable,
true,
false,
));
}
}
let pos = sp.lo() + BytePos(1);
return Some((
vec![(sp.with_hi(pos), String::new())],
"consider removing the leading `b`".to_string(),
Applicability::MachineApplicable,
true,
false,
));
}
}
(&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => {
if let hir::ExprKind::Lit(_) = expr.kind
&& let Ok(src) = sm.span_to_snippet(sp)
&& replace_prefix(&src, "\"", "b\"").is_some()
{
return Some((
sp.shrink_to_lo(),
"consider adding a leading `b`".to_string(),
"b".to_string(),
Applicability::MachineApplicable,
true,
false,
));
return Some((
vec![(sp.shrink_to_lo(), "b".to_string())],
"consider adding a leading `b`".to_string(),
Applicability::MachineApplicable,
true,
false,
));
}
}
_ => {}
Expand Down Expand Up @@ -1320,14 +1318,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

if let hir::ExprKind::Unary(hir::UnOp::Deref, ref inner) = expr.kind
&& let Some(1) = self.deref_steps(expected, checked_ty) {
&& let Some(1) = self.deref_steps(expected, checked_ty)
{
// We have `*&T`, check if what was expected was `&T`.
// If so, we may want to suggest removing a `*`.
sugg_sp = sugg_sp.with_hi(inner.span.lo());
return Some((
sugg_sp,
vec![(sugg_sp, String::new())],
"consider removing deref here".to_string(),
"".to_string(),
Applicability::MachineApplicable,
true,
false,
Expand All @@ -1342,13 +1340,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => false,
};

if let Some(sugg) = self.can_use_as_ref(expr) {
if let Some((sugg, msg)) = self.can_use_as_ref(expr) {
return Some((
sugg.0,
sugg.1.to_string(),
sugg.2,
sugg,
msg.to_string(),
Applicability::MachineApplicable,
false,
true,
false,
));
}
Expand All @@ -1369,16 +1366,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

let (sp, sugg_expr, verbose) = if needs_parens {
let src = sm.span_to_snippet(sugg_sp).ok()?;
(sp, format!("({src})"), false)
let sugg = mutability.ref_prefix_str();
let (sugg, verbose) = if needs_parens {
(
vec![
(sp.shrink_to_lo(), format!("{prefix}{sugg}(")),
(sp.shrink_to_hi(), ")".to_string()),
],
false,
)
} else {
(sp.shrink_to_lo(), "".to_string(), true)
(vec![(sp.shrink_to_lo(), format!("{prefix}{sugg}"))], true)
};
return Some((
sp,
sugg,
format!("consider {}borrowing here", mutability.mutably_str()),
format!("{prefix}{}{sugg_expr}", mutability.ref_prefix_str()),
Applicability::MachineApplicable,
verbose,
false,
Expand All @@ -1404,23 +1406,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&& sm.is_span_accessible(call_span)
{
return Some((
sp.with_hi(call_span.lo()),
vec![(sp.with_hi(call_span.lo()), String::new())],
"consider removing the borrow".to_string(),
String::new(),
Applicability::MachineApplicable,
true,
true
true,
));
}
return None;
}
if sp.contains(expr.span)
&& sm.is_span_accessible(expr.span)
{
if sp.contains(expr.span) && sm.is_span_accessible(expr.span) {
return Some((
sp.with_hi(expr.span.lo()),
vec![(sp.with_hi(expr.span.lo()), String::new())],
"consider removing the borrow".to_string(),
String::new(),
Applicability::MachineApplicable,
true,
true,
Expand All @@ -1444,23 +1442,30 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let suggestion = replace_prefix(&src, old_prefix, &new_prefix).map(|_| {
// skip `&` or `&mut ` if both mutabilities are mutable
let lo = sp.lo() + BytePos(min(old_prefix.len(), mutbl_b.ref_prefix_str().len()) as _);
let lo = sp.lo()
+ BytePos(min(old_prefix.len(), mutbl_b.ref_prefix_str().len()) as _);
// skip `&` or `&mut `
let hi = sp.lo() + BytePos(old_prefix.len() as _);
let sp = sp.with_lo(lo).with_hi(hi);

(
sp,
format!("{}{derefs}", if mutbl_a != mutbl_b { mutbl_b.prefix_str() } else { "" }),
if mutbl_b <= mutbl_a { Applicability::MachineApplicable } else { Applicability::MaybeIncorrect }
format!(
"{}{derefs}",
if mutbl_a != mutbl_b { mutbl_b.prefix_str() } else { "" }
),
if mutbl_b <= mutbl_a {
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
},
)
});

if let Some((span, src, applicability)) = suggestion {
return Some((
span,
vec![(span, src)],
"consider dereferencing".to_string(),
src,
applicability,
true,
false,
Expand Down Expand Up @@ -1489,9 +1494,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// If we've reached our target type with just removing `&`, then just print now.
if steps == 0 && !remove.trim().is_empty() {
return Some((
prefix_span,
vec![(prefix_span, String::new())],
format!("consider removing the `{}`", remove.trim()),
String::new(),
// Do not remove `&&` to get to bool, because it might be something like
// { a } && b, which we have a separate fixup suggestion that is more
// likely correct...
Expand Down Expand Up @@ -1551,9 +1555,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

return Some((
span,
vec![(span, suggestion)],
message,
suggestion,
Applicability::MachineApplicable,
true,
false,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,13 +274,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
) -> bool {
let expr = expr.peel_blocks();
if let Some((sp, msg, suggestion, applicability, verbose, annotation)) =
if let Some((suggestion, msg, applicability, verbose, annotation)) =
self.suggest_deref_or_ref(expr, found, expected)
{
if verbose {
err.span_suggestion_verbose(sp, &msg, suggestion, applicability);
err.multipart_suggestion_verbose(&msg, suggestion, applicability);
} else {
err.span_suggestion(sp, &msg, suggestion, applicability);
err.multipart_suggestion(&msg, suggestion, applicability);
}
if annotation {
let suggest_annotation = match expr.peel_drop_temps().kind {
Expand Down
20 changes: 12 additions & 8 deletions tests/ui/issues/issue-46756-consider-borrowing-cast-or-binexpr.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,37 @@ error[E0308]: mismatched types
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:12:42
|
LL | light_flows_our_war_of_mocking_words(behold as usize);
| ------------------------------------ ^^^^^^^^^^^^^^^
| | |
| | expected `&usize`, found `usize`
| | help: consider borrowing here: `&(behold as usize)`
| ------------------------------------ ^^^^^^^^^^^^^^^ expected `&usize`, found `usize`
| |
| arguments to this function are incorrect
|
note: function defined here
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:5:4
|
LL | fn light_flows_our_war_of_mocking_words(and_yet: &usize) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------
help: consider borrowing here
|
LL | light_flows_our_war_of_mocking_words(&(behold as usize));
| ++ +

error[E0308]: mismatched types
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:14:42
|
LL | light_flows_our_war_of_mocking_words(with_tears + 4);
| ------------------------------------ ^^^^^^^^^^^^^^
| | |
| | expected `&usize`, found `usize`
| | help: consider borrowing here: `&(with_tears + 4)`
| ------------------------------------ ^^^^^^^^^^^^^^ expected `&usize`, found `usize`
| |
| arguments to this function are incorrect
|
note: function defined here
--> $DIR/issue-46756-consider-borrowing-cast-or-binexpr.rs:5:4
|
LL | fn light_flows_our_war_of_mocking_words(and_yet: &usize) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ---------------
help: consider borrowing here
|
LL | light_flows_our_war_of_mocking_words(&(with_tears + 4));
| ++ +

error: aborting due to 2 previous errors

Expand Down
2 changes: 0 additions & 2 deletions tests/ui/methods/method-not-found-generic-arg-elision.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ error[E0599]: no method named `extend` found for struct `Map` in the current sco
|
LL | v.iter().map(|x| x * x).extend(std::iter::once(100));
| ^^^^^^ method not found in `Map<Iter<'_, i32>, [closure@method-not-found-generic-arg-elision.rs:87:18]>`
|
= note: the full type name has been written to '$TEST_BUILD_DIR/methods/method-not-found-generic-arg-elision/method-not-found-generic-arg-elision.long-type-5032081584140815090.txt'

error[E0599]: no method named `method` found for struct `Wrapper<bool>` in the current scope
--> $DIR/method-not-found-generic-arg-elision.rs:90:13
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/range/issue-54505-no-std.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,30 @@ fn main() {
take_range(0..1);
//~^ ERROR mismatched types [E0308]
//~| HELP consider borrowing here
//~| SUGGESTION &(0..1)
//~| SUGGESTION &(

take_range(1..);
//~^ ERROR mismatched types [E0308]
//~| HELP consider borrowing here
//~| SUGGESTION &(1..)
//~| SUGGESTION &(

take_range(..);
//~^ ERROR mismatched types [E0308]
//~| HELP consider borrowing here
//~| SUGGESTION &(..)
//~| SUGGESTION &(

take_range(0..=1);
//~^ ERROR mismatched types [E0308]
//~| HELP consider borrowing here
//~| SUGGESTION &(0..=1)
//~| SUGGESTION &(

take_range(..5);
//~^ ERROR mismatched types [E0308]
//~| HELP consider borrowing here
//~| SUGGESTION &(..5)
//~| SUGGESTION &(

take_range(..=42);
//~^ ERROR mismatched types [E0308]
//~| HELP consider borrowing here
//~| SUGGESTION &(..=42)
//~| SUGGESTION &(
}
Loading

0 comments on commit cf24419

Please sign in to comment.